On Fri, 28 Mar 2025 10:39:59 +0000 Andre Przywara <andre.przyw...@arm.com> wrote:
Hi, > On Fri, 28 Mar 2025 11:28:05 +0100 > Heinrich Schuchardt <xypron.g...@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(-) > > > > > >
