clang-tidy from LLVM 13.0.1 reports some new warnings from these checkers: - altera-unroll-loops, altera-id-dependent-backward-branch: ignore for the moment being, add a TODO item - bugprone-easily-swappable-parameters: ignore, nothing to do about those - readability-function-cognitive-complexity: ignore for the moment being, add a TODO item - altera-struct-pack-align: ignore, alignment is forced in protocol headers - concurrency-mt-unsafe: ignore for the moment being, add a TODO item Fix bugprone-implicit-widening-of-multiplication-result warnings, though, that's doable and they seem to make sense. Signed-off-by: Stefano Brivio --- Makefile | 24 +++++++++++++++++++++++- checksum.c | 7 +++++-- conf.c | 4 ++-- dhcp.c | 8 ++++---- qrap.c | 2 +- tap.c | 8 ++++---- tcp.c | 10 +++++----- util.c | 8 +++++--- util.h | 6 +++--- 9 files changed, 52 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index 443c39d..5085578 100644 --- a/Makefile +++ b/Makefile @@ -167,6 +167,23 @@ pkgs: # # - bugprone-suspicious-string-compare # Return value of memcmp(), not really suspicious +# +# - altera-unroll-loops +# - altera-id-dependent-backward-branch +# TODO: check paths where it might make sense to improve performance +# +# - bugprone-easily-swappable-parameters +# Not much can be done about them other than being careful +# +# - readability-function-cognitive-complexity +# TODO: split reported functions +# +# - altera-struct-pack-align +# "Poor" alignment needed for structs reflecting message formats/headers +# +# - concurrency-mt-unsafe +# TODO: check again if multithreading is implemented + clang-tidy: $(wildcard *.c) $(wildcard *.h) clang-tidy -checks=*,-modernize-*,\ -clang-analyzer-valist.Uninitialized,\ @@ -187,7 +204,12 @@ clang-tidy: $(wildcard *.c) $(wildcard *.h) -bugprone-narrowing-conversions,\ -cppcoreguidelines-narrowing-conversions,\ -cppcoreguidelines-avoid-non-const-global-variables,\ - -bugprone-suspicious-string-compare \ + -bugprone-suspicious-string-compare,\ + -altera-unroll-loops,-altera-id-dependent-backward-branch,\ + -bugprone-easily-swappable-parameters,\ + -readability-function-cognitive-complexity,\ + -altera-struct-pack-align,\ + -concurrency-mt-unsafe \ --warnings-as-errors=* $(wildcard *.c) -- $(CFLAGS) ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1) diff --git a/checksum.c b/checksum.c index c9905d1..acb1e3e 100644 --- a/checksum.c +++ b/checksum.c @@ -108,10 +108,13 @@ uint16_t csum_unaligned(const void *buf, size_t len, uint32_t init) */ void csum_tcp4(struct iphdr *iph) { - struct tcphdr *th = (struct tcphdr *)((char *)iph + iph->ihl * 4); - uint16_t tlen = ntohs(iph->tot_len) - iph->ihl * 4, *p = (uint16_t *)th; + uint16_t tlen = ntohs(iph->tot_len) - iph->ihl * 4, *p; + struct tcphdr *th; uint32_t sum = 0; + th = (struct tcphdr *)((char *)iph + (intptr_t)(iph->ihl * 4)); + p = (uint16_t *)th; + sum += (iph->saddr >> 16) & 0xffff; sum += iph->saddr & 0xffff; sum += (iph->daddr >> 16) & 0xffff; diff --git a/conf.c b/conf.c index 9a9dade..abe63a1 100644 --- a/conf.c +++ b/conf.c @@ -867,7 +867,7 @@ void conf(struct ctx *c, int argc, char **argv) for (i = 0; i < ETH_ALEN; i++) { errno = 0; - b = strtol(optarg + i * 3, NULL, 16); + b = strtol(optarg + (intptr_t)i * 3, NULL, 16); if (b < 0 || b > UCHAR_MAX || errno) { err("Invalid MAC address: %s", optarg); usage(argv[0]); @@ -1032,7 +1032,7 @@ void conf(struct ctx *c, int argc, char **argv) case 'M': for (i = 0; i < ETH_ALEN; i++) { errno = 0; - b = strtol(optarg + i * 3, NULL, 16); + b = strtol(optarg + (intptr_t)i * 3, NULL, 16); if (b < 0 || b > UCHAR_MAX || errno) { err("Invalid MAC address: %s", optarg); usage(argv[0]); diff --git a/dhcp.c b/dhcp.c index 0d6d698..a052397 100644 --- a/dhcp.c +++ b/dhcp.c @@ -271,10 +271,10 @@ int dhcp(struct ctx *c, struct ethhdr *eh, size_t len) if (len < sizeof(*eh) + sizeof(*iph)) return 0; - if (len < sizeof(*eh) + iph->ihl * 4 + sizeof(*uh)) + if (len < sizeof(*eh) + (long)iph->ihl * 4 + sizeof(*uh)) return 0; - uh = (struct udphdr *)((char *)iph + iph->ihl * 4); + uh = (struct udphdr *)((char *)iph + (long)(iph->ihl * 4)); m = (struct msg *)(uh + 1); if (uh->dest != htons(67)) @@ -283,7 +283,7 @@ int dhcp(struct ctx *c, struct ethhdr *eh, size_t len) if (c->no_dhcp) return 1; - mlen = len - sizeof(*eh) - iph->ihl * 4 - sizeof(*uh); + mlen = len - sizeof(*eh) - (long)iph->ihl * 4 - sizeof(*uh); if (mlen != ntohs(uh->len) - sizeof(*uh) || mlen < offsetof(struct msg, o) || m->op != BOOTREQUEST) @@ -349,7 +349,7 @@ int dhcp(struct ctx *c, struct ethhdr *eh, size_t len) iph->daddr = c->addr4; iph->saddr = c->gw4; iph->check = 0; - iph->check = csum_unaligned(iph, iph->ihl * 4, 0); + iph->check = csum_unaligned(iph, (intptr_t)(iph->ihl * 4), 0); len += sizeof(*eh); memcpy(eh->h_dest, eh->h_source, ETH_ALEN); diff --git a/qrap.c b/qrap.c index 1eb82fa..3ee73a7 100644 --- a/qrap.c +++ b/qrap.c @@ -111,7 +111,7 @@ void usage(const char *name) */ int main(int argc, char **argv) { - struct timeval tv = { .tv_sec = 0, .tv_usec = 500 * 1000 }; + struct timeval tv = { .tv_sec = 0, .tv_usec = (long)(500 * 1000) }; int i, s, qemu_argc = 0, addr_map = 0, has_dev = 0; char *qemu_argv[ARG_MAX], dev_str[ARG_MAX]; struct sockaddr_un addr = { diff --git a/tap.c b/tap.c index 34a5705..22db9c5 100644 --- a/tap.c +++ b/tap.c @@ -131,7 +131,7 @@ void tap_ip_send(struct ctx *c, struct in6_addr *src, uint8_t proto, memcpy(&iph->saddr, &src->s6_addr[12], 4); iph->check = 0; - iph->check = csum_unaligned(iph, iph->ihl * 4, 0); + iph->check = csum_unaligned(iph, (size_t)iph->ihl * 4, 0); memcpy(data, in, len); @@ -337,9 +337,9 @@ resume: continue; iph = (struct iphdr *)(eh + 1); - if ((iph->ihl * 4) + sizeof(*eh) > len) + if ((size_t)iph->ihl * 4 + sizeof(*eh) > len) continue; - if (iph->ihl * 4 < (int)sizeof(*iph)) + if ((size_t)iph->ihl * 4 < (int)sizeof(*iph)) continue; if (iph->saddr && c->addr4_seen != iph->saddr) { @@ -347,7 +347,7 @@ resume: proto_update_l2_buf(NULL, NULL, &c->addr4_seen); } - l4h = (char *)iph + iph->ihl * 4; + l4h = (char *)iph + (size_t)iph->ihl * 4; l4_len = len - ((intptr_t)l4h - (intptr_t)eh); if (iph->protocol == IPPROTO_ICMP) { diff --git a/tcp.c b/tcp.c index 7122898..723b18e 100644 --- a/tcp.c +++ b/tcp.c @@ -345,7 +345,7 @@ #define TCP_TAP_FRAMES 256 -#define MAX_PIPE_SIZE (2 * 1024 * 1024) +#define MAX_PIPE_SIZE (2UL * 1024 * 1024) #define TCP_HASH_TABLE_LOAD 70 /* % */ #define TCP_HASH_TABLE_SIZE (MAX_TAP_CONNS * 100 / \ @@ -1040,8 +1040,8 @@ static int tcp_opt_get(struct tcphdr *th, size_t len, uint8_t type_search, uint8_t type, optlen; char *p; - if (len > (unsigned)th->doff * 4) - len = th->doff * 4; + if (len > (size_t)th->doff * 4) + len = (size_t)th->doff * 4; len -= sizeof(*th); p = (char *)(th + 1); @@ -1568,7 +1568,7 @@ static int tcp_update_seqack_wnd(struct ctx *c, struct tcp_tap_conn *conn, #else if (conn->state > ESTABLISHED || (flags & (DUP_ACK | FORCE_ACK)) || conn->local || tcp_rtt_dst_low(conn) || - conn->snd_buf < SNDBUF_SMALL) { + (unsigned long)conn->snd_buf < SNDBUF_SMALL) { conn->seq_ack_to_tap = conn->seq_from_tap; } else if (conn->seq_ack_to_tap != conn->seq_from_tap) { if (!tinfo) { @@ -2393,7 +2393,7 @@ static void tcp_data_from_tap(struct ctx *c, struct tcp_tap_conn *conn, return; } - off = th->doff * 4; + off = (size_t)th->doff * 4; if (off < sizeof(*th) || off > len) { tcp_rst(c, conn); return; diff --git a/util.c b/util.c index 46a539b..57b987a 100644 --- a/util.c +++ b/util.c @@ -51,7 +51,7 @@ void name(const char *format, ...) { \ clock_gettime(CLOCK_REALTIME, &tp); \ fprintf(stderr, "%li.%04li: ", \ tp.tv_sec - log_debug_start, \ - tp.tv_nsec / (100 * 1000)); \ + tp.tv_nsec / (100L * 1000)); \ } else { \ va_start(args, format); \ passt_vsyslog(level, format, args); \ @@ -305,12 +305,14 @@ void sock_probe_mem(struct ctx *c) sl = sizeof(v); if (setsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v)) || - getsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, &sl) || v < SNDBUF_BIG) + getsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, &sl) || + (size_t)v < SNDBUF_BIG) c->low_wmem = 1; v = INT_MAX / 2; if (setsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v)) || - getsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, &sl) || v < RCVBUF_BIG) + getsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, &sl) || + (size_t)v < RCVBUF_BIG) c->low_rmem = 1; close(s); diff --git a/util.h b/util.h index 800a91d..add4c1e 100644 --- a/util.h +++ b/util.h @@ -144,9 +144,9 @@ enum { .daddr = IN6ADDR_ANY_INIT, \ } -#define RCVBUF_BIG (2 * 1024 * 1024) -#define SNDBUF_BIG (4 * 1024 * 1024) -#define SNDBUF_SMALL (128 * 1024) +#define RCVBUF_BIG (2UL * 1024 * 1024) +#define SNDBUF_BIG (4UL * 1024 * 1024) +#define SNDBUF_SMALL (128UL * 1024) #include #include -- 2.34.1