[PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes

[PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes

[Laurent Vivier]

This patch series addresses a collection of warnings and errors flagged
by GCC and Clang's static analyzer across various modules. The fixes
primarily involve clarifying code intent where warnings were false
positives or applying specific attributes to suppress warnings for
intentionally designed code constructs.

The series consists of the following patches (in order of application):

1.  **dhcpv6: fix GCC error (unterminated-string-initialization)**
    Silences GCC error for an intentionally non-NUL-terminated string.

2.  **virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)**
    Justifies intentional `sizeof` usage in pointer arithmetic against Clang warnings.

3.  **ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)**
    Clarifies pointer subtraction as a valid C idiom for struct offset calculation.

4.  **flow: fix clang error (clang-analyzer-security.PointerSub)**
    Confirms pointers in `flow_idx()` reference the same array, validating subtraction.

Laurent Vivier (4):
  dhcpv6: fix GCC error (unterminated-string-initialization)
  virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)
  ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)
  flow: Fix clang error (clang-analyzer-security.PointerSub)

 dhcpv6.c     | 4 +++-
 flow_table.h | 1 +
 ndp.c        | 1 +
 virtio.c     | 1 +
 4 files changed, 6 insertions(+), 1 deletion(-)

-- 
2.49.0

[PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization)

[Laurent Vivier]

The string STR_NOTONLINK is intentionally not NUL-terminated.
Ignore the GCC error using __attribute__((nonstring)).

This error is reported by GCC 15.1.1 on Fedora 42. However,
Clang 20.1.3 does not support __attribute__((nonstring)).
Therefore, NOLINTNEXTLINE(clang-diagnostic-unknown-attributes)
is also added to suppress Clang's unknown attribute warning.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 dhcpv6.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dhcpv6.c b/dhcpv6.c
index 373a98869f9b..ba16c664ee24 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -144,7 +144,9 @@ struct opt_ia_addr {
 struct opt_status_code {
 	struct opt_hdr hdr;
 	uint16_t code;
-	char status_msg[sizeof(STR_NOTONLINK) - 1];
+	/* "nonstring" is only supported since clang 23 */
+	/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
+	__attribute__((nonstring)) char status_msg[sizeof(STR_NOTONLINK) - 1];
 } __attribute__((packed));
 
 /**
-- 
@@ -144,7 +144,9 @@ struct opt_ia_addr {
 struct opt_status_code {
 	struct opt_hdr hdr;
 	uint16_t code;
-	char status_msg[sizeof(STR_NOTONLINK) - 1];
+	/* "nonstring" is only supported since clang 23 */
+	/* NOLINTNEXTLINE(clang-diagnostic-unknown-attributes) */
+	__attribute__((nonstring)) char status_msg[sizeof(STR_NOTONLINK) - 1];
 } __attribute__((packed));
 
 /**
-- 
2.49.0

[PATCH 2/4] virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)

[Laurent Vivier]

In `virtqueue_read_indirect_desc()`, the pointer arithmetic involving
`desc` is intentional. We add the length in bytes (`read_len`)
divided by the size of `struct vring_desc` to `desc`, which is
an array of `struct vring_desc`. This correctly calculates the
offset in terms of the number of `struct vring_desc` elements.

Clang issues the following warning due to this explicit scaling:

virtio.c:238:8: error: suspicious usage of 'sizeof(...)' in pointer
arithmetic; this scaled value will be scaled again by the '+='
operator [bugprone-sizeof-expression,cert-arr39-c,-Werror]
  238 |         desc += read_len / sizeof(struct vring_desc);
      |               ^            ~~~~~~~~~~~~~~~~~~~~~~~~~
virtio.c:238:8: note: '+=' in pointer arithmetic internally scales
with 'sizeof(struct vring_desc)' == 16

This behavior is intended, so the warning can be considered a
false positive in this context. The code correctly advances the
pointer by the desired number of descriptor entries.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 virtio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/virtio.c b/virtio.c
index bc2b89a7f4a7..f7db0073b66c 100644
--- a/virtio.c
+++ b/virtio.c
@@ -235,6 +235,7 @@ static int virtqueue_read_indirect_desc(const struct vu_dev *dev,
 		memcpy(desc, orig_desc, read_len);
 		len -= read_len;
 		addr += read_len;
+		/* NOLINTNEXTLINE(bugprone-sizeof-expression,cert-arr39-c) */
 		desc += read_len / sizeof(struct vring_desc);
 	}
 
-- 
@@ -235,6 +235,7 @@ static int virtqueue_read_indirect_desc(const struct vu_dev *dev,
 		memcpy(desc, orig_desc, read_len);
 		len -= read_len;
 		addr += read_len;
+		/* NOLINTNEXTLINE(bugprone-sizeof-expression,cert-arr39-c) */
 		desc += read_len / sizeof(struct vring_desc);
 	}
 
-- 
2.49.0

[PATCH 3/4] ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)

[Laurent Vivier]

Addresses Clang warning: "Subtraction of two pointers that do not
point into the same array is undefined behavior" for the line:
  `ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);`

Here, `ptr` is `&ra.var[0]`. The subtraction calculates the offset
of `var[0]` within the `struct ra_options ra`. Since `ptr` points
inside `ra`, this pointer arithmetic is well-defined for
calculating the size of the data to send, even if `ptr` and `&ra`
are not strictly considered part of the same "array" by the analyzer.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 ndp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ndp.c b/ndp.c
index ded2081ddd1d..b664034b00b1 100644
--- a/ndp.c
+++ b/ndp.c
@@ -328,6 +328,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
 
 	memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
 
+	/* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */
 	ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);
 }
 
-- 
@@ -328,6 +328,7 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
 
 	memcpy(&ra.source_ll.mac, c->our_tap_mac, ETH_ALEN);
 
+	/* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */
 	ndp_send(c, dst, &ra, ptr - (unsigned char *)&ra);
 }
 
-- 
2.49.0

[PATCH 4/4] flow: Fix clang error (clang-analyzer-security.PointerSub)

[Laurent Vivier]

Fixes the following clang-analyzer warning:

flow_table.h:96:25: note: Subtraction of two pointers that do not point into the same array is undefined behavior
   96 |         return (union flow *)f - flowtab;

The `flow_idx()` function is called via `FLOW_IDX()` from
`flow_foreach_slot()`, where `f` is set to `&flowtab[idx].f`.
Therefore, `f` and `flowtab` do point to the same array.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 flow_table.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/flow_table.h b/flow_table.h
index 2d5c65ca4d94..3f3f4b7527bf 100644
--- a/flow_table.h
+++ b/flow_table.h
@@ -93,6 +93,7 @@ extern union flow flowtab[];
  */
 static inline unsigned flow_idx(const struct flow_common *f)
 {
+	/* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */
 	return (union flow *)f - flowtab;
 }
 
-- 
@@ -93,6 +93,7 @@ extern union flow flowtab[];
  */
 static inline unsigned flow_idx(const struct flow_common *f)
 {
+	/* NOLINTNEXTLINE(clang-analyzer-security.PointerSub) */
 	return (union flow *)f - flowtab;
 }
 
-- 
2.49.0

Re: [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes

[Stefano Brivio]

On Tue, 13 May 2025 11:40:58 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> This patch series addresses a collection of warnings and errors flagged
> by GCC and Clang's static analyzer across various modules. The fixes
> primarily involve clarifying code intent where warnings were false
> positives or applying specific attributes to suppress warnings for
> intentionally designed code constructs.
> 
> The series consists of the following patches (in order of application):
> 
> 1.  **dhcpv6: fix GCC error (unterminated-string-initialization)**
>     Silences GCC error for an intentionally non-NUL-terminated string.
> 
> 2.  **virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)**
>     Justifies intentional `sizeof` usage in pointer arithmetic against Clang warnings.
> 
> 3.  **ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)**
>     Clarifies pointer subtraction as a valid C idiom for struct offset calculation.
> 
> 4.  **flow: fix clang error (clang-analyzer-security.PointerSub)**
>     Confirms pointers in `flow_idx()` reference the same array, validating subtraction.
> 
> Laurent Vivier (4):
>   dhcpv6: fix GCC error (unterminated-string-initialization)
>   virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)
>   ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)
>   flow: Fix clang error (clang-analyzer-security.PointerSub)

Applied. I also checked this against musl (with Clang) on Alpine, no
new errors from 'make clang-tidy'.

-- 
Stefano