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 <g...@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,
--
Antonio Quartulli
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
