[Openvpn-devel,v3,09/21] Refactor early initialisation and uninitialisation into methods

Message ID 20211019183127.614175-10-arne@rfc2549.org
State Superseded
Headers show
Series OpenSSL 3.0 improvements for OpenVPN | expand

Commit Message

Arne Schwabe Oct. 19, 2021, 7:31 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

Selva Nair Oct. 23, 2021, 6:23 a.m. UTC | #1
Hi,

On Tue, Oct 19, 2021 at 2:32 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..f8e94509f 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);
>

&c->net_ctx would be more readable and less mysterious.


> +
> +    /* 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);
>

&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);
>      }


Looks good otherwise. The nits could be fixed at commit time?

Selva
<div dir="ltr"><div dir="ltr">Hi,</div><div class="gmail_quote"><div class="gmail_attr"><br></div><div dir="ltr" class="gmail_attr">On Tue, Oct 19, 2021 at 2:32 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..f8e94509f 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, &amp;(*c).net_ctx);<br></blockquote><div><br></div><div>&amp;c-&gt;net_ctx would be more readable and less mysterious.</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)</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+{<br>
+    net_ctx_free(&amp;(*c).net_ctx);<br></blockquote><div><br></div><div>&amp;c-&gt;net_ctx</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>
+<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); </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
         }<br>
         while (c.sig-&gt;signal_received == SIGHUP);<br>
     }</blockquote><div> </div><div>Looks good otherwise. The nits could be fixed at commit time?</div><div><br></div><div>Selva</div></div></div>
Maximilian Fillinger Oct. 25, 2021, 12:52 a.m. UTC | #2
On 19/10/2021 20:31, Arne Schwabe 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>

Acked-by: Max Fillinger <maximilian.fillinger@foxcrypto.com>

It's easy to see that this does not change the behavior. Compiled and 
tested with OpenSSL 3 only because it doesn't actually touch any OpenSSL 
code.

Patch

diff --git a/src/openvpn/openvpn.c b/src/openvpn/openvpn.c
index 0ac961429..f8e94509f 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);
     }