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

[Andre Przywara <andre.przywara@arm.com>]

On Tue, 8 Apr 2025 16:29:18 -0600
Tom Rini <trini@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@arm.com>
> > Reviewed-by: Tom Rini <trini@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 :-(

Cheers,
Andre