* [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() @ 2025-05-15 16:05 Stefano Brivio 2025-05-16 7:25 ` David Gibson 0 siblings, 1 reply; 5+ messages in thread From: Stefano Brivio @ 2025-05-15 16:05 UTC (permalink / raw) To: passt-dev Cppcheck 2.14.2 on Alpine Linux (musl) says that: tap.c:111:19: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn] switch (c->mode) { ^ This exit path is not reachable because we ASSERT(0) after the switch, but for some reason cppcheck doesn't see this with musl headers. Hide the warning with a redundant return statement. Signed-off-by: Stefano Brivio <sbrivio@redhat.com> --- tap.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tap.c b/tap.c index d630f6d..6db5d88 100644 --- a/tap.c +++ b/tap.c @@ -118,6 +118,8 @@ unsigned long tap_l2_max_len(const struct ctx *c) } /* NOLINTEND(bugprone-branch-clone) */ ASSERT(0); + + return 0; /* Unreachable, for cppcheck's sake */ } /** -- @@ -118,6 +118,8 @@ unsigned long tap_l2_max_len(const struct ctx *c) } /* NOLINTEND(bugprone-branch-clone) */ ASSERT(0); + + return 0; /* Unreachable, for cppcheck's sake */ } /** -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() 2025-05-15 16:05 [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() Stefano Brivio @ 2025-05-16 7:25 ` David Gibson 2025-05-16 16:26 ` Stefano Brivio 0 siblings, 1 reply; 5+ messages in thread From: David Gibson @ 2025-05-16 7:25 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- Attachment #1: Type: text/plain, Size: 1461 bytes --] On Thu, May 15, 2025 at 06:05:01PM +0200, Stefano Brivio wrote: > Cppcheck 2.14.2 on Alpine Linux (musl) says that: > > tap.c:111:19: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn] > switch (c->mode) { > ^ > > This exit path is not reachable because we ASSERT(0) after the switch, > but for some reason cppcheck doesn't see this with musl headers. Hide > the warning with a redundant return statement. > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> Hm. Alternatively you could change this to call abort_with_msg() directly. That's marked as ((noreturn)) so with any luck cppcheck will then be able to figure out what's going on. Although, I'm slightly surprised it can't do so already: I thought I put the ((noreturn)) in there to avoid warnings exactly like this. > --- > tap.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tap.c b/tap.c > index d630f6d..6db5d88 100644 > --- a/tap.c > +++ b/tap.c > @@ -118,6 +118,8 @@ unsigned long tap_l2_max_len(const struct ctx *c) > } > /* NOLINTEND(bugprone-branch-clone) */ > ASSERT(0); > + > + return 0; /* Unreachable, for cppcheck's sake */ > } > > /** -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() 2025-05-16 7:25 ` David Gibson @ 2025-05-16 16:26 ` Stefano Brivio 2025-05-17 10:29 ` David Gibson 0 siblings, 1 reply; 5+ messages in thread From: Stefano Brivio @ 2025-05-16 16:26 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev On Fri, 16 May 2025 17:25:21 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Thu, May 15, 2025 at 06:05:01PM +0200, Stefano Brivio wrote: > > Cppcheck 2.14.2 on Alpine Linux (musl) says that: > > > > tap.c:111:19: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn] > > switch (c->mode) { > > ^ > > > > This exit path is not reachable because we ASSERT(0) after the switch, > > but for some reason cppcheck doesn't see this with musl headers. Hide > > the warning with a redundant return statement. > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > Hm. Alternatively you could change this to call abort_with_msg() > directly. That's marked as ((noreturn)) so with any luck cppcheck > will then be able to figure out what's going on. Although, I'm > slightly surprised it can't do so already: I thought I put the > ((noreturn)) in there to avoid warnings exactly like this. So, I checked, and abort_with_msg(""); works. ASSERT() ultimately expands to abort_with_msg(), but for some reason cppcheck together with Clang can't "follow" that, I suppose it's something related to the pre-processor. But in any case, abort_with_msg("") doesn't look as descriptive and as obviously redundant as a return statement with a comment, so I don't quite see the benefit changing it (also considering that I need to test other versions, which takes time). -- Stefano ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() 2025-05-16 16:26 ` Stefano Brivio @ 2025-05-17 10:29 ` David Gibson 2025-05-19 7:27 ` Stefano Brivio 0 siblings, 1 reply; 5+ messages in thread From: David Gibson @ 2025-05-17 10:29 UTC (permalink / raw) To: Stefano Brivio; +Cc: passt-dev [-- Attachment #1: Type: text/plain, Size: 2023 bytes --] On Fri, May 16, 2025 at 06:26:23PM +0200, Stefano Brivio wrote: > On Fri, 16 May 2025 17:25:21 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > On Thu, May 15, 2025 at 06:05:01PM +0200, Stefano Brivio wrote: > > > Cppcheck 2.14.2 on Alpine Linux (musl) says that: > > > > > > tap.c:111:19: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn] > > > switch (c->mode) { > > > ^ > > > > > > This exit path is not reachable because we ASSERT(0) after the switch, > > > but for some reason cppcheck doesn't see this with musl headers. Hide > > > the warning with a redundant return statement. > > > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > > > Hm. Alternatively you could change this to call abort_with_msg() > > directly. That's marked as ((noreturn)) so with any luck cppcheck > > will then be able to figure out what's going on. Although, I'm > > slightly surprised it can't do so already: I thought I put the > > ((noreturn)) in there to avoid warnings exactly like this. > > So, I checked, and abort_with_msg(""); works. ASSERT() ultimately > expands to abort_with_msg(), but for some reason cppcheck together with > Clang can't "follow" that, I suppose it's something related to the > pre-processor. > > But in any case, abort_with_msg("") doesn't look as descriptive and > as obviously redundant as a return statement with a comment, so I don't > quite see the benefit changing it (also considering that I need to test > other versions, which takes time). Well, kind of the whole point of abort_with_msg() is that you can give it a meaningful message, which might change the picture. It would also be trivial to write an unreachable() wrapper around that. -- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() 2025-05-17 10:29 ` David Gibson @ 2025-05-19 7:27 ` Stefano Brivio 0 siblings, 0 replies; 5+ messages in thread From: Stefano Brivio @ 2025-05-19 7:27 UTC (permalink / raw) To: David Gibson; +Cc: passt-dev On Sat, 17 May 2025 20:29:20 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Fri, May 16, 2025 at 06:26:23PM +0200, Stefano Brivio wrote: > > On Fri, 16 May 2025 17:25:21 +1000 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > On Thu, May 15, 2025 at 06:05:01PM +0200, Stefano Brivio wrote: > > > > Cppcheck 2.14.2 on Alpine Linux (musl) says that: > > > > > > > > tap.c:111:19: error: Found an exit path from function with non-void return type that has missing return statement [missingReturn] > > > > switch (c->mode) { > > > > ^ > > > > > > > > This exit path is not reachable because we ASSERT(0) after the switch, > > > > but for some reason cppcheck doesn't see this with musl headers. Hide > > > > the warning with a redundant return statement. > > > > > > > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com> > > > > > > Hm. Alternatively you could change this to call abort_with_msg() > > > directly. That's marked as ((noreturn)) so with any luck cppcheck > > > will then be able to figure out what's going on. Although, I'm > > > slightly surprised it can't do so already: I thought I put the > > > ((noreturn)) in there to avoid warnings exactly like this. > > > > So, I checked, and abort_with_msg(""); works. ASSERT() ultimately > > expands to abort_with_msg(), but for some reason cppcheck together with > > Clang can't "follow" that, I suppose it's something related to the > > pre-processor. > > > > But in any case, abort_with_msg("") doesn't look as descriptive and > > as obviously redundant as a return statement with a comment, so I don't > > quite see the benefit changing it (also considering that I need to test > > other versions, which takes time). > > Well, kind of the whole point of abort_with_msg() is that you can give > it a meaningful message, which might change the picture. Right, but I was struggling with making it meaningful: "dodged cppcheck warning, you'll never see this"? I much prefer a return with a comment. > It would > also be trivial to write an unreachable() wrapper around that. Ah, that would be nice, yes. I would consider writing one after 2-3 more cases like this. -- Stefano ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-19 7:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-05-15 16:05 [PATCH] tap: Avoid bogus missingReturn cppcheck warning in tap_l2_max_len() Stefano Brivio 2025-05-16 7:25 ` David Gibson 2025-05-16 16:26 ` Stefano Brivio 2025-05-17 10:29 ` David Gibson 2025-05-19 7:27 ` 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).