[PATCH 0/8] Fix assorted warnings

[PATCH 0/8] Fix assorted warnings

[David Gibson]

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

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

[David Gibson]

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 <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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 <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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 <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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 <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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 <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 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

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

[David Gibson]

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 <david@gibson.dropbear.id.au>
---
 .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

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>
> ---
>   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 <lvivier@redhat.com>

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>
> ---
>   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 <lvivier@redhat.com>

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   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;

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   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
>   
>   /**

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   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;
>   

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   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);

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   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];

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

[Laurent Vivier]

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 <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   .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

Re: [PATCH 0/8] Fix assorted warnings

[Stefano Brivio]

On Mon, 11 May 2026 20:03:14 +1000
David Gibson <david@gibson.dropbear.id.au> 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