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 <sbrivio(a)redhat.com>
---
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 <net/if.h>
#include <limits.h>
--
2.34.1