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/+/1268?usp=email to review the following change. Change subject: dhcp: Clean up type handling of write_dhcp_* ...................................................................... dhcp: Clean up type handling of write_dhcp_* Use more appropriate types. Add casts where necessary but ensure that they are safe. Change-Id: I30a50826350ac3176443cf3bf16d3972609723a2 Signed-off-by: Frank Lichtenheld <fr...@lichtenheld.com> --- M src/openvpn/dhcp.c M src/openvpn/options.c M src/openvpn/tun.h 3 files changed, 21 insertions(+), 32 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/68/1268/1 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 f2e6dec..a0f5743 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); @@ -8002,7 +8002,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]; -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1268?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: I30a50826350ac3176443cf3bf16d3972609723a2 Gerrit-Change-Number: 1268 Gerrit-PatchSet: 1 Gerrit-Owner: flichtenheld <fr...@lichtenheld.com> Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel