[Openvpn-devel,v4] Refactor early initialisation and uninitialisation into methods

Message ID 20211105161302.2916786-1-arne@rfc2549.org
State Superseded
Headers show
Series [Openvpn-devel,v4] Refactor early initialisation and uninitialisation into methods | expand

Commit Message

Arne Schwabe Nov. 5, 2021, 5:13 a.m. UTC
This put the early initialisation and uninitialisation that needs to
happen between option parsing and post processing into small methods.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/openvpn.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Gert Doering Nov. 5, 2021, 5:20 a.m. UTC | #1
Hi,

On Fri, Nov 05, 2021 at 05:13:02PM +0100, Arne Schwabe wrote:
> +static void uninit_early(struct context *c)
> +{
> +    net_ctx_free(&c->net_ctx);
> +}

The extra "&" here does not look right.  Shouldn't this be
"c->net_ctx" as in "init_early()"?

gert
Antonio Quartulli Nov. 5, 2021, 5:31 a.m. UTC | #2
Hi,

On 05/11/2021 17:20, Gert Doering wrote:
> Hi,
> 
> On Fri, Nov 05, 2021 at 05:13:02PM +0100, Arne Schwabe wrote:
>> +static void uninit_early(struct context *c)
>> +{
>> +    net_ctx_free(&c->net_ctx);
>> +}
> 
> The extra "&" here does not look right.  Shouldn't this be
> "c->net_ctx" as in "init_early()"?

it's the other way around.

"init_early()" should also use the "&" operator.

We need to pass a pointer to the networking context, but the member is a 
real object.

(Check the code being removed for reference).

Regards,
Selva Nair Nov. 5, 2021, 6:17 a.m. UTC | #3
Hi

On Fri, Nov 5, 2021 at 12:14 PM Arne Schwabe <arne@rfc2549.org> wrote:

> This put the early initialisation and uninitialisation that needs to
> happen between option parsing and post processing into small methods.
>
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> ---
>  src/openvpn/openvpn.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
> index 0ac961429..05b5dc2d0 100644
> --- a/src/openvpn/openvpn.c
> +++ b/src/openvpn/openvpn.c
> @@ -105,6 +105,20 @@ tunnel_point_to_point(struct context *c)
>
>  #undef PROCESS_SIGNAL_P2P
>
> +void init_early(struct context *c)
> +{
> +    net_ctx_init(c, c->net_ctx);
>

hmm... this should have been &c->net_ctx. Can this be fixed during commit?
The rest looks good.


> +
> +    /* init verbosity and mute levels */
> +    init_verb_mute(c, IVM_LEVEL_1);
> +
> +}
> +
> +static void uninit_early(struct context *c)
> +{
> +    net_ctx_free(&c->net_ctx);
> +}
> +
>
>
>  /**************************************************************************/
>  /**
> @@ -193,10 +207,9 @@ openvpn_main(int argc, char *argv[])
>              open_plugins(&c, true, OPENVPN_PLUGIN_INIT_PRE_CONFIG_PARSE);
>  #endif
>
> -            net_ctx_init(&c, &c.net_ctx);
> -
> -            /* init verbosity and mute levels */
> -            init_verb_mute(&c, IVM_LEVEL_1);
> +            /* Early initialisation that need to happen before option
> +             * post processing and other early startup but after parsing
> */
> +            init_early(&c);
>
>              /* set dev options */
>              init_options_dev(&c.options);
> @@ -308,7 +321,7 @@ openvpn_main(int argc, char *argv[])
>              env_set_destroy(c.es);
>              uninit_options(&c.options);
>              gc_reset(&c.gc);
> -            net_ctx_free(&c.net_ctx);
> +            uninit_early(&c);
>          }
>          while (c.sig->signal_received == SIGHUP);
>      }
> --
> 2.33.0
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
<div dir="ltr"><div>Hi</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Nov 5, 2021 at 12:14 PM Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org">arne@rfc2549.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This put the early initialisation and uninitialisation that needs to<br>
happen between option parsing and post processing into small methods.<br>
<br>
Signed-off-by: Arne Schwabe &lt;<a href="mailto:arne@rfc2549.org" target="_blank">arne@rfc2549.org</a>&gt;<br>
---<br>
 src/openvpn/openvpn.c | 23 ++++++++++++++++++-----<br>
 1 file changed, 18 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c<br>
index 0ac961429..05b5dc2d0 100644<br>
--- a/src/openvpn/openvpn.c<br>
+++ b/src/openvpn/openvpn.c<br>
@@ -105,6 +105,20 @@ tunnel_point_to_point(struct context *c)<br>
<br>
 #undef PROCESS_SIGNAL_P2P<br>
<br>
+void init_early(struct context *c)<br>
+{<br>
+    net_ctx_init(c, c-&gt;net_ctx);<br></blockquote><div><br></div><div>hmm... this should have been<span style="color:rgb(80,0,80)"> &amp;c-&gt;net_ctx. Can this be fixed during commit? The rest looks good.</span></div><div>  </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+    /* init verbosity and mute levels */<br>
+    init_verb_mute(c, IVM_LEVEL_1);<br>
+<br>
+}<br>
+<br>
+static void uninit_early(struct context *c)<br>
+{<br>
+    net_ctx_free(&amp;c-&gt;net_ctx);<br>
+}<br>
+<br>
<br>
 /**************************************************************************/<br>
 /**<br>
@@ -193,10 +207,9 @@ openvpn_main(int argc, char *argv[])<br>
             open_plugins(&amp;c, true, OPENVPN_PLUGIN_INIT_PRE_CONFIG_PARSE);<br>
 #endif<br>
<br>
-            net_ctx_init(&amp;c, &amp;c.net_ctx);<br>
-<br>
-            /* init verbosity and mute levels */<br>
-            init_verb_mute(&amp;c, IVM_LEVEL_1);<br>
+            /* Early initialisation that need to happen before option<br>
+             * post processing and other early startup but after parsing */<br>
+            init_early(&amp;c);<br>
<br>
             /* set dev options */<br>
             init_options_dev(&amp;c.options);<br>
@@ -308,7 +321,7 @@ openvpn_main(int argc, char *argv[])<br>
             env_set_destroy(<a href="http://c.es" rel="noreferrer" target="_blank">c.es</a>);<br>
             uninit_options(&amp;c.options);<br>
             gc_reset(&amp;c.gc);<br>
-            net_ctx_free(&amp;c.net_ctx);<br>
+            uninit_early(&amp;c);<br>
         }<br>
         while (c.sig-&gt;signal_received == SIGHUP);<br>
     }<br>
-- <br>
2.33.0<br>
<br>
<br>
<br>
_______________________________________________<br>
Openvpn-devel mailing list<br>
<a href="mailto:Openvpn-devel@lists.sourceforge.net" target="_blank">Openvpn-devel@lists.sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br>
</blockquote></div></div>

Patch

diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index 0ac961429..05b5dc2d0 100644
--- a/src/openvpn/openvpn.c
+++ b/src/openvpn/openvpn.c
@@ -105,6 +105,20 @@  tunnel_point_to_point(struct context *c)
 
 #undef PROCESS_SIGNAL_P2P
 
+void init_early(struct context *c)
+{
+    net_ctx_init(c, c->net_ctx);
+
+    /* init verbosity and mute levels */
+    init_verb_mute(c, IVM_LEVEL_1);
+
+}
+
+static void uninit_early(struct context *c)
+{
+    net_ctx_free(&c->net_ctx);
+}
+
 
 /**************************************************************************/
 /**
@@ -193,10 +207,9 @@  openvpn_main(int argc, char *argv[])
             open_plugins(&c, true, OPENVPN_PLUGIN_INIT_PRE_CONFIG_PARSE);
 #endif
 
-            net_ctx_init(&c, &c.net_ctx);
-
-            /* init verbosity and mute levels */
-            init_verb_mute(&c, IVM_LEVEL_1);
+            /* Early initialisation that need to happen before option
+             * post processing and other early startup but after parsing */
+            init_early(&c);
 
             /* set dev options */
             init_options_dev(&c.options);
@@ -308,7 +321,7 @@  openvpn_main(int argc, char *argv[])
             env_set_destroy(c.es);
             uninit_options(&c.options);
             gc_reset(&c.gc);
-            net_ctx_free(&c.net_ctx);
+            uninit_early(&c);
         }
         while (c.sig->signal_received == SIGHUP);
     }