[Openvpn-devel,v2] options: drop useless init_gc param for init_options()

Message ID 20260217135605.154129-1-frank@lichtenheld.com
State New
Headers show
Series [Openvpn-devel,v2] options: drop useless init_gc param for init_options() | expand

Commit Message

Frank Lichtenheld Feb. 17, 2026, 1:56 p.m. UTC
From: Antonio Quartulli <antonio@mandelbit.com>

The init_option() function is always invoked with the second
param "init_gc" set to "true".
This makes the parameter useless and it can therefore be removed
while always taking the "true" branch in the related logic.

This way we can also drop the options->gc_owned member as it
would also be always set to true.

Change-Id: I633d8cbf75ab4da85e16df44684aef60523811c5
Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1536
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1536
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering Feb. 28, 2026, 10 a.m. UTC | #1
Well spotted... on the first glance, this was like "uh, would that be
safe?  what if anything else relies on gc_owned?", but on more thorough
reading it's really as you say - all callers pass "true", and this field
is not used anywhere ever again ("confirmed by grep and buildbot" ;-) ).

Your patch has been applied to the master and release/2.7 branch
(prerequisite for a subsequent bugfix).

commit 92937c267b20e3e1ffd4e0e80d8048a57da7edb6 (master)
commit acbe874c71444afffa2238a0b8b45d330b78f86c (release/2.7)
Author: Antonio Quartulli
Date:   Tue Feb 17 14:56:05 2026 +0100

     options: drop useless init_gc param for init_options()

     Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1536
     Message-Id: <20260217135605.154129-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg35695.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index b7eca19..f38660f 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -202,7 +202,7 @@ 
 #endif
 
             /* initialize options to default state */
-            init_options(&c.options, true);
+            init_options(&c.options);
 
             /* parse command line options, and read configuration file */
             parse_argv(&c.options, argc, argv, M_USAGE, OPT_P_DEFAULT, NULL, c.es);
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index cdb02e9..4e1e32a 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -792,15 +792,12 @@ 
  * will be set to 0.
  */
 void
-init_options(struct options *o, const bool init_gc)
+init_options(struct options *o)
 {
     CLEAR(*o);
-    if (init_gc)
-    {
-        gc_init(&o->gc);
-        gc_init(&o->dns_options.gc);
-        o->gc_owned = true;
-    }
+    gc_init(&o->gc);
+    gc_init(&o->dns_options.gc);
+
     o->mode = MODE_POINT_TO_POINT;
     o->topology = TOP_UNDEF;
     o->ce.proto = PROTO_UDP;
@@ -925,11 +922,9 @@ 
     {
         CLEAR(*o->remote_list);
     }
-    if (o->gc_owned)
-    {
-        gc_free(&o->gc);
-        gc_free(&o->dns_options.gc);
-    }
+
+    gc_free(&o->gc);
+    gc_free(&o->dns_options.gc);
 }
 
 #ifndef ENABLE_SMALL
@@ -2256,7 +2251,7 @@ 
     int dev = DEV_TYPE_UNDEF;
     bool pull = false;
 
-    init_options(&defaults, true);
+    init_options(&defaults);
 
     if (!options->test_crypto)
     {
@@ -4824,7 +4819,7 @@ 
 #else
 
     struct options o;
-    init_options(&o, true);
+    init_options(&o);
 
     fprintf(fp, usage_message, title_string, o.ce.connect_retry_seconds,
             o.ce.connect_retry_seconds_max, o.ce.local_port, o.ce.remote_port, TUN_MTU_DEFAULT,
@@ -5979,7 +5974,7 @@ 
             struct options sub;
             struct connection_entry *e;
 
-            init_options(&sub, true);
+            init_options(&sub);
             sub.ce = options->ce;
             read_config_string("[CONNECTION-OPTIONS]", &sub, p[1], msglevel, OPT_P_CONNECTION,
                                option_types_found, es);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index cf9936b..3d8b505 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -254,7 +254,6 @@ 
 struct options
 {
     struct gc_arena gc;
-    bool gc_owned;
 
     /* first config file */
     const char *config;
@@ -890,7 +889,7 @@ 
 
 void show_dco_version(const unsigned int flags);
 
-void init_options(struct options *o, const bool init_gc);
+void init_options(struct options *o);
 
 void uninit_options(struct options *o);
 
diff --git a/tests/unit_tests/openvpn/test_options_parse.c b/tests/unit_tests/openvpn/test_options_parse.c
index bb34f01..0b3d7fe 100644
--- a/tests/unit_tests/openvpn/test_options_parse.c
+++ b/tests/unit_tests/openvpn/test_options_parse.c
@@ -248,7 +248,6 @@ 
     CLEAR(o); /* NB: avoiding init_options to limit dependencies */
     gc_init(&o.gc);
     gc_init(&o.dns_options.gc);
-    o.gc_owned = true;
 
     char *p_expect_someopt[MAX_PARMS];
     char *p_expect_otheropt[MAX_PARMS];