From 54294e8c28de82d8b5587957e53d9febdb343fe5 Mon Sep 17 00:00:00 2001 From: Matt Kimball Date: Fri, 30 Dec 2016 16:32:45 +0000 Subject: [PATCH] build: fix compiler warnings when for OpenBSD, NetBSD and Solaris When building for NetBSD, the compiler was warning about index into an array with a character when using isspace() and similar macros. IPPROTO_SCTP is also not defined by NetBSD, so there were warnings about unused SCTP related variables. OpenBSD complains about using sprintf rather than snprintf. It's a good idea to use snprintf, anyway, to avoid buffer overruns, though I believe in these particular cases sprintf was safe. Nevertheless, snprintf is now used instead. Solaris requires strings.h to find index(). Solaris complaints about a missing sentinel pointer unless the terminating NULL in execl is cast to a "char *". --- packet/cmdparse.c | 4 ++-- packet/command.c | 1 + packet/deconstruct_unix.c | 16 ++++++++++------ packet/probe.c | 3 ++- packet/probe_unix.c | 2 +- packet/wait_unix.c | 1 + ui/asn.c | 11 +++++++---- ui/cmdpipe.c | 5 +++-- ui/curses.c | 19 +++++++++++-------- ui/dns.c | 5 ++++- ui/split.c | 4 ++-- ui/utils.c | 4 ++-- 12 files changed, 46 insertions(+), 29 deletions(-) diff --git a/packet/cmdparse.c b/packet/cmdparse.c index 1ce250a..6c1d16a 100644 --- a/packet/cmdparse.c +++ b/packet/cmdparse.c @@ -41,7 +41,7 @@ int tokenize_command( for (i = 0; command_string[i]; i++) { if (on_space) { - if (!isspace(command_string[i])) { + if (!isspace((unsigned char)command_string[i])) { /* Take care not to exceed the token array length */ if (token_count >= max_tokens) { return -1; @@ -51,7 +51,7 @@ int tokenize_command( on_space = 0; } } else { - if (isspace(command_string[i])) { + if (isspace((unsigned char)command_string[i])) { command_string[i] = 0; on_space = 1; } diff --git a/packet/command.c b/packet/command.c index 54c3a48..2fa9331 100644 --- a/packet/command.c +++ b/packet/command.c @@ -24,6 +24,7 @@ #include #include #include +#include #include "cmdparse.h" #include "platform.h" diff --git a/packet/deconstruct_unix.c b/packet/deconstruct_unix.c index 5fa169e..bdb7dee 100644 --- a/packet/deconstruct_unix.c +++ b/packet/deconstruct_unix.c @@ -110,13 +110,15 @@ void handle_inner_ip4_packet( sizeof(struct IPHeader) + sizeof(struct UDPHeader); const int ip_tcp_size = sizeof(struct IPHeader) + sizeof(struct TCPHeader); - const int ip_sctp_size = - sizeof(struct IPHeader) + sizeof(struct SCTPHeader); const struct ICMPHeader *icmp; const struct UDPHeader *udp; const struct TCPHeader *tcp; - const struct SCTPHeader *sctp; int udp_length; +#ifdef IPPROTO_SCTP + const int ip_sctp_size = + sizeof(struct IPHeader) + sizeof(struct SCTPHeader); + const struct SCTPHeader *sctp; +#endif if (ip->protocol == IPPROTO_ICMP) { if (packet_length < ip_icmp_size) { @@ -185,13 +187,15 @@ void handle_inner_ip6_packet( sizeof(struct IP6Header) + sizeof(struct UDPHeader); const int ip_tcp_size = sizeof(struct IP6Header) + sizeof(struct TCPHeader); - const int ip_sctp_size = - sizeof(struct IPHeader) + sizeof(struct SCTPHeader); const struct ICMPHeader *icmp; const struct UDPHeader *udp; const struct TCPHeader *tcp; - const struct SCTPHeader *sctp; int udp_length; +#ifdef IPPROTO_SCTP + const int ip_sctp_size = + sizeof(struct IPHeader) + sizeof(struct SCTPHeader); + const struct SCTPHeader *sctp; +#endif if (ip->protocol == IPPROTO_ICMPV6) { if (packet_length < ip_icmp_size) { diff --git a/packet/probe.c b/packet/probe.c index aedbdf0..31f47f3 100644 --- a/packet/probe.c +++ b/packet/probe.c @@ -200,7 +200,8 @@ void format_mpls_string( char *append_pos = str; const struct mpls_label_t *mpls; - strcpy(str, ""); + /* Start with an empty string */ + str[0] = 0; for (i = 0; i < mpls_count; i++) { mpls = &mpls_list[i]; diff --git a/packet/probe_unix.c b/packet/probe_unix.c index 3f52575..e115ffe 100644 --- a/packet/probe_unix.c +++ b/packet/probe_unix.c @@ -150,9 +150,9 @@ static void check_sctp_support( struct net_state_t *net_state) { +#ifdef IPPROTO_SCTP int sctp_socket; -#ifdef IPPROTO_SCTP sctp_socket = socket(AF_INET, SOCK_STREAM, IPPROTO_SCTP); if (sctp_socket != -1) { close(sctp_socket); diff --git a/packet/wait_unix.c b/packet/wait_unix.c index 408df82..c4ca5c5 100644 --- a/packet/wait_unix.c +++ b/packet/wait_unix.c @@ -23,6 +23,7 @@ #include #include #include +#include #include /* diff --git a/ui/asn.c b/ui/asn.c index bbf4180..4239ad0 100644 --- a/ui/asn.c +++ b/ui/asn.c @@ -190,11 +190,14 @@ static char* split_txtrec(struct mtr_ctl *ctl, char *txt_rec) { #ifdef ENABLE_IPV6 /* from dns.c:addr2ip6arpa() */ -static void reverse_host6(struct in6_addr *addr, char *buff) { +static void reverse_host6(struct in6_addr *addr, char *buff, int buff_length) { int i; char *b = buff; for (i=(sizeof(*addr)/2-1); i>=0; i--, b+=4) /* 64b portion */ - sprintf(b, "%x.%x.", addr->s6_addr[i] & 0xf, addr->s6_addr[i] >> 4); + snprintf( + b, buff_length, + "%x.%x.", addr->s6_addr[i] & 0xf, addr->s6_addr[i] >> 4); + buff[strlen(buff) - 1] = '\0'; } #endif @@ -210,7 +213,7 @@ static char *get_ipinfo(struct mtr_ctl *ctl, ip_t *addr){ if (ctl->af == AF_INET6) { #ifdef ENABLE_IPV6 - reverse_host6(addr, key); + reverse_host6(addr, key, NAMELEN); if (snprintf(lookup_key, NAMELEN, "%s.origin6.asn.cymru.com", key) >= NAMELEN) return NULL; #else @@ -243,7 +246,7 @@ static char *get_ipinfo(struct mtr_ctl *ctl, ip_t *addr){ DEB_syslog(LOG_INFO, "Looked up: %s", key); if (iihash) if ((item.key = xstrdup(key))) { - item.data = items; + item.data = (void *)items; hsearch(item, ENTER); DEB_syslog(LOG_INFO, "Insert into hash: %s", key); } diff --git a/ui/cmdpipe.c b/ui/cmdpipe.c index 9ce4ce1..81b1e7b 100644 --- a/ui/cmdpipe.c +++ b/ui/cmdpipe.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -225,7 +226,7 @@ void execute_packet_child(void) First, try to execute using /usr/bin/env, because this will search the PATH for mtr-packet */ - execl("/usr/bin/env", "mtr-packet", mtr_packet_path, NULL); + execl("/usr/bin/env", "mtr-packet", mtr_packet_path, (char *)NULL); /* If env fails to execute, try to use the MTR_PACKET environment @@ -236,7 +237,7 @@ void execute_packet_child(void) could be executed. This will only be the case if /usr/bin/env doesn't exist. */ - execl(mtr_packet_path, "mtr-packet", NULL); + execl(mtr_packet_path, "mtr-packet", (char *)NULL); /* Both exec attempts failed, so nothing to do but exit */ exit(1); diff --git a/ui/curses.c b/ui/curses.c index 94681f0..214f27d 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -91,7 +91,7 @@ static void pwcenter(char *str) getmaxyx(stdscr, __unused_int, maxx); cx = (size_t)(maxx - strlen(str)) / 2; - printw("%*s%s", cx, "", str); + printw("%*s%s", (int)cx, "", str); } @@ -360,7 +360,7 @@ extern int mtr_curses_keyaction(struct mtr_ctl *ctl) } -static void format_field (char *dst, const char *format, int n) +static void format_field (char *dst, int dst_length, const char *format, int n) { if (index (format, 'N' ) ) { *dst++ = ' '; @@ -368,10 +368,10 @@ static void format_field (char *dst, const char *format, int n) } else if (strchr( format, 'f' ) ) { /* this is for fields where we measure integer microseconds but display floating point miliseconds. Convert to float here. */ - sprintf(dst, format, n / 1000.0 ); + snprintf(dst, dst_length, format, n / 1000.0 ); /* this was marked as a temporary hack over 10 years ago. -- REW */ } else { - sprintf(dst, format, n); + snprintf(dst, dst_length, format, n); } } @@ -423,7 +423,9 @@ static void mtr_curses_hosts(struct mtr_ctl *ctl, int startstat) can't be careful enough. */ j = ctl->fld_index[ctl->fld_active[i]]; if (j == -1) continue; - format_field (buf+hd_len, data_fields[j].format, data_fields[j].net_xxx(at)); + format_field ( + buf+hd_len, sizeof(buf) - hd_len, + data_fields[j].format, data_fields[j].net_xxx(at)); hd_len += data_fields[j].length; } buf[hd_len] = 0; @@ -648,8 +650,9 @@ extern void mtr_curses_redraw(struct mtr_ctl *ctl) j = ctl->fld_index[ctl->fld_active[i]]; if (j < 0) continue; - sprintf( fmt, "%%%ds", data_fields[j].length ); - sprintf( buf + hd_len, fmt, data_fields[j].title ); + snprintf( fmt, sizeof(fmt), "%%%ds", data_fields[j].length ); + snprintf( + buf + hd_len, sizeof(buf) - hd_len, fmt, data_fields[j].title ); hd_len += data_fields[j].length; } attron(A_BOLD); @@ -673,7 +676,7 @@ extern void mtr_curses_redraw(struct mtr_ctl *ctl) max_cols = maxx <= SAVED_PINGS + padding ? maxx-padding : SAVED_PINGS; startstat = padding - 2; - sprintf(msg, " Last %3d pings", max_cols); + snprintf(msg, sizeof(msg), " Last %3d pings", max_cols); mvprintw(rowstat - 1, startstat, msg); attroff(A_BOLD); diff --git a/ui/dns.c b/ui/dns.c index e945c04..7e976cf 100644 --- a/ui/dns.c +++ b/ui/dns.c @@ -179,7 +179,10 @@ extern void dns_open(struct mtr_ctl *ctl) rv = getnameinfo ((struct sockaddr *) &sa, salen, hostname, sizeof (hostname), NULL, 0, 0); if (rv == 0) { - sprintf (result, "%s %s\n", strlongip (ctl, &host), hostname); + snprintf ( + result, sizeof(result), + "%s %s\n", strlongip (ctl, &host), hostname); + rv = write (fromdns[1], result, strlen (result)); if (rv < 0) error (0, errno, "write DNS lookup result"); diff --git a/ui/split.c b/ui/split.c index 1bcce4e..dc4b34e 100644 --- a/ui/split.c +++ b/ui/split.c @@ -106,7 +106,7 @@ extern void split_redraw(struct mtr_ctl *ctl) net_best(at) /1000, net_avg(at)/1000, net_worst(at)/1000); } else { - sprintf(newLine, "???"); + snprintf(newLine, sizeof(newLine), "???"); } if (strcmp(newLine, Lines[at]) == 0) { @@ -150,7 +150,7 @@ extern void split_close(void) extern int split_keyaction(void) { #ifdef HAVE_CURSES - char c = getch(); + unsigned char c = getch(); #else fd_set readfds; struct timeval tv; diff --git a/ui/utils.c b/ui/utils.c index 6681038..af12b1e 100644 --- a/ui/utils.c +++ b/ui/utils.c @@ -46,7 +46,7 @@ extern char *trim(char *str, const char c) size_t len; /* left trim */ - while (*p && (isspace(*p) || (c && *p == c))) + while (*p && (isspace((unsigned char)*p) || (c && *p == c))) p++; if (str < p) { len = strlen(str); @@ -57,7 +57,7 @@ extern char *trim(char *str, const char c) len = strlen(str); while (len) { len--; - if (isspace(str[len]) || (c && str[len] == c)) { + if (isspace((unsigned char)str[len]) || (c && str[len] == c)) { continue; } len++; -- 2.47.2