diff mbox series

[Openvpn-devel] FreeBSD networking cleanup

Message ID 20220822092834.14231-2-kprovost@netgate.com
State Accepted
Headers show
Series [Openvpn-devel] FreeBSD networking cleanup | expand

Commit Message

Kristof Provost Aug. 21, 2022, 11:28 p.m. UTC 
From: Kristof Provost <kp@FreeBSD.org>

Address a few minor code review remarks:

 - use constants for the inet_ntop() buffers
 - replace argv_printf() + argv_printf_cat() with a single argv_printf()
 - net_route_v4/6 both add and remove, so adjust the error message to
   reflect that.

Signed-off-by: Kristof Provost <kprovost@netgate.com>
---
 src/openvpn/networking_freebsd.c | 34 +++++++++++++++-----------------
 1 file changed, 16 insertions(+), 18 deletions(-)

Comments

Gert Doering Sept. 12, 2022, 12:26 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks.  Tested on FreeBSD with and without DCO (the latter is more
for good measure, as the changes only influence the FreeBSD-iroute part).

Your patch has been applied to the master branch.

commit b5b132c1ba36e6d2ca261d15a9d70648890021f7
Author: Kristof Provost
Date:   Mon Aug 22 11:28:34 2022 +0200

     FreeBSD networking cleanup

     Signed-off-by: Kristof Provost <kprovost@netgate.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20220822092834.14231-2-kprovost@netgate.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25054.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/networking_freebsd.c b/src/openvpn/networking_freebsd.c
index 5fcb738e..0633dce7 100644
--- a/src/openvpn/networking_freebsd.c
+++ b/src/openvpn/networking_freebsd.c
@@ -15,7 +15,7 @@  net_route_v4(const char *op, const in_addr_t *dst, int prefixlen,
              const in_addr_t *gw, const char *iface, uint32_t table,
              int metric)
 {
-    char buf1[16], buf2[16];
+    char buf1[INET_ADDRSTRLEN], buf2[INET_ADDRSTRLEN];
     in_addr_t _dst, _gw;
     struct argv argv = argv_new();
     bool status;
@@ -23,17 +23,16 @@  net_route_v4(const char *op, const in_addr_t *dst, int prefixlen,
     _dst = ntohl(*dst);
     _gw = ntohl(*gw);
 
-    argv_printf(&argv, "%s %s",
-                ROUTE_PATH, op);
-    argv_printf_cat(&argv, "-net %s/%d %s -fib %d",
-                    inet_ntop(AF_INET, &_dst, buf1, sizeof(buf1)),
-                    prefixlen,
-                    inet_ntop(AF_INET, &_gw, buf2, sizeof(buf2)),
-                    table);
+    argv_printf(&argv, "%s %s -net %s/%d %s -fib %d",
+                ROUTE_PATH, op,
+                inet_ntop(AF_INET, &_dst, buf1, sizeof(buf1)),
+                prefixlen,
+                inet_ntop(AF_INET, &_gw, buf2, sizeof(buf2)),
+                table);
 
     argv_msg(M_INFO, &argv);
     status = openvpn_execve_check(&argv, NULL, 0,
-                                  "ERROR: FreeBSD route add command failed");
+                                  "ERROR: FreeBSD route command failed");
 
     argv_free(&argv);
 
@@ -45,21 +44,20 @@  net_route_v6(const char *op, const struct in6_addr *dst,
              int prefixlen, const struct in6_addr *gw, const char *iface,
              uint32_t table, int metric)
 {
-    char buf1[64], buf2[64];
+    char buf1[INET6_ADDRSTRLEN], buf2[INET6_ADDRSTRLEN];
     struct argv argv = argv_new();
     bool status;
 
-    argv_printf(&argv, "%s -6 %s",
-                ROUTE_PATH, op);
-    argv_printf_cat(&argv, "-net %s/%d %s -fib %d",
-                    inet_ntop(AF_INET6, dst, buf1, sizeof(buf1)),
-                    prefixlen,
-                    inet_ntop(AF_INET6, gw, buf2, sizeof(buf2)),
-                    table);
+    argv_printf(&argv, "%s -6 %s -net %s/%d %s -fib %d",
+                ROUTE_PATH, op,
+                inet_ntop(AF_INET6, dst, buf1, sizeof(buf1)),
+                prefixlen,
+                inet_ntop(AF_INET6, gw, buf2, sizeof(buf2)),
+                table);
 
     argv_msg(M_INFO, &argv);
     status = openvpn_execve_check(&argv, NULL, 0,
-                                  "ERROR: FreeBSD route add command failed");
+                                  "ERROR: FreeBSD route command failed");
 
     argv_free(&argv);