[Openvpn-devel,v2,1/3] tun: ensure gc and argv are properly handled

Message ID 20180613122824.4207-2-a@unstable.cc
State Accepted
Headers show
Series pre-ipv6-only clean up patchset | expand

Commit Message

Antonio Quartulli June 13, 2018, 2:28 a.m. UTC
From: Antonio Quartulli <antonio@openvpn.net>

Everytime a argv object is initialized with argv_new(), it has
to be released with argv_reset() once not needed anymore.

Ensure this kind of objects are always properly released to avoid
memory leaks.

At the same time, remove those gc_arena objects that are initialized
but never used/released.

Signed-off-by: Antonio Quartulli <antonio@openvpn.net>

---

Changes from v1:
- remove gc_arena at all when not used


 src/openvpn/tun.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Comments

Gert Doering June 13, 2018, 3:12 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

"Proper code hygiene"... thanks.  "Stared at code" plus run on a few
platforms, including AIX ("because I can!").

Your patch has been applied to the master branch.

commit 18225f0fd5c45164308d4d66db055a1ad3960887
Author: Antonio Quartulli
Date:   Wed Jun 13 20:28:22 2018 +0800

     tun: ensure gc and argv are properly handled

     Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20180613122824.4207-2-a@unstable.cc>
     URL: https://www.mail-archive.com/search?l=mid&q=20180613122824.4207-2-a@unstable.cc
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 3eb0f78a..72506c4b 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -2600,7 +2600,6 @@  close_tun(struct tuntap *tt)
     }
     else if (tt)
     {
-        struct gc_arena gc = gc_new();
         struct argv argv = argv_new();
 
         /* setup command, close tun dev (clears tt->actual_name!), run command
@@ -2615,6 +2614,7 @@  close_tun(struct tuntap *tt)
         openvpn_execve_check(&argv, NULL, 0, "OpenBSD 'destroy tun interface' failed (non-critical)");
 
         free(tt);
+        argv_reset(&argv);
     }
 }
 
@@ -2685,7 +2685,6 @@  close_tun(struct tuntap *tt)
     }
     else if (tt)
     {
-        struct gc_arena gc = gc_new();
         struct argv argv = argv_new();
 
         /* setup command, close tun dev (clears tt->actual_name!), run command
@@ -2700,6 +2699,7 @@  close_tun(struct tuntap *tt)
         openvpn_execve_check(&argv, NULL, 0, "NetBSD 'destroy tun interface' failed (non-critical)");
 
         free(tt);
+        argv_reset(&argv);
     }
 }
 
@@ -2837,6 +2837,7 @@  close_tun(struct tuntap *tt)
         openvpn_execve_check(&argv, NULL, 0, "FreeBSD 'destroy tun interface' failed (non-critical)");
 
         free(tt);
+        argv_reset(&argv);
     }
 }
 
@@ -3308,6 +3309,7 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
         env_set_add( es, "ODMDIR=/etc/objrepos" );
         openvpn_execve_check(&argv, es, S_FATAL, "AIX 'create tun interface' failed");
         env_set_destroy(es);
+        argv_reset(&argv);
     }
     else
     {
@@ -3333,7 +3335,6 @@  open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun
 void
 close_tun(struct tuntap *tt)
 {
-    struct gc_arena gc = gc_new();
     struct argv argv = argv_new();
     struct env_set *es = env_set_create(NULL);
 
@@ -3362,6 +3363,7 @@  close_tun(struct tuntap *tt)
 
     free(tt);
     env_set_destroy(es);
+    argv_reset(&argv);
 }
 
 int