Replace buffer backed strings for management_android_control with simple stack variables
PATCH 1/1 Replace buffer backed strings for management_android_control with simple stack variables
This simplifies the code a bit and also silences compiler warnings about uint8_t pointers passed to char pointers without cast
--- src/openvpn/route.c | 14 +++++++------- src/openvpn/tun.c | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-)
src/openvpn/route.c +7 -7
@@ -1610,17 +1610,17 @@ add_route(struct route_ipv4 *r, status =openvpn_execve_check (&argv, es,0 ,"ERROR: Linux route add command failed" );#elif defined (TARGET_ANDROID ) -struct buffer out =alloc_buf_gc (128 , &gc); +char out[128 ];if (rgi) { -buf_printf (&out,"%s %s %s dev %s" , network, netmask, gateway, rgi->iface); +snprintf (out,sizeof (out),"%s %s %s dev %s" , network, netmask, gateway, rgi->iface); }else { -buf_printf (&out,"%s %s %s" , network, netmask, gateway); +snprintf (out,sizeof (out),"%s %s %s" , network, netmask, gateway); } -management_android_control (management,"ROUTE" ,buf_bptr (&out)); +management_android_control (management,"ROUTE" , out);#elif defined (_WIN32) {
@@ -1963,11 +1963,11 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag status =openvpn_execve_check (&argv, es,0 ,"ERROR: Linux route -6/-A inet6 add command failed" );#elif defined (TARGET_ANDROID ) -struct buffer out =alloc_buf_gc (64 , &gc); +char out[64 ]; -buf_printf (&out,"%s/%d %s" , network, r6->netbits, device); +snprintf (out,sizeof (out),"%s/%d %s" , network, r6->netbits, device); -management_android_control (management,"ROUTE6" ,buf_bptr (&out)); +management_android_control (management,"ROUTE6" , out);#elif defined (_WIN32)
src/openvpn/tun.c +6 -6
@@ -1031,12 +1031,12 @@ do_ifconfig(struct tuntap *tt,if (do_ipv6) { -struct buffer out6 =alloc_buf_gc (64 , &gc); -buf_printf (&out6,"%s/%d" , ifconfig_ipv6_local,tt->netbits_ipv6); -management_android_control (management,"IFCONFIG6" ,buf_bptr (&out6)); +char out6[64 ]; +snprintf (out6,sizeof (out6),"%s/%d" , ifconfig_ipv6_local,tt->netbits_ipv6); +management_android_control (management,"IFCONFIG6" , out6); } -struct buffer out =alloc_buf_gc (64 , &gc); +char out[64 ];char *top; switch (tt->topology)
@@ -1057,8 +1057,8 @@ do_ifconfig(struct tuntap *tt, top = "undef"; } -buf_printf (&out,"%s %s %d %s" , ifconfig_local, ifconfig_remote_netmask, tun_mtu, top); -management_android_control (management,"IFCONFIG" ,buf_bptr (&out)); +snprintf (out,sizeof (out),"%s %s %d %s" , ifconfig_local, ifconfig_remote_netmask, tun_mtu, top); +management_android_control (management,"IFCONFIG" , out);#elif defined(TARGET_SOLARIS)/* Solaris 2.6 (and 7?) cannot set all parameters in one go...
Signature · 1 line
2.15.1
Hi,
The commit message summary and description lines are slightly long. Try to not exceed 72 chars line length.
Quoted 11 lines · Arne Schwabe
This simplifies the code a bit and also silences compiler warnings about
uint8_tpointers passed to char pointers without cast--- src/openvpn/route.c | 14 +++++++------- src/openvpn/tun.c | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-)src/openvpn/route.c +0
-0
src/openvpn/route.c add_route(struct route_ipv4 *r, · line 1617
Quoted 10 lines
src/openvpn/route.c +2
-2@@ -1610,17 +1610,17 @@ add_route(struct route_ipv4 *r, status =openvpn_execve_check (&argv, es,0 ,"ERROR: Linux route add command failed" );#elif defined (TARGET_ANDROID ) -struct buffer out =alloc_buf_gc (128 , &gc); +char out[128 ];if (rgi) { -buf_printf (&out,"%s %s %s dev %s" , network, netmask, gateway, rgi->iface); +snprintf (out,sizeof (out),"%s %s %s dev %s" , network, netmask, gateway, rgi->iface);
We have an openvpn_snprintf to account for "certain bugger snprintf
implementations that do not null-terminate" (see buffer.c). I have no
idea what implementation that actually are (or whether it's still valid at all), but let's use openvpn_snprintf() to be sure until we know :)
Quoted 49 lines
src/openvpn/route.c +5
-5}else { -buf_printf (&out,"%s %s %s" , network, netmask, gateway); +snprintf (out,sizeof (out),"%s %s %s" , network, netmask, gateway); } -management_android_control (management,"ROUTE" ,buf_bptr (&out)); +management_android_control (management,"ROUTE" , out);#elif defined (_WIN32) {@@ -1963,11 +1963,11 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag status =openvpn_execve_check (&argv, es,0 ,"ERROR: Linux route -6/-A inet6 add command failed" );#elif defined (TARGET_ANDROID ) -struct buffer out =alloc_buf_gc (64 , &gc); +char out[64 ]; -buf_printf (&out,"%s/%d %s" , network, r6->netbits, device); +snprintf (out,sizeof (out),"%s/%d %s" , network, r6->netbits, device); -management_android_control (management,"ROUTE6" ,buf_bptr (&out)); +management_android_control (management,"ROUTE6" , out);#elif defined (_WIN32)src/openvpn/tun.c +4
-4@@ -1031,12 +1031,12 @@ do_ifconfig(struct tuntap *tt,if (do_ipv6) { -struct buffer out6 =alloc_buf_gc (64 , &gc); -buf_printf (&out6,"%s/%d" , ifconfig_ipv6_local,tt->netbits_ipv6); -management_android_control (management,"IFCONFIG6" ,buf_bptr (&out6)); +char out6[64 ]; +snprintf (out6,sizeof (out6),"%s/%d" , ifconfig_ipv6_local,tt->netbits_ipv6); +management_android_control (management,"IFCONFIG6" , out6); } -struct buffer out =alloc_buf_gc (64 , &gc); +char out[64 ];char *top; switch (tt->topology)
src/openvpn/tun.c do_ifconfig(struct tuntap *tt, · line 1064
Quoted 10 lines
src/openvpn/tun.c +2
-2@@ -1057,8 +1057,8 @@ do_ifconfig(struct tuntap *tt, top = "undef"; } -buf_printf (&out,"%s %s %d %s" , ifconfig_local, ifconfig_remote_netmask, tun_mtu, top); -management_android_control (management,"IFCONFIG" ,buf_bptr (&out)); +snprintf (out,sizeof (out),"%s %s %d %s" , ifconfig_local, ifconfig_remote_netmask, tun_mtu, top); +management_android_control (management,"IFCONFIG" , out);#elif defined(TARGET_SOLARIS)/* Solaris 2.6 (and 7?) cannot set all parameters in one go...
Otherwise this looks good.
-Steffan
PATCH v2 1/1 Replace buffer backed strings for management_android_control with simple stack variables
This simplifies the code a bit and also silences compiler warnings about uint8_t pointers passed to char pointers without cast
Patch V2: Use openvpn_snprintf instead snprintf
--- src/openvpn/route.c | 14 +++++++------- src/openvpn/tun.c | 12 ++++++------ 2 files changed, 13 insertions(+), 13 deletions(-)
src/openvpn/route.c +7 -7
@@ -1610,17 +1610,17 @@ add_route(struct route_ipv4 *r, status =openvpn_execve_check (&argv, es,0 ,"ERROR: Linux route add command failed" );#elif defined (TARGET_ANDROID ) -struct buffer out =alloc_buf_gc (128 , &gc); +char out[128 ];if (rgi) { -buf_printf (&out,"%s %s %s dev %s" , network, netmask, gateway, rgi->iface); +openvpn_snprintf (out,sizeof (out),"%s %s %s dev %s" , network, netmask, gateway, rgi->iface); }else { -buf_printf (&out,"%s %s %s" , network, netmask, gateway); +openvpn_snprintf (out,sizeof (out),"%s %s %s" , network, netmask, gateway); } -management_android_control (management,"ROUTE" ,buf_bptr (&out)); +management_android_control (management,"ROUTE" , out);#elif defined (_WIN32) {
@@ -1963,11 +1963,11 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag status =openvpn_execve_check (&argv, es,0 ,"ERROR: Linux route -6/-A inet6 add command failed" );#elif defined (TARGET_ANDROID ) -struct buffer out =alloc_buf_gc (64 , &gc); +char out[64 ]; -buf_printf (&out,"%s/%d %s" , network, r6->netbits, device); +openvpn_snprintf (out,sizeof (out),"%s/%d %s" , network, r6->netbits, device); -management_android_control (management,"ROUTE6" ,buf_bptr (&out)); +management_android_control (management,"ROUTE6" , out);#elif defined (_WIN32)
src/openvpn/tun.c +6 -6
@@ -1031,12 +1031,12 @@ do_ifconfig(struct tuntap *tt,if (do_ipv6) { -struct buffer out6 =alloc_buf_gc (64 , &gc); -buf_printf (&out6,"%s/%d" , ifconfig_ipv6_local,tt->netbits_ipv6); -management_android_control (management,"IFCONFIG6" ,buf_bptr (&out6)); +char out6[64 ]; +openvpn_snprintf (out6,sizeof (out6),"%s/%d" , ifconfig_ipv6_local,tt->netbits_ipv6); +management_android_control (management,"IFCONFIG6" , out6); } -struct buffer out =alloc_buf_gc (64 , &gc); +char out[64 ];char *top; switch (tt->topology)
@@ -1057,8 +1057,8 @@ do_ifconfig(struct tuntap *tt, top = "undef"; } -buf_printf (&out,"%s %s %d %s" , ifconfig_local, ifconfig_remote_netmask, tun_mtu, top); -management_android_control (management,"IFCONFIG" ,buf_bptr (&out)); +openvpn_snprintf (out,sizeof (out),"%s %s %d %s" , ifconfig_local, ifconfig_remote_netmask, tun_mtu, top); +management_android_control (management,"IFCONFIG" , out);#elif defined(TARGET_SOLARIS)/* Solaris 2.6 (and 7?) cannot set all parameters in one go...
Signature · 1 line
2.15.1
Acked-by Gert Doering <[email protected]...>
Patch looks reasonable, and is inside TARGET_ANDROID anyway, so if you break it, it's your bug reports...
Your patch has been applied to the master branch.
commit 8fcc63b2e7d24d2fbf6d7ab10767c2347c723d31
Author: Arne Schwabe Date: Wed Jan 17 11:38:36 2018 +0100
Replace buffer backed strings for management_android_control with simple stack variables
Acked-by: Gert Doering <[email protected]...>
Message-Id: <[email protected]...>
URL: https://www.mail-archive.com/search?l=mid&[email protected]...
Signed-off-by: Gert Doering <[email protected]...>--
kind regards,
Gert Doering
Hi,
src/openvpn/route.c add_route(struct route_ipv4 *r, · line 1617
Quoted 4 lines · Arne Schwabe
if (rgi) {src/openvpn/route.c +1
-1-buf_printf (&out,"%s %s %s dev %s" , network, netmask, gateway, rgi->iface); +snprintf (out,sizeof (out),"%s %s %s dev %s" , network, netmask, gateway, rgi->iface);
Everything else uses openvpn_snprintf(), which presumably works around snprintf() platform undefined'ness wrt null-termination on too-long input.
Most likely this is not *needed* on TARGET_ANDROID code, but for clarity it might be worth considering.
gert
Signature · 3 lines
now what should I write here... Gert Doering - Munich, Germany [email protected]...