[PATCH 05/18] net/net: fix switch/case fallthrough annotations

[Tom Rini]

On Wed, Apr 09, 2025 at 12:53:47AM +0100, Andre Przywara wrote:
> On Tue, 8 Apr 2025 16:29:18 -0600
> Tom Rini <trini at konsulko.com> wrote:
> 
> Hi Tom,
> 
> thanks for staying on this!
> 
> > On Thu, Mar 27, 2025 at 03:33:00PM +0000, Andre Przywara wrote:
> > 
> > > The net_check_prereq() routine in the generic network handling code
> > > mixes case: labels with #ifdef's, which makes predicting fallthrough
> > > situations tricky. We had two "fall through" comments in the code, but
> > > at the wrong places.
> > > 
> > > Remove one unneeded comment (no annotations necessary between just empty
> > > labels), and move one other instance to the right place (before any
> > > label sequence).
> > > This makes GCC's implicit fallthrough checker happy.
> > > 
> > > Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> > > Reviewed-by: Tom Rini <trini at konsulko.com>
> > > ---
> > >  net/net.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/net/net.c b/net/net.c
> > > index 5219367e391..f191f16357c 100644
> > > --- a/net/net.c
> > > +++ b/net/net.c
> > > @@ -1525,7 +1525,6 @@ static int net_check_prereq(enum proto_t protocol)
> > >  #if defined(CONFIG_CMD_NFS)
> > >  	case NFS:
> > >  #endif
> > > -		/* Fall through */
> > >  	case TFTPGET:
> > >  	case TFTPPUT:
> > >  		if (IS_ENABLED(CONFIG_IPV6) && use_ip6) {
> > > @@ -1539,11 +1538,11 @@ static int net_check_prereq(enum proto_t protocol)
> > >  			puts("*** ERROR: `serverip' not set\n");
> > >  			return 1;
> > >  		}
> > > +		fallthrough;
> > >  #if	defined(CONFIG_CMD_PING) || \
> > >  	defined(CONFIG_CMD_DNS) || defined(CONFIG_PROT_UDP)
> > >  common:
> > >  #endif
> > > -		/* Fall through */
> > >  
> > >  	case NETCONS:
> > >  	case FASTBOOT_UDP:  
> > 
> > So this one is harder than it looks. With clang, we cannot seemingly
> > have:
> > 	fallthrough;
> > #if defined(CONFIG_CMD_PING) || \
> >     defined(CONFIG_CMD_DNS) || defined(CONFIG_PROT_UDP)
> > common:
> > #endif
> > 
> > And gcc was failing on:
> > 	}
> > #if defined(CONFIG_CMD_PING) || \
> >     defined(CONFIG_CMD_DNS) || defined(CONFIG_PROT_UDP)
> > common:
> > #endif
> > 	fallthrough;
> 
> Yes, later stages of the CI told me so already ;-)
> 
> > Maybe we can move the label to inside the next set of cases, and then
> > also add CONFIG_CMD_PING6 to the checks, as that also has 'goto common;'
> 
> I found some other solution: dropping the #if's around the common:
> label, then marking this with __maybe_unused instead. Seems to work for
> both GCC and clang, and makes the code even more readable.
> 
> Will send this among the other gazillion fixes I meanwhile added in a
> v2, to a mailbox near you.
> If you can't wait: sunxi/fallthrough has the bits, though not yet split
> up and without commit messages. Still not passing all checks, but the
> CI builds seem to stop early, before revealing all issues, so this is a
> piecemeal job :-(

I thought I had tried what you suggest but maybe didn't quite get things
aligned right (but I also modified sandbox so it would trigger the
unused warning). That said, I'm applying most of v1 now'ish, and have
only stopped as part of trying to narrow down the seemingly random
evb-ast2600 CI failure.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250408/867cd127/attachment.sig>