[PATCH 00/18] Annotate switch/case fallthrough cases
Andre Przywara
andre.przywara at arm.com
Fri Mar 28 14:45:54 CET 2025
On Fri, 28 Mar 2025 10:39:59 +0000
Andre Przywara <andre.przywara at arm.com> wrote:
Hi,
> On Fri, 28 Mar 2025 11:28:05 +0100
> Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> Hi,
>
> > On 27.03.25 16:32, Andre Przywara wrote:
> > > C's implicit fallthrough behaviour in switch/case statements can lead to
> > > subtle bugs. Quite some while ago many compilers introduced warnings in
> > > those cases, requiring intentional fallthrough's to be annotated.
> > >
> > > So far we were not enabling that compiler option, so many ambiguities
> > > and some bugs in the code went unnoticed.
> > >
> > > This series adds the required annotations in code paths that the first
> > > stage of the U-Boot CI covers. There is a large number of cases left
> > > in the libbz2 code. The usage of switch/case is borderline insane there,
> > > labels are hidden in macros, and there are no breaks, but just goto's.
> > > Upstream still uses very similar code, without any annotations. I still
> > > am not 100% sure those are meant to fall through or not, and plan to do
> > > further investigations, but didn't want to hold the rest of the patches
> > > back. You can see for yourself by applying patch 18/18 and building for
> > > sandbox64, for instance.
> >
> > Can we use something like
> >
> > CFLAGS_REMOVE_bzlib.o = -Wimplicit-fallthrough
>
> Ah, didn't know we have that in U-Boot as well! Sounds promising, and
> fixes the sandbox build for me (when using bzlib_decompress.o). I will add
> this to the last patch and will check what the CI has to say about this.
So for the records: silencing the libbzip2 warnings uncovered a whole new
bunch of warnings, in stage 1 still. Some compilers used in the CI (clang?)
seem to be more picky about how to annotate, so a pure comment (/* fall
through */) would not cut it, it has to be the attribute - provided by our
"statement macro".
So I fixed those quickly, stuffed them into some patch, and now the first
CI stage (test.py) passes - but only to uncover a large number of new
warnings in the world build.
So I will keep on patching, as some kind of procrastination project ;-)
Cheers,
Andre
>
> Cheers,
> Andre
>
> >
> > in the Makefile if that module is too hard to fix?
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > Because of this we cannot quite enable the warning in the Makefile yet,
> > > but those fixes are worth regardless, and be it to increase readability.
> > >
> > > Please not that those patches do not fix anything, really, they just add
> > > those fallthrough annotations, so the series is not really critical.
> > >
> > > Cheers,
> > > Andre
> > >
> > > Andre Przywara (18):
> > > spl: mmc: properly annotate fallthrough
> > > zlib: annotate switch/case fallthrough cases
> > > gadget: f_thor: annotate switch/case fallthrough
> > > use proper fallthrough annotations
> > > net/net: fix switch/case fallthrough annotations
> > > fastboot: annotate switch/case fallthrough case
> > > net: sun8i-emac: annotate fallthrough
> > > usb: ohci-hcd: annotate switch/case fallthrough
> > > usb: xhci: annotate switch/case fallthrough properly (TBF)
> > > video: annotate switch/case fall-through
> > > net: e1000: annotate switch/case fallthrough
> > > mtd: ubi: annotate fallthrough (TBF)
> > > arm: mach-k3: am62p: annotate switch/case fallthrough
> > > mtd: spi-nor-tiny: annotate switch/case fallthrough
> > > mtd: rawnand: nand_base: annotate switch/case fallthrough
> > > cmd: pmic: annotate switch/case fallthrough
> > > cmd: spl: annotate switch/case fallthrough
> > > [DO NOT MERGE] Makefile: enable switch/case fallthrough warnings
> > >
> > > Makefile | 1 +
> > > arch/arm/mach-k3/am62px/am62p5_init.c | 1 +
> > > cmd/pmic.c | 1 +
> > > cmd/spl.c | 2 ++
> > > common/command.c | 2 +-
> > > common/spl/spl_mmc.c | 1 +
> > > drivers/fastboot/fb_command.c | 1 +
> > > drivers/mtd/nand/raw/nand_base.c | 4 ++++
> > > drivers/mtd/spi/spi-nor-tiny.c | 1 +
> > > drivers/mtd/ubi/attach.c | 1 +
> > > drivers/mtd/ubi/build.c | 3 +++
> > > drivers/net/e1000.c | 2 ++
> > > drivers/net/sun8i_emac.c | 1 +
> > > drivers/usb/gadget/f_thor.c | 1 +
> > > drivers/usb/host/ohci-hcd.c | 3 ++-
> > > drivers/usb/host/xhci.c | 2 +-
> > > drivers/video/video-uclass.c | 2 ++
> > > lib/tiny-printf.c | 2 +-
> > > lib/zlib/inflate.c | 21 +++++++++++++++++++++
> > > net/net.c | 5 ++---
> > > 20 files changed, 50 insertions(+), 7 deletions(-)
> > >
> >
>
More information about the U-Boot
mailing list