[PATCH 0/8] Fix assorted warnings

David Gibson

11 May 2026 11 May '26

12:03 p.m.

With Fedora 44, I'm hitting a bunch of new warnings, one from the compiler, a few from cppcheck and many from clang-tidy. Fix them. David Gibson (8): netlink: erromsg should be const in nl_status() virtio: Reduce scope of variable conf: Fix not-actually-const parameter to conf_runas() and conf_ugid() clang-tidy: Squash inconsistent brace warnings in foreach macros clang-tidy: Suppress sscanf() warning harder packet, clang-tidy: Packet pool buffers are not NULL treewide: Make some additional variables static clang-tidy: Suppress some new unhelpful new warnings .clang-tidy | 11 +++++++++++ conf.c | 9 +++++---- flow.c | 2 ++ flow_table.h | 8 ++++++-- fwd.c | 3 ++- log.c | 2 +- netlink.c | 3 ++- packet.c | 2 ++ tcp.c | 6 +++--- tcp_buf.c | 4 ++-- virtio.c | 4 ++-- 11 files changed, 38 insertions(+), 16 deletions(-) -- 2.54.0

Show replies by date

David Gibson

11 May 11 May

12:03 p.m.

New subject: [PATCH 3/8] conf: Fix not-actually-const parameter to conf_runas() and conf_ugid()

Commit 4af3d83170fd changed the @opt parameter to conf_runas() to a const pointer, to remove a cppcheck error. However, that error was a false positive. We *do* modify that parameter via an aliased pointer within it retreived from strchr(). At least with gcc 16.1.1 that now causes a "discards const" warning. Revert that change and instead directly suppress the cppcheck warning. Fixes: 4af3d83170fd ("treewide: Fix more pointers which can be const") Signed-off-by: David Gibson --- conf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/conf.c b/conf.c index 4a4ab489..8acf66cc 100644 --- a/conf.c +++ b/conf.c @@ -925,7 +925,8 @@ dns6: * * Return: 0 on success, negative error code on failure */ -static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid) +/* cppcheck-suppress [constParameterPointer,unmatchedSuppression] */ +static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid) { const char *uopt, *gopt = NULL; char *sep = strchr(opt, ':'); @@ -977,7 +978,7 @@ static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid) * @uid: User ID, set on success * @gid: Group ID, set on success */ -static void conf_ugid(const char *runas, uid_t *uid, gid_t *gid) +static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) { /* If user has specified --runas, that takes precedence... */ if (runas) { @@ -1247,7 +1248,7 @@ void conf(struct ctx *c, int argc, char **argv) uint8_t prefix_len_from_opt = 0; unsigned int ifi4 = 0, ifi6 = 0; const char *logfile = NULL; - const char *runas = NULL; + char *runas = NULL; size_t logsize = 0; long fd_tap_opt; int name, ret; -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 2/8] virtio: Reduce scope of variable

As cppcheck points out, the 'min' variable in vu_log_queue_fill() can have its scope reduced. Do so. Signed-off-by: David Gibson --- virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/virtio.c b/virtio.c index b283de66..d7016cc3 100644 --- a/virtio.c +++ b/virtio.c @@ -630,9 +630,9 @@ static void vu_log_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, { struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE]; struct vring_desc *desc = vq->vring.desc; - unsigned int max, min; unsigned num_bufs = 0; uint64_t read_len; + unsigned int max; if (!vdev->log_table || !len || !vu_has_feature(vdev, VHOST_F_LOG_ALL)) return; @@ -672,7 +672,7 @@ static void vu_log_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, die("Looped descriptor"); if (le16toh(desc[index].flags) & VRING_DESC_F_WRITE) { - min = MIN(le32toh(desc[index].len), len); + unsigned min = MIN(le32toh(desc[index].len), len); vu_log_write(vdev, le64toh(desc[index].addr), min); len -= min; } -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 1/8] netlink: erromsg should be const in nl_status()

This pointer aliases part of the const nlmsghdr we're passed, so it should be const. While we're there, remove an unnecessary explicit cast (NLMSG_DATA() returns a void *, which implicitly casts to anything). This also removes a warning on cppcheck 2.20.0 and probably many other versions. Signed-off-by: David Gibson --- netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/netlink.c b/netlink.c index 6b74e882..9076462b 100644 --- a/netlink.c +++ b/netlink.c @@ -170,7 +170,7 @@ static int nl_status(const struct nlmsghdr *nh, ssize_t n, uint32_t seq) return 0; } if (nh->nlmsg_type == NLMSG_ERROR) { - struct nlmsgerr *errmsg = (struct nlmsgerr *)NLMSG_DATA(nh); + const struct nlmsgerr *errmsg = NLMSG_DATA(nh); return errmsg->error; } -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 4/8] clang-tidy: Squash inconsistent brace warnings in foreach macros

clang-tidy, at least as of 22.1.4 complains about if/else statements with inconsistent braces. We generally do want consistend bracing per our coding style. However, some of our foreach macros generate inconsistent bracing in a way that can't really be avoided. Add suppressions to stop clang-tidy complaining about these. Signed-off-by: David Gibson --- flow.c | 2 ++ flow_table.h | 8 ++++++-- netlink.c | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/flow.c b/flow.c index 91f2b81f..565ed2b2 100644 --- a/flow.c +++ b/flow.c @@ -67,9 +67,11 @@ static_assert(ARRAY_SIZE(flow_epoll) == FLOW_NUM_TYPES, #define foreach_established_tcp_flow(flow) \ flow_foreach_of_type((flow), FLOW_TCP) \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ if (!tcp_flow_is_established(&(flow)->tcp)) \ /* NOLINTNEXTLINE(bugprone-branch-clone) */ \ continue; \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ else /* Global Flow Table */ diff --git a/flow_table.h b/flow_table.h index 7694e726..e4ff6f73 100644 --- a/flow_table.h +++ b/flow_table.h @@ -67,8 +67,10 @@ extern union flow flowtab[]; */ #define flow_foreach(flow) \ flow_foreach_slot((flow)) \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ if ((flow)->f.state == FLOW_STATE_FREE) \ (flow) += (flow)->free.n - 1; \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ else if ((flow)->f.state != FLOW_STATE_ACTIVE) { \ flow_err((flow), "Bad flow state during traversal"); \ continue; \ @@ -81,10 +83,12 @@ extern union flow flowtab[]; */ #define flow_foreach_of_type(flow, type_) \ flow_foreach((flow)) \ - if ((flow)->f.type != (type_)) \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ + if ((flow)->f.type != (type_)) \ /* NOLINTNEXTLINE(bugprone-branch-clone) */ \ continue; \ - else + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ + else \ /** flow_idx() - Index of flow from common structure diff --git a/netlink.c b/netlink.c index 9076462b..c3c830e1 100644 --- a/netlink.c +++ b/netlink.c @@ -224,6 +224,7 @@ static struct nlmsghdr *nl_next(int s, char *buf, struct nlmsghdr *nh, ssize_t * nl_foreach((nh), (status), (s), (buf), (seq)) \ if ((nh)->nlmsg_type != (type)) { \ warn("netlink: Unexpected message type"); \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ } else /** -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 5/8] clang-tidy: Suppress sscanf() warning harder

We already have a clang-tidy suppression to stop if complaining about our use of sscanf() in procfs_scan_listen(). However, it now (clang-tidy 22.1.4 or thereabouts) seems there's an additional warning category that complains about the same thing. Add that to the suppression. Signed-off-by: David Gibson --- fwd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fwd.c b/fwd.c index 0697435d..c0a6adac 100644 --- a/fwd.c +++ b/fwd.c @@ -599,7 +599,8 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) lineread_init(&lr, fd); lineread_get(&lr, &line); /* throw away header */ while (lineread_get(&lr, &line) > 0) { - /* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */ + /* != 2 if conversion fails */ + /* NOLINTNEXTLINE(bugprone-unchecked-string-to-number-conversion,cert-err34-c) */ if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2) continue; -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 8/8] clang-tidy: Suppress some new unhelpful new warnings

clang-tidy 22.1.4 (or thereabouts) introduced some new warning categories that while theoretically useful, trip in too many silly occasions to be useful. Suppress them. Signed-off-by: David Gibson --- .clang-tidy | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.clang-tidy b/.clang-tidy index 773121f5..9ba43bf0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -86,6 +86,17 @@ Checks: # #if directives. Don't insist on #ifdef instead. - "-readability-use-concise-preprocessor-directives" + # clang-tidy, at least version 22.1.4 seems to generate a heap of + # false positives for this warning, asking us to make things + # static that are already used in other modules. + - "-misc-use-internal-linkage" + + # Warning about bool/int conversions could sometimes be useful. + # Unfortunately (as of clang-tidy 22.1.4) it warns in some really + # dumb situations, like not allowing !, && etc. on bool inputs to + # be treated directly as a bool. + - "-readability-implicit-bool-conversion" + WarningsAsErrors: "*" HeaderFileExtensions: - h -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 6/8] packet, clang-tidy: Packet pool buffers are not NULL

struct pool always needs a non-NULL buf field: it points either to the actual memory used to store packets, or for vhost-user to the vhost user memory structure which will contain the packets. We set this pointer when we initialise the pool. However, clang-tidy (as of 22.1.4, at least) doesn't realise this in packet_check_range(), causing UB warnings due to the subtraction of ptr and p->buf. Clue it in with an assert(). Signed-off-by: David Gibson --- packet.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packet.c b/packet.c index 1cb74b74..7a347be6 100644 --- a/packet.c +++ b/packet.c @@ -51,6 +51,8 @@ static int packet_check_range(const struct pool *p, const char *ptr, size_t len, { struct vdev_memory *memory; + assert(p->buf); + if (len > PACKET_MAX_LEN) { debug("packet range length %zu (max %zu), %s:%i", len, PACKET_MAX_LEN, func, line); -- 2.54.0

David Gibson

12:03 p.m.

New subject: [PATCH 7/8] treewide: Make some additional variables static

Mark a number of extra variables local to a single module as static. Signed-off-by: David Gibson --- conf.c | 2 +- log.c | 2 +- tcp.c | 6 +++--- tcp_buf.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/conf.c b/conf.c index 8acf66cc..029b9c7c 100644 --- a/conf.c +++ b/conf.c @@ -67,7 +67,7 @@ {{{ 0xfe, 0x80, 0, 0, 0, 0, 0, 0, \ 0, 0, 0, 0, 0, 0, 0, 0x01 }}} -const char *pasta_default_ifn = "tap0"; +static const char *pasta_default_ifn = "tap0"; /** * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration diff --git a/log.c b/log.c index 21e3673e..cbac2efd 100644 --- a/log.c +++ b/log.c @@ -85,7 +85,7 @@ static int logtime_fmt(char *buf, size_t size, const struct timespec *ts) } /* Prefixes for log file messages, indexed by priority */ -const char *logfile_prefix[] = { +static const char *logfile_prefix[] = { NULL, NULL, NULL, /* Unused: LOG_EMERG, LOG_ALERT, LOG_CRIT */ "ERROR: ", "WARNING: ", diff --git a/tcp.c b/tcp.c index 92d9797a..d6a9ba28 100644 --- a/tcp.c +++ b/tcp.c @@ -370,8 +370,8 @@ enum { */ #define TCP_MIGRATE_SND_QUEUE_MAX (64 << 20) #define TCP_MIGRATE_RCV_QUEUE_MAX (64 << 20) -uint8_t tcp_migrate_snd_queue [TCP_MIGRATE_SND_QUEUE_MAX]; -uint8_t tcp_migrate_rcv_queue [TCP_MIGRATE_RCV_QUEUE_MAX]; +static uint8_t tcp_migrate_snd_queue [TCP_MIGRATE_SND_QUEUE_MAX]; +static uint8_t tcp_migrate_rcv_queue [TCP_MIGRATE_RCV_QUEUE_MAX]; #define TCP_MIGRATE_RESTORE_CHUNK_MIN 1024 /* Try smaller when above this */ @@ -420,7 +420,7 @@ char tcp_buf_discard [BUF_DISCARD_SIZE]; bool peek_offset_cap; /* Size of data returned by TCP_INFO getsockopt() */ -socklen_t tcp_info_size; +static socklen_t tcp_info_size; #define tcp_info_cap(f_) \ ((offsetof(struct tcp_info_linux, tcpi_##f_) + \ diff --git a/tcp_buf.c b/tcp_buf.c index 41965b10..a092cb37 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -45,8 +45,8 @@ static struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM]; static struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; /* IP headers for IPv4 and IPv6 */ -struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; -struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; +static struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; +static struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; /* TCP segments with payload for IPv4 and IPv6 frames */ static struct tcp_payload_t tcp_payload[TCP_FRAMES_MEM]; -- 2.54.0

Laurent Vivier

2:50 p.m.

New subject: [PATCH 1/8] netlink: erromsg should be const in nl_status()

On 5/11/26 12:03, David Gibson wrote:

...

This pointer aliases part of the const nlmsghdr we're passed, so it should be const. While we're there, remove an unnecessary explicit cast (NLMSG_DATA() returns a void *, which implicitly casts to anything). This also removes a warning on cppcheck 2.20.0 and probably many other versions.

Signed-off-by: David Gibson --- netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/netlink.c b/netlink.c index 6b74e882..9076462b 100644 --- a/netlink.c +++ b/netlink.c @@ -170,7 +170,7 @@ static int nl_status(const struct nlmsghdr *nh, ssize_t n, uint32_t seq) return 0; } if (nh->nlmsg_type == NLMSG_ERROR) { - struct nlmsgerr *errmsg = (struct nlmsgerr *)NLMSG_DATA(nh); + const struct nlmsgerr *errmsg = NLMSG_DATA(nh); return errmsg->error; }

Reviewed-by: Laurent Vivier

Laurent Vivier

2:51 p.m.

New subject: [PATCH 2/8] virtio: Reduce scope of variable

On 5/11/26 12:03, David Gibson wrote:

...

As cppcheck points out, the 'min' variable in vu_log_queue_fill() can have its scope reduced. Do so.

Signed-off-by: David Gibson --- virtio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virtio.c b/virtio.c index b283de66..d7016cc3 100644 --- a/virtio.c +++ b/virtio.c @@ -630,9 +630,9 @@ static void vu_log_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, { struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE]; struct vring_desc *desc = vq->vring.desc; - unsigned int max, min; unsigned num_bufs = 0; uint64_t read_len; + unsigned int max;

if (!vdev->log_table || !len || !vu_has_feature(vdev, VHOST_F_LOG_ALL)) return; @@ -672,7 +672,7 @@ static void vu_log_queue_fill(const struct vu_dev *vdev, struct vu_virtq *vq, die("Looped descriptor");

if (le16toh(desc[index].flags) & VRING_DESC_F_WRITE) { - min = MIN(le32toh(desc[index].len), len); + unsigned min = MIN(le32toh(desc[index].len), len); vu_log_write(vdev, le64toh(desc[index].addr), min); len -= min; } Reviewed-by: Laurent Vivier

Laurent Vivier

3 p.m.

New subject: [PATCH 3/8] conf: Fix not-actually-const parameter to conf_runas() and conf_ugid()

On 5/11/26 12:03, David Gibson wrote:

...

Commit 4af3d83170fd changed the @opt parameter to conf_runas() to a const pointer, to remove a cppcheck error. However, that error was a false positive. We *do* modify that parameter via an aliased pointer within it retreived from strchr(). At least with gcc 16.1.1 that now causes a "discards const" warning. Revert that change and instead directly suppress the cppcheck warning.

Fixes: 4af3d83170fd ("treewide: Fix more pointers which can be const") Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

...

--- conf.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/conf.c b/conf.c index 4a4ab489..8acf66cc 100644 --- a/conf.c +++ b/conf.c @@ -925,7 +925,8 @@ dns6: * * Return: 0 on success, negative error code on failure */ -static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid) +/* cppcheck-suppress [constParameterPointer,unmatchedSuppression] */ +static int conf_runas(char *opt, unsigned int *uid, unsigned int *gid) { const char *uopt, *gopt = NULL; char *sep = strchr(opt, ':'); @@ -977,7 +978,7 @@ static int conf_runas(const char *opt, unsigned int *uid, unsigned int *gid) * @uid: User ID, set on success * @gid: Group ID, set on success */ -static void conf_ugid(const char *runas, uid_t *uid, gid_t *gid) +static void conf_ugid(char *runas, uid_t *uid, gid_t *gid) { /* If user has specified --runas, that takes precedence... */ if (runas) { @@ -1247,7 +1248,7 @@ void conf(struct ctx *c, int argc, char **argv) uint8_t prefix_len_from_opt = 0; unsigned int ifi4 = 0, ifi6 = 0; const char *logfile = NULL; - const char *runas = NULL; + char *runas = NULL; size_t logsize = 0; long fd_tap_opt; int name, ret;

Laurent Vivier

3:21 p.m.

New subject: [PATCH 4/8] clang-tidy: Squash inconsistent brace warnings in foreach macros

On 5/11/26 12:03, David Gibson wrote:

...

clang-tidy, at least as of 22.1.4 complains about if/else statements with inconsistent braces. We generally do want consistend bracing per our coding style. However, some of our foreach macros generate inconsistent bracing in a way that can't really be avoided. Add suppressions to stop clang-tidy complaining about these.

Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

...

--- flow.c | 2 ++ flow_table.h | 8 ++++++-- netlink.c | 1 + 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/flow.c b/flow.c index 91f2b81f..565ed2b2 100644 --- a/flow.c +++ b/flow.c @@ -67,9 +67,11 @@ static_assert(ARRAY_SIZE(flow_epoll) == FLOW_NUM_TYPES,

#define foreach_established_tcp_flow(flow) \ flow_foreach_of_type((flow), FLOW_TCP) \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ if (!tcp_flow_is_established(&(flow)->tcp)) \ /* NOLINTNEXTLINE(bugprone-branch-clone) */ \ continue; \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ else

/* Global Flow Table */ diff --git a/flow_table.h b/flow_table.h index 7694e726..e4ff6f73 100644 --- a/flow_table.h +++ b/flow_table.h @@ -67,8 +67,10 @@ extern union flow flowtab[]; */ #define flow_foreach(flow) \ flow_foreach_slot((flow)) \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ if ((flow)->f.state == FLOW_STATE_FREE) \ (flow) += (flow)->free.n - 1; \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ else if ((flow)->f.state != FLOW_STATE_ACTIVE) { \ flow_err((flow), "Bad flow state during traversal"); \ continue; \ @@ -81,10 +83,12 @@ extern union flow flowtab[]; */ #define flow_foreach_of_type(flow, type_) \ flow_foreach((flow)) \ - if ((flow)->f.type != (type_)) \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ + if ((flow)->f.type != (type_)) \ /* NOLINTNEXTLINE(bugprone-branch-clone) */ \ continue; \ - else + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ + else \

/** flow_idx() - Index of flow from common structure diff --git a/netlink.c b/netlink.c index 9076462b..c3c830e1 100644 --- a/netlink.c +++ b/netlink.c @@ -224,6 +224,7 @@ static struct nlmsghdr *nl_next(int s, char *buf, struct nlmsghdr *nh, ssize_t * nl_foreach((nh), (status), (s), (buf), (seq)) \ if ((nh)->nlmsg_type != (type)) { \ warn("netlink: Unexpected message type"); \ + /* NOLINTNEXTLINE(readability-inconsistent-ifelse-braces) */\ } else

/**

Laurent Vivier

3:23 p.m.

New subject: [PATCH 5/8] clang-tidy: Suppress sscanf() warning harder

On 5/11/26 12:03, David Gibson wrote:

...

We already have a clang-tidy suppression to stop if complaining about our use of sscanf() in procfs_scan_listen(). However, it now (clang-tidy 22.1.4 or thereabouts) seems there's an additional warning category that complains about the same thing. Add that to the suppression.

Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

...

--- fwd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fwd.c b/fwd.c index 0697435d..c0a6adac 100644 --- a/fwd.c +++ b/fwd.c @@ -599,7 +599,8 @@ static void procfs_scan_listen(int fd, unsigned int lstate, uint8_t *map) lineread_init(&lr, fd); lineread_get(&lr, &line); /* throw away header */ while (lineread_get(&lr, &line) > 0) { - /* NOLINTNEXTLINE(cert-err34-c): != 2 if conversion fails */ + /* != 2 if conversion fails */ + /* NOLINTNEXTLINE(bugprone-unchecked-string-to-number-conversion,cert-err34-c) */ if (sscanf(line, "%*u: %*x:%lx %*x:%*x %x", &port, &state) != 2) continue;

Laurent Vivier

3:27 p.m.

New subject: [PATCH 6/8] packet, clang-tidy: Packet pool buffers are not NULL

On 5/11/26 12:03, David Gibson wrote:

...

struct pool always needs a non-NULL buf field: it points either to the actual memory used to store packets, or for vhost-user to the vhost user memory structure which will contain the packets. We set this pointer when we initialise the pool. However, clang-tidy (as of 22.1.4, at least) doesn't realise this in packet_check_range(), causing UB warnings due to the subtraction of ptr and p->buf.

Clue it in with an assert().

Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

...

--- packet.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/packet.c b/packet.c index 1cb74b74..7a347be6 100644 --- a/packet.c +++ b/packet.c @@ -51,6 +51,8 @@ static int packet_check_range(const struct pool *p, const char *ptr, size_t len, { struct vdev_memory *memory;

+ assert(p->buf); + if (len > PACKET_MAX_LEN) { debug("packet range length %zu (max %zu), %s:%i", len, PACKET_MAX_LEN, func, line);

Laurent Vivier

3:30 p.m.

New subject: [PATCH 7/8] treewide: Make some additional variables static

On 5/11/26 12:03, David Gibson wrote:

...

Mark a number of extra variables local to a single module as static.

Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

...

--- conf.c | 2 +- log.c | 2 +- tcp.c | 6 +++--- tcp_buf.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/conf.c b/conf.c index 8acf66cc..029b9c7c 100644 --- a/conf.c +++ b/conf.c @@ -67,7 +67,7 @@ {{{ 0xfe, 0x80, 0, 0, 0, 0, 0, 0, \ 0, 0, 0, 0, 0, 0, 0, 0x01 }}}

-const char *pasta_default_ifn = "tap0"; +static const char *pasta_default_ifn = "tap0";

/** * add_dns4() - Possibly add the IPv4 address of a DNS resolver to configuration diff --git a/log.c b/log.c index 21e3673e..cbac2efd 100644 --- a/log.c +++ b/log.c @@ -85,7 +85,7 @@ static int logtime_fmt(char *buf, size_t size, const struct timespec *ts) }

/* Prefixes for log file messages, indexed by priority */ -const char *logfile_prefix[] = { +static const char *logfile_prefix[] = { NULL, NULL, NULL, /* Unused: LOG_EMERG, LOG_ALERT, LOG_CRIT */ "ERROR: ", "WARNING: ", diff --git a/tcp.c b/tcp.c index 92d9797a..d6a9ba28 100644 --- a/tcp.c +++ b/tcp.c @@ -370,8 +370,8 @@ enum { */ #define TCP_MIGRATE_SND_QUEUE_MAX (64 << 20) #define TCP_MIGRATE_RCV_QUEUE_MAX (64 << 20) -uint8_t tcp_migrate_snd_queue [TCP_MIGRATE_SND_QUEUE_MAX]; -uint8_t tcp_migrate_rcv_queue [TCP_MIGRATE_RCV_QUEUE_MAX]; +static uint8_t tcp_migrate_snd_queue [TCP_MIGRATE_SND_QUEUE_MAX]; +static uint8_t tcp_migrate_rcv_queue [TCP_MIGRATE_RCV_QUEUE_MAX];

#define TCP_MIGRATE_RESTORE_CHUNK_MIN 1024 /* Try smaller when above this */

@@ -420,7 +420,7 @@ char tcp_buf_discard [BUF_DISCARD_SIZE]; bool peek_offset_cap;

/* Size of data returned by TCP_INFO getsockopt() */ -socklen_t tcp_info_size; +static socklen_t tcp_info_size;

#define tcp_info_cap(f_) \ ((offsetof(struct tcp_info_linux, tcpi_##f_) + \ diff --git a/tcp_buf.c b/tcp_buf.c index 41965b10..a092cb37 100644 --- a/tcp_buf.c +++ b/tcp_buf.c @@ -45,8 +45,8 @@ static struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM]; static struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM];

/* IP headers for IPv4 and IPv6 */ -struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; -struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; +static struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; +static struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM];

/* TCP segments with payload for IPv4 and IPv6 frames */ static struct tcp_payload_t tcp_payload[TCP_FRAMES_MEM];

Laurent Vivier

3:32 p.m.

New subject: [PATCH 8/8] clang-tidy: Suppress some new unhelpful new warnings

On 5/11/26 12:03, David Gibson wrote:

...

clang-tidy 22.1.4 (or thereabouts) introduced some new warning categories that while theoretically useful, trip in too many silly occasions to be useful. Suppress them.

Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

...

--- .clang-tidy | 11 +++++++++++ 1 file changed, 11 insertions(+)

diff --git a/.clang-tidy b/.clang-tidy index 773121f5..9ba43bf0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -86,6 +86,17 @@ Checks: # #if directives. Don't insist on #ifdef instead. - "-readability-use-concise-preprocessor-directives"

+ # clang-tidy, at least version 22.1.4 seems to generate a heap of + # false positives for this warning, asking us to make things + # static that are already used in other modules. + - "-misc-use-internal-linkage" + + # Warning about bool/int conversions could sometimes be useful. + # Unfortunately (as of clang-tidy 22.1.4) it warns in some really + # dumb situations, like not allowing !, && etc. on bool inputs to + # be treated directly as a bool. + - "-readability-implicit-bool-conversion" + WarningsAsErrors: "*" HeaderFileExtensions: - h

Stefano Brivio

12 May 12 May

7:45 a.m.

On Mon, 11 May 2026 20:03:14 +1000 David Gibson wrote:

...

With Fedora 44, I'm hitting a bunch of new warnings, one from the compiler, a few from cppcheck and many from clang-tidy. Fix them.

David Gibson (8): netlink: erromsg should be const in nl_status() virtio: Reduce scope of variable conf: Fix not-actually-const parameter to conf_runas() and conf_ugid() clang-tidy: Squash inconsistent brace warnings in foreach macros clang-tidy: Suppress sscanf() warning harder packet, clang-tidy: Packet pool buffers are not NULL treewide: Make some additional variables static clang-tidy: Suppress some new unhelpful new warnings

Applied. -- Stefano

Age (days ago)

Last active (days ago)

List overview

Download

17 comments

3 participants

participants (3)

David Gibson
Laurent Vivier
Stefano Brivio

[PATCH 0/8] Fix assorted warnings

tags

participants (3)