Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1520?usp=email

to review the following change.


Change subject: DNS/DHCP: Make _len attributes consistently unsigned
......................................................................

DNS/DHCP: Make _len attributes consistently unsigned

Use unsigned int instead of size_t since 32bit
values are quite enough here and this avoids some
unneccesary casts.

Change-Id: Id2c5df9df32a02e944e13b55f57a2c1928b652f4
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M include/openvpn-msg.h
M src/openvpn/dns.h
M src/openvpn/options.c
M src/openvpn/tun.c
M src/openvpn/tun.h
M src/openvpnserv/interactive.c
6 files changed, 62 insertions(+), 78 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1520/1

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 34f50c8..bef20cf 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -103,7 +103,7 @@
     interface_t iface;
     char domains[512];
     short family;
-    int addr_len;
+    unsigned int addr_len;
     inet_address_t addr[4]; /* support up to 4 dns addresses */
 } dns_cfg_message_t;

@@ -130,7 +130,7 @@
 {
     message_header_t header;
     interface_t iface;
-    int addr_len;
+    unsigned int addr_len;
     inet_address_t addr[4]; /* support up to 4 dns addresses */
 } wins_cfg_message_t;

diff --git a/src/openvpn/dns.h b/src/openvpn/dns.h
index 882e474..51bc2de 100644
--- a/src/openvpn/dns.h
+++ b/src/openvpn/dns.h
@@ -99,14 +99,14 @@
 struct dhcp_options
 {
     in_addr_t dns[N_DHCP_ADDR];
-    int dns_len;
+    unsigned int dns_len;

     struct in6_addr dns6[N_DHCP_ADDR];
-    int dns6_len;
+    unsigned int dns6_len;
 
     const char *domain;
     const char *domain_search_list[N_SEARCH_LIST_LEN];
-    int domain_search_list_len;
+    unsigned int domain_search_list_len;
 };

 struct dns_options
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6bebeb7..ddd1165 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1271,23 +1271,21 @@
 #ifndef ENABLE_SMALL

 static void
-show_dhcp_option_list(const char *name, const char *const *array, int len)
+show_dhcp_option_list(const char *name, const char *const *array, unsigned int 
len)
 {
-    int i;
-    for (i = 0; i < len; ++i)
+    for (unsigned int i = 0; i < len; ++i)
     {
-        msg(D_SHOW_PARMS, "  %s[%d] = %s", name, i, array[i]);
+        msg(D_SHOW_PARMS, "  %s[%u] = %s", name, i, array[i]);
     }
 }

 static void
-show_dhcp_option_addrs(const char *name, const in_addr_t *array, int len)
+show_dhcp_option_addrs(const char *name, const in_addr_t *array, unsigned int 
len)
 {
     struct gc_arena gc = gc_new();
-    int i;
-    for (i = 0; i < len; ++i)
+    for (unsigned int i = 0; i < len; ++i)
     {
-        msg(D_SHOW_PARMS, "  %s[%d] = %s", name, i, print_in_addr_t(array[i], 
0, &gc));
+        msg(D_SHOW_PARMS, "  %s[%u] = %s", name, i, print_in_addr_t(array[i], 
0, &gc));
     }
     gc_free(&gc);
 }
@@ -1319,12 +1317,12 @@
 #endif /* ifdef _WIN32 */

 static void
-dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, int *len, 
msglvl_t msglevel)
+dhcp_option_dns6_parse(const char *parm, struct in6_addr *dns6_list, unsigned 
int *len, msglvl_t msglevel)
 {
     struct in6_addr addr;
     if (*len >= N_DHCP_ADDR)
     {
-        msg(msglevel, "--dhcp-option DNS: maximum of %d IPv6 dns servers can 
be specified",
+        msg(msglevel, "--dhcp-option DNS: maximum of %u IPv6 dns servers can 
be specified",
             N_DHCP_ADDR);
     }
     else if (get_ipv6_addr(parm, &addr, NULL, msglevel))
@@ -1333,12 +1331,12 @@
     }
 }
 static void
-dhcp_option_address_parse(const char *name, const char *parm, in_addr_t 
*array, int *len,
+dhcp_option_address_parse(const char *name, const char *parm, in_addr_t 
*array, unsigned int *len,
                           msglvl_t msglevel)
 {
     if (*len >= N_DHCP_ADDR)
     {
-        msg(msglevel, "--dhcp-option %s: maximum of %d %s servers can be 
specified", name,
+        msg(msglevel, "--dhcp-option %s: maximum of %u %s servers can be 
specified", name,
             N_DHCP_ADDR, name);
     }
     else
@@ -3664,7 +3662,7 @@
             new->name = dhcp->domain;
             entry = &new->next;

-            for (int i = 0; i < dhcp->domain_search_list_len; ++i)
+            for (unsigned int i = 0; i < dhcp->domain_search_list_len; ++i)
             {
                 ALLOC_OBJ_CLEAR_GC(*entry, struct dns_domain, &dns->gc);
                 struct dns_domain *new = *entry;
@@ -3674,13 +3672,13 @@

             struct dns_server *server = dns_server_get(&dns->servers, 0, 
&dns->gc);
             const size_t max_addrs = SIZE(server->addr);
-            for (int i = 0; i < dhcp->dns_len && server->addr_count < 
max_addrs; ++i)
+            for (unsigned int i = 0; i < dhcp->dns_len && server->addr_count < 
max_addrs; ++i)
             {
                 server->addr[server->addr_count].in.a4.s_addr = 
htonl(dhcp->dns[i]);
                 server->addr[server->addr_count].family = AF_INET;
                 server->addr_count += 1;
             }
-            for (int i = 0; i < dhcp->dns6_len && server->addr_count < 
max_addrs; ++i)
+            for (unsigned int i = 0; i < dhcp->dns6_len && server->addr_count 
< max_addrs; ++i)
             {
                 server->addr[server->addr_count].in.a6 = dhcp->dns6[i];
                 server->addr[server->addr_count].family = AF_INET6;
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 87b6569..1fa9ab0 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -103,7 +103,7 @@

 static void windows_set_mtu(const int iface_index, const short family, const 
int mtu);

-static void netsh_set_dns6_servers(const struct in6_addr *addr_list, const int 
addr_len,
+static void netsh_set_dns6_servers(const struct in6_addr *addr_list, const 
unsigned int addr_len,
                                    DWORD adapter_index);

 static void netsh_command(const struct argv *a, int n, msglvl_t msglevel);
@@ -112,11 +112,6 @@

 static const char *netsh_get_id(const char *dev_node, struct gc_arena *gc);

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-
 static bool
 do_address_service(const bool add, const short family, const struct tuntap *tt)
 {
@@ -247,8 +242,8 @@
     ack_message_t ack;
     struct gc_arena gc = gc_new();
     HANDLE pipe = tt->options.msg_channel;
-    int len = family == AF_INET6 ? tt->options.dns6_len : tt->options.dns_len;
-    int addr_len = add ? len : 0;
+    unsigned int len = family == AF_INET6 ? tt->options.dns6_len : 
tt->options.dns_len;
+    unsigned int addr_len = add ? len : 0;
     const char *ip_proto_name = family == AF_INET6 ? "IPv6" : "IPv4";

     if (len == 0)
@@ -273,11 +268,11 @@
     {
         addr_len = _countof(dns.addr);
         dns.addr_len = addr_len;
-        msg(M_WARN, "Number of %s DNS addresses sent to service truncated to 
%d", ip_proto_name,
-            addr_len);
+        msg(M_WARN, "Number of %s DNS addresses sent to service truncated to 
%u",
+            ip_proto_name, addr_len);
     }

-    for (int i = 0; i < addr_len; ++i)
+    for (unsigned int i = 0; i < addr_len; ++i)
     {
         if (family == AF_INET6)
         {
@@ -317,7 +312,7 @@
     ack_message_t ack;
     struct gc_arena gc = gc_new();
     HANDLE pipe = tt->options.msg_channel;
-    int addr_len = add ? tt->options.wins_len : 0;
+    unsigned int addr_len = add ? tt->options.wins_len : 0;

     if (tt->options.wins_len == 0)
     {
@@ -338,10 +333,10 @@
     {
         addr_len = _countof(wins.addr);
         wins.addr_len = addr_len;
-        msg(M_WARN, "Number of WINS addresses sent to service truncated to 
%d", addr_len);
+        msg(M_WARN, "Number of WINS addresses sent to service truncated to 
%u", addr_len);
     }

-    for (int i = 0; i < addr_len; ++i)
+    for (unsigned int i = 0; i < addr_len; ++i)
     {
         wins.addr[i].ipv4.s_addr = htonl(tt->options.wins[i]);
     }
@@ -368,10 +363,6 @@
     gc_free(&gc);
 }

-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 static bool
 do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
 {
@@ -1998,13 +1989,13 @@

     /* Prefer IPv6 DNS servers,
      * Android will use the DNS server in the order we specify*/
-    for (int i = 0; i < tt->options.dns6_len; i++)
+    for (unsigned int i = 0; i < tt->options.dns6_len; i++)
     {
         management_android_control(management, "DNS6SERVER",
                                    print_in6_addr(tt->options.dns6[i], 0, 
&gc));
     }

-    for (int i = 0; i < tt->options.dns_len; i++)
+    for (unsigned int i = 0; i < tt->options.dns_len; i++)
     {
         management_android_control(management, "DNSSERVER",
                                    print_in_addr_t(tt->options.dns[i], 0, 
&gc));
@@ -4922,10 +4913,10 @@
     msg(D_TUNTAP_INFO, "End ipconfig commands for register-dns...");
 }

-void
-ip_addr_string_to_array(in_addr_t *dest, int *dest_len, const IP_ADDR_STRING 
*src)
+static void
+ip_addr_string_to_array(in_addr_t *dest, unsigned int *dest_len, const 
IP_ADDR_STRING *src)
 {
-    int i = 0;
+    unsigned int i = 0;
     while (src)
     {
         const unsigned int getaddr_flags = GETADDR_HOST_ORDER;
@@ -4967,11 +4958,11 @@
 }

 static bool
-ip_addr_one_to_one(const in_addr_t *a1, const int a1len, const IP_ADDR_STRING 
*ias)
+ip_addr_one_to_one(const in_addr_t *a1, const unsigned int a1len, const 
IP_ADDR_STRING *ias)
 {
-    in_addr_t a2[8];
-    int a2len = SIZE(a2);
-    int i;
+#define MAX_ADDRS 8
+    in_addr_t a2[MAX_ADDRS];
+    unsigned int a2len = MAX_ADDRS;

     ip_addr_string_to_array(a2, &a2len, ias);
     /*msg (M_INFO, "a1len=%d a2len=%d", a1len, a2len);*/
@@ -4980,7 +4971,7 @@
         return false;
     }

-    for (i = 0; i < a1len; ++i)
+    for (unsigned int i = 0; i < a1len; ++i)
     {
         if (a1[i] != a2[i])
         {
@@ -4993,12 +4984,11 @@
 static bool
 ip_addr_member_of(const in_addr_t addr, const IP_ADDR_STRING *ias)
 {
-    in_addr_t aa[8];
-    int len = SIZE(aa);
-    int i;
+    in_addr_t aa[MAX_ADDRS];
+    unsigned int len = MAX_ADDRS;

     ip_addr_string_to_array(aa, &len, ias);
-    for (i = 0; i < len; ++i)
+    for (unsigned int i = 0; i < len; ++i)
     {
         if (addr == aa[i])
         {
@@ -5007,6 +4997,7 @@
     }
     return false;
 }
+#undef MAX_ADDRS

 /**
  * Set the ipv6 dns servers on the specified interface.
@@ -5014,7 +5005,7 @@
  * are cleared first.
  */
 static void
-netsh_set_dns6_servers(const struct in6_addr *addr_list, const int addr_len, 
DWORD adapter_index)
+netsh_set_dns6_servers(const struct in6_addr *addr_list, const unsigned int 
addr_len, DWORD adapter_index)
 {
     struct gc_arena gc = gc_new();
     struct argv argv = argv_new();
@@ -5024,7 +5015,7 @@
                 NETSH_PATH_SUFFIX, adapter_index);
     netsh_command(&argv, 2, M_FATAL);

-    for (int i = 0; i < addr_len; ++i)
+    for (unsigned int i = 0; i < addr_len; ++i)
     {
         const char *fmt = (i == 0) ? "%s%s interface ipv6 set dns %lu static 
%s"
                                    : "%s%s interface ipv6 add dns %lu %s";
@@ -5043,7 +5034,7 @@
 }

 static void
-netsh_ifconfig_options(const char *type, const in_addr_t *addr_list, const int 
addr_len,
+netsh_ifconfig_options(const char *type, const in_addr_t *addr_list, const 
unsigned int addr_len,
                        const IP_ADDR_STRING *current, DWORD adapter_index, 
const bool test_first)
 {
     struct gc_arena gc = gc_new();
@@ -5074,14 +5065,13 @@

     /* add new DNS/WINS settings to TAP interface */
     {
-        int count = 0;
-        int i;
-        for (i = 0; i < addr_len; ++i)
+        bool first = true;
+        for (unsigned int i = 0; i < addr_len; ++i)
         {
             if (delete_first || !test_first || 
!ip_addr_member_of(addr_list[i], current))
             {
-                const char *fmt = count ? "%s%s interface ip add %s %lu %s"
-                                        : "%s%s interface ip set %s %lu static 
%s";
+                const char *fmt = first ? "%s%s interface ip set %s %lu static 
%s"
+                                        : "%s%s interface ip add %s %lu %s";

                 argv_printf(&argv, fmt, get_win_sys_path(), NETSH_PATH_SUFFIX, 
type, adapter_index,
                             print_in_addr_t(addr_list[i], 0, &gc));
@@ -5094,7 +5084,7 @@

                 netsh_command(&argv, 2, M_FATAL);

-                ++count;
+                first = false;
             }
             else
             {
@@ -6094,7 +6084,7 @@
     struct argv argv = argv_new();

     /* delete ipvX dns servers if any were set */
-    int len = ipv6 ? tt->options.dns6_len : tt->options.dns_len;
+    unsigned int len = ipv6 ? tt->options.dns6_len : tt->options.dns_len;
     if (len > 0)
     {
         argv_printf(&argv, "%s%s interface %s delete dns %lu all", 
get_win_sys_path(),
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 388f5b7..962a079 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -109,25 +109,25 @@

     /* DNS (6) */
     in_addr_t dns[N_DHCP_ADDR];
-    int dns_len;
+    unsigned int dns_len;

     /* WINS (44) */
     in_addr_t wins[N_DHCP_ADDR];
-    int wins_len;
+    unsigned int wins_len;

     /* NTP (42) */
     in_addr_t ntp[N_DHCP_ADDR];
-    int ntp_len;
+    unsigned int ntp_len;

     /* NBDD (45) */
     in_addr_t nbdd[N_DHCP_ADDR];
-    int nbdd_len;
+    unsigned int nbdd_len;

 #define N_SEARCH_LIST_LEN 10 /* Max # of entries in domin-search list */

     /* SEARCH (119), MacOS, Linux, Win10 1809+ */
     const char *domain_search_list[N_SEARCH_LIST_LEN];
-    int domain_search_list_len;
+    unsigned int domain_search_list_len;

     /* DISABLE_NBT (43, Vendor option 001) */
     bool disable_nbt;
@@ -138,7 +138,7 @@
     bool register_dns;

     struct in6_addr dns6[N_DHCP_ADDR];
-    int dns6_len;
+    unsigned int dns6_len;
 #if defined(TARGET_ANDROID)
     const char *http_proxy;
     int http_proxy_port;
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index b50aeb3..c96d3f2 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1814,10 +1814,10 @@
 {
     DWORD err = 0;
     undo_type_t undo_type = (msg->family == AF_INET6) ? undo_dns6 : undo_dns4;
-    int addr_len = msg->addr_len;
+    unsigned int addr_len = msg->addr_len;

     /* sanity check */
-    const int max_addrs = _countof(msg->addr);
+    const unsigned int max_addrs = _countof(msg->addr);
     if (addr_len > max_addrs)
     {
         addr_len = max_addrs;
@@ -1875,7 +1875,7 @@
          * time constant by all compilers and constexpr is C23 */
         CHAR addrs[_countof(msg->addr) * 64]; /* 64 is enough for one IPv4/6 
address */
         size_t offset = 0;
-        for (int i = 0; i < addr_len; ++i)
+        for (unsigned int i = 0; i < addr_len; ++i)
         {
             if (i != 0)
             {
@@ -2936,14 +2936,10 @@
 {
     DWORD err = NO_ERROR;
     wchar_t addr[16]; /* large enough to hold string representation of an ipv4 
*/
-    int addr_len = msg->addr_len;
+    unsigned int addr_len = msg->addr_len;

     /* sanity check */
-    if (addr_len < 0)
-    {
-        addr_len = 0;
-    }
-    if ((unsigned int)addr_len > _countof(msg->addr))
+    if (addr_len > _countof(msg->addr))
     {
         addr_len = _countof(msg->addr);
     }
@@ -2971,7 +2967,7 @@
         goto out; /* job done */
     }

-    for (int i = 0; i < addr_len; ++i)
+    for (unsigned int i = 0; i < addr_len; ++i)
     {
         RtlIpv4AddressToStringW(&msg->addr[i].ipv4, addr);
         err = netsh_wins_cmd(i == 0 ? L"set" : L"add", msg->iface.index, addr);

--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1520?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id2c5df9df32a02e944e13b55f57a2c1928b652f4
Gerrit-Change-Number: 1520
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to