[Openvpn-devel,v4] dhcp: Clean up type handling of write_dhcp_*

Message ID 20251013161759.1656-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v4] dhcp: Clean up type handling of write_dhcp_* | expand

Commit Message

Gert Doering Oct. 13, 2025, 4:17 p.m. UTC
From: Frank Lichtenheld <frank@lichtenheld.com>

Use more appropriate types. Add casts where
necessary but ensure that they are safe.

Change-Id: I30a50826350ac3176443cf3bf16d3972609723a2
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1268
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1268
This mail reflects revision 4 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Oct. 13, 2025, 4:42 p.m. UTC | #1
"Change looks good" and the new UT still passes...

And thanks for fixing the clang-format accident in N_DHCP_ADDR ;-)

Your patch has been applied to the master branch.

commit b298a7418e3776972159d84c4636c829ec6f6946
Author: Frank Lichtenheld
Date:   Mon Oct 13 18:17:53 2025 +0200

     dhcp: Clean up type handling of write_dhcp_*

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1268
     Message-Id: <20251013161759.1656-1-gert@greenie.muc.de>
     URL: https://sourceforge.net/p/openvpn/mailman/message/59246219/
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/dhcp.c b/src/openvpn/dhcp.c
index 0893ec7..a34bfca 100644
--- a/src/openvpn/dhcp.c
+++ b/src/openvpn/dhcp.c
@@ -188,18 +188,13 @@ 
 
 #if defined(_WIN32) || defined(DHCP_UNIT_TEST)
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 /*
  * Convert DHCP options from the command line / config file
  * into a raw DHCP-format options string.
  */
 
 static void
-write_dhcp_u8(struct buffer *buf, const int type, const int data, bool *error)
+write_dhcp_u8(struct buffer *buf, const uint8_t type, const uint8_t data, bool *error)
 {
     if (!buf_safe(buf, 3))
     {
@@ -213,13 +208,12 @@ 
 }
 
 static void
-write_dhcp_u32_array(struct buffer *buf, const int type, const uint32_t *data,
+write_dhcp_u32_array(struct buffer *buf, const uint8_t type, const uint32_t *data,
                      const unsigned int len, bool *error)
 {
     if (len > 0)
     {
-        int i;
-        const int size = len * sizeof(uint32_t);
+        const size_t size = len * sizeof(uint32_t);
 
         if (!buf_safe(buf, 2 + size))
         {
@@ -230,12 +224,12 @@ 
         if (size < 1 || size > 255)
         {
             *error = true;
-            msg(M_WARN, "write_dhcp_u32_array: size (%d) must be > 0 and <= 255", size);
+            msg(M_WARN, "write_dhcp_u32_array: size (%zu) must be > 0 and <= 255", size);
             return;
         }
         buf_write_u8(buf, type);
-        buf_write_u8(buf, size);
-        for (i = 0; i < len; ++i)
+        buf_write_u8(buf, (uint8_t)size);
+        for (unsigned int i = 0; i < len; ++i)
         {
             buf_write_u32(buf, data[i]);
         }
@@ -243,9 +237,9 @@ 
 }
 
 static void
-write_dhcp_str(struct buffer *buf, const int type, const char *str, bool *error)
+write_dhcp_str(struct buffer *buf, const uint8_t type, const char *str, bool *error)
 {
-    const int len = strlen(str);
+    const size_t len = strlen(str);
     if (!buf_safe(buf, 2 + len))
     {
         *error = true;
@@ -259,7 +253,7 @@ 
         return;
     }
     buf_write_u8(buf, type);
-    buf_write_u8(buf, len);
+    buf_write_u8(buf, (uint8_t)len);
     buf_write(buf, str, len);
 }
 
@@ -272,15 +266,14 @@ 
  *  0x1D  0x7 openvpn 0x3 net 0x00 0x0A duckduckgo 0x3 com 0x00
  */
 static void
-write_dhcp_search_str(struct buffer *buf, const int type, const char *const *str_array,
+write_dhcp_search_str(struct buffer *buf, const uint8_t type, const char *const *str_array,
                       int array_len, bool *error)
 {
     char tmp_buf[256];
-    int i;
-    int len = 0;
-    int label_length_pos;
+    size_t len = 0;
+    size_t label_length_pos;
 
-    for (i = 0; i < array_len; i++)
+    for (int i = 0; i < array_len; i++)
     {
         const char *ptr = str_array[i];
 
@@ -301,7 +294,8 @@ 
         {
             if (*ptr == '.' || *ptr == '\0')
             {
-                tmp_buf[label_length_pos] = (len - label_length_pos) - 1;
+                /* cast is protected by sizeof(tmp_buf) */
+                tmp_buf[label_length_pos] = (char)(len - label_length_pos - 1);
                 label_length_pos = len;
                 if (*ptr == '\0')
                 {
@@ -328,14 +322,10 @@ 
     }
 
     buf_write_u8(buf, type);
-    buf_write_u8(buf, len);
+    buf_write_u8(buf, (uint8_t)len);
     buf_write(buf, tmp_buf, len);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 bool
 build_dhcp_options_string(struct buffer *buf, const struct tuntap_options *o)
 {
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fec17b4..c0d8fe6 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1322,7 +1322,7 @@ 
     SHOW_BOOL(dhcp_pre_release);
     SHOW_STR(domain);
     SHOW_STR(netbios_scope);
-    SHOW_INT(netbios_node_type);
+    SHOW_UNSIGNED(netbios_node_type);
     SHOW_BOOL(disable_nbt);
 
     show_dhcp_option_addrs("DNS", o->dns, o->dns_len);
@@ -8001,7 +8001,7 @@ 
                 msg(msglevel, "--dhcp-option NBT: parameter (%d) must be 1, 2, 4, or 8", t);
                 goto err;
             }
-            o->netbios_node_type = t;
+            o->netbios_node_type = (uint8_t)t;
             o->dhcp_options |= DHCP_OPTIONS_DHCP_REQUIRED;
         }
         else if (streq(p[1], "WINS") && p[2] && !p[3])
diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h
index 741798d..e13f99f 100644
--- a/src/openvpn/tun.h
+++ b/src/openvpn/tun.h
@@ -104,11 +104,10 @@ 
 
     const char *netbios_scope; /* NBS (47) */
 
-    int netbios_node_type;     /* NBT 1,2,4,8 (46) */
+    uint8_t netbios_node_type; /* NBT 1,2,4,8 (46) */
 
-#define N_DHCP_ADDR                     \
-    4 /* Max # of addresses allowed for \
-       * DNS, WINS, etc. */
+/* Max # of addresses allowed for  DNS, WINS, etc. */
+#define N_DHCP_ADDR 4
 
     /* DNS (6) */
     in_addr_t dns[N_DHCP_ADDR];