public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes
@ 2025-05-13  9:40 Laurent Vivier
  2025-05-13  9:40 ` [PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization) Laurent Vivier
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Laurent Vivier @ 2025-05-13  9:40 UTC (permalink / raw)
  To: passt-dev; +Cc: 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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization)
  2025-05-13  9:40 [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Laurent Vivier
@ 2025-05-13  9:40 ` Laurent Vivier
  2025-05-13  9:41 ` [PATCH 2/4] virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c) Laurent Vivier
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2025-05-13  9:40 UTC (permalink / raw)
  To: passt-dev; +Cc: 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c)
  2025-05-13  9:40 [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Laurent Vivier
  2025-05-13  9:40 ` [PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization) Laurent Vivier
@ 2025-05-13  9:41 ` Laurent Vivier
  2025-05-13  9:41 ` [PATCH 3/4] ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub) Laurent Vivier
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2025-05-13  9:41 UTC (permalink / raw)
  To: passt-dev; +Cc: 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub)
  2025-05-13  9:40 [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Laurent Vivier
  2025-05-13  9:40 ` [PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization) Laurent Vivier
  2025-05-13  9:41 ` [PATCH 2/4] virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c) Laurent Vivier
@ 2025-05-13  9:41 ` Laurent Vivier
  2025-05-13  9:41 ` [PATCH 4/4] flow: Fix clang error (clang-analyzer-security.PointerSub) Laurent Vivier
  2025-05-14 16:27 ` [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Stefano Brivio
  4 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2025-05-13  9:41 UTC (permalink / raw)
  To: passt-dev; +Cc: 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] flow: Fix clang error (clang-analyzer-security.PointerSub)
  2025-05-13  9:40 [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Laurent Vivier
                   ` (2 preceding siblings ...)
  2025-05-13  9:41 ` [PATCH 3/4] ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub) Laurent Vivier
@ 2025-05-13  9:41 ` Laurent Vivier
  2025-05-14 16:27 ` [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Stefano Brivio
  4 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2025-05-13  9:41 UTC (permalink / raw)
  To: passt-dev; +Cc: 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes
  2025-05-13  9:40 [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Laurent Vivier
                   ` (3 preceding siblings ...)
  2025-05-13  9:41 ` [PATCH 4/4] flow: Fix clang error (clang-analyzer-security.PointerSub) Laurent Vivier
@ 2025-05-14 16:27 ` Stefano Brivio
  4 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2025-05-14 16:27 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: passt-dev

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-05-14 16:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-13  9:40 [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Laurent Vivier
2025-05-13  9:40 ` [PATCH 1/4] dhcpv6: fix GCC error (unterminated-string-initialization) Laurent Vivier
2025-05-13  9:41 ` [PATCH 2/4] virtio: Fix Clang warning (bugprone-sizeof-expression, cert-arr39-c) Laurent Vivier
2025-05-13  9:41 ` [PATCH 3/4] ndp: Fix Clang analyzer warning (clang-analyzer-security.PointerSub) Laurent Vivier
2025-05-13  9:41 ` [PATCH 4/4] flow: Fix clang error (clang-analyzer-security.PointerSub) Laurent Vivier
2025-05-14 16:27 ` [PATCH 0/4] Fedora 42: Static Analysis and Compiler Warning Fixes Stefano Brivio

Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).