diff mbox series

[Openvpn-devel] Use more C99 initialization in add_route/add_route_ipv6().

Message ID 20210727143741.29039-1-gert@greenie.muc.de
State Changes Requested
Headers
Series [Openvpn-devel] Use more C99 initialization in add_route/add_route_ipv6(). |

Commit Message

Gert Doering July 27, 2021, 4:37 a.m. UTC 
  This gets rid of a few #ifdef and also removes the need for
commit a11bea18b1c93 (argv is only initialized after the
early exit check on RT_DEFINED).

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 src/openvpn/route.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

Comments

Antonio Quartulli July 27, 2021, 10:40 p.m. UTC | #1 
Hi,

On 27/07/2021 16:37, Gert Doering wrote:
> This gets rid of a few #ifdef and also removes the need for
> commit a11bea18b1c93 (argv is only initialized after the
> early exit check on RT_DEFINED).
> 
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>  src/openvpn/route.c | 34 ++++++++++------------------------
>  1 file changed, 10 insertions(+), 24 deletions(-)
> 
> diff --git a/src/openvpn/route.c b/src/openvpn/route.c
> index e429e8c0..bec5d6ff 100644
> --- a/src/openvpn/route.c
> +++ b/src/openvpn/route.c
> @@ -1570,32 +1570,24 @@ add_route(struct route_ipv4 *r,
>            const struct env_set *es,
>            openvpn_net_ctx_t *ctx)
>  {
> -    struct gc_arena gc;
> -    struct argv argv = argv_new();
> -#if !defined(TARGET_LINUX)
> -    const char *network;
> -#if !defined(TARGET_AIX)
> -    const char *netmask;
> -#endif
> -    const char *gateway;
> -#endif
>      bool status = false;
>      int is_local_route;
>  
>      if (!(r->flags & RT_DEFINED))
>      {
> -        argv_free(&argv);
>          return;
>      }
> +    struct argv argv = argv_new();
>  

isn't it nicer to attach the two declarations and leave an empty line
between them and the upper block? :-)

> +    struct gc_arena gc;
>      gc_init(&gc);

Since you are here, why not compressing these two lines in one:

   struct gc_arena gc = gc_new();

?

>  
>  #if !defined(TARGET_LINUX)
> -    network = print_in_addr_t(r->network, 0, &gc);
> +    const char *network = print_in_addr_t(r->network, 0, &gc);
>  #if !defined(TARGET_AIX)
> -    netmask = print_in_addr_t(r->netmask, 0, &gc);
> +    const char *netmask = print_in_addr_t(r->netmask, 0, &gc);
>  #endif
> -    gateway = print_in_addr_t(r->gateway, 0, &gc);
> +    const char *gateway = print_in_addr_t(r->gateway, 0, &gc);
>  #endif
>  
>      is_local_route = local_route(r->network, r->netmask, r->gateway, rgi);
> @@ -1879,23 +1871,18 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt,
>                 openvpn_net_ctx_t *ctx)
>  {
>      struct gc_arena gc;

why not moving this gc declaration like you did above?

> -    struct argv argv = argv_new();
>  
> -    const char *network;
> -    const char *gateway;
>      bool status = false;
>      const char *device = tt->actual_name;
> -#if defined(TARGET_LINUX)
> -    int metric;
> -#endif
>      bool gateway_needed = false;
>  
>      if (!(r6->flags & RT_DEFINED) )
>      {
> -        argv_free(&argv);
>          return;
>      }
>  
> +    struct argv argv = argv_new();
> +
>  #ifndef _WIN32
>      if (r6->iface != NULL)              /* vpn server special route */
>      {
> @@ -1910,9 +1897,8 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt,
>      gc_init(&gc);
>  
>      route_ipv6_clear_host_bits(r6);
> -
> -    network = print_in6_addr( r6->network, 0, &gc);
> -    gateway = print_in6_addr( r6->gateway, 0, &gc);
> +    const char *network = print_in6_addr( r6->network, 0, &gc);
> +    const char *gateway = print_in6_addr( r6->gateway, 0, &gc);
>  
>  #if defined(TARGET_DARWIN)    \
>      || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)    \
> @@ -1963,7 +1949,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt,
>      }
>  
>  #if defined(TARGET_LINUX)
> -    metric = -1;
> +    int metric = -1;
>      if ((r6->flags & RT_METRIC_DEFINED) && (r6->metric > 0))
>      {
>          metric = r6->metric;
> 

The rest looks good!

I have compiled it on Linux and Windows, but I presume you've compiled
it on other platforms as well.

Regards,
diff mbox series

Patch

diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index e429e8c0..bec5d6ff 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1570,32 +1570,24 @@  add_route(struct route_ipv4 *r,
           const struct env_set *es,
           openvpn_net_ctx_t *ctx)
 {
-    struct gc_arena gc;
-    struct argv argv = argv_new();
-#if !defined(TARGET_LINUX)
-    const char *network;
-#if !defined(TARGET_AIX)
-    const char *netmask;
-#endif
-    const char *gateway;
-#endif
     bool status = false;
     int is_local_route;
 
     if (!(r->flags & RT_DEFINED))
     {
-        argv_free(&argv);
         return;
     }
+    struct argv argv = argv_new();
 
+    struct gc_arena gc;
     gc_init(&gc);
 
 #if !defined(TARGET_LINUX)
-    network = print_in_addr_t(r->network, 0, &gc);
+    const char *network = print_in_addr_t(r->network, 0, &gc);
 #if !defined(TARGET_AIX)
-    netmask = print_in_addr_t(r->netmask, 0, &gc);
+    const char *netmask = print_in_addr_t(r->netmask, 0, &gc);
 #endif
-    gateway = print_in_addr_t(r->gateway, 0, &gc);
+    const char *gateway = print_in_addr_t(r->gateway, 0, &gc);
 #endif
 
     is_local_route = local_route(r->network, r->netmask, r->gateway, rgi);
@@ -1879,23 +1871,18 @@  add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt,
                openvpn_net_ctx_t *ctx)
 {
     struct gc_arena gc;
-    struct argv argv = argv_new();
 
-    const char *network;
-    const char *gateway;
     bool status = false;
     const char *device = tt->actual_name;
-#if defined(TARGET_LINUX)
-    int metric;
-#endif
     bool gateway_needed = false;
 
     if (!(r6->flags & RT_DEFINED) )
     {
-        argv_free(&argv);
         return;
     }
 
+    struct argv argv = argv_new();
+
 #ifndef _WIN32
     if (r6->iface != NULL)              /* vpn server special route */
     {
@@ -1910,9 +1897,8 @@  add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt,
     gc_init(&gc);
 
     route_ipv6_clear_host_bits(r6);
-
-    network = print_in6_addr( r6->network, 0, &gc);
-    gateway = print_in6_addr( r6->gateway, 0, &gc);
+    const char *network = print_in6_addr( r6->network, 0, &gc);
+    const char *gateway = print_in6_addr( r6->gateway, 0, &gc);
 
 #if defined(TARGET_DARWIN)    \
     || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY)    \
@@ -1963,7 +1949,7 @@  add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt,
     }
 
 #if defined(TARGET_LINUX)
-    metric = -1;
+    int metric = -1;
     if ((r6->flags & RT_METRIC_DEFINED) && (r6->metric > 0))
     {
         metric = r6->metric;