[PATCH 1/1] tools: ftdgrep: use /* fallthrough */ as needed

[Tom Rini]

On Mon, May 11, 2020 at 09:08:03PM +0200, Heinrich Schuchardt wrote:
> On 5/11/20 8:40 PM, Tom Rini wrote:
> > On Sun, May 10, 2020 at 10:12:07PM +0900, Masahiro Yamada wrote:
> >> On Sun, May 10, 2020 at 12:12 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>>
> >>> GCC recognizes /* fallthrough */ if -Wimplicit-fallthrough=3 is enabled.
> >>
> >> FYI.
> >>
> >> Linux decided to not use /* fallthrough */ any more
> >> because Clang does not recognize it.
> >>
> >> __attribute__((__fallthrough__)) is supported
> >> by both Clang and recent GCC.
> In fact Linux has a define:
> 
> include/linux/compiler_attributes.h:200:# define fallthrough
>         __attribute__((__fallthrough__))
> 
> And in the code you would use
> 
> 	case foo:
> 		fallthrough;
> 	case bar:
> 
> But the Linux kernel still has a lot of lines with
> 
> /* fallthrough */
> 
> Documentation/process/deprecated.rst:
> 
> <cite>
> As there have been a long list of flaws `due to missing "break"
> statements <https://cwe.mitre.org/data/definitions/484.html>`_, we no
> longer allow implicit fall-through. In order to identify intentional
> fall-through cases, we have adopted a pseudo-keyword macro "fallthrough"
> which expands to gcc's extension `__attribute__((__fallthrough__))
> <https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_. (When
> the C17/C18  `[[fallthrough]]` syntax is more commonly supported by C
> compilers, static analyzers, and IDEs, we can switch to using that
> syntax for the macro pseudo-keyword.)
> </cite>
> 
> Using the attribute is not standard C and not any better than using the
> comment. The real target is the C17 syntax.
> 
> >>
> >>
> >> Linux is now doing treewide conversion
> >> from /* fallthrough */ to 'fallthrough;'.
> >>
> >> See include/linux/compiler_attributes.h in Linux.
> >>
> >> I do not know if U-Boot wants to align with it.
> >> (up to Tom ?)
> >
> > A re-sync on the compiler headers again and making use of this sounds
> > like a good idea, yes.
> >
> 
> We should enable -Wimplicit-fallthrough like the kernel does. This
> defaults to -Wimplicit-fallthrough=3 and is happy with both the comment
> as well as with the attribute.
> 
> @Tom:
> Will you update the compiler headers within this release cycle?
> Otherwise we should take the patch as is to get us closer to the
> -Wimplicit-fallthrough target.

I'm not going to update it for this release cycle.  I've done the
initial import and build and there's some fairly large changes related
to inlining that I want to look at harder to see if we can/should do
something about (I don't want to derail this thread, I'll start
another).  But it's very far from zero size change and given the inline
changes I think it'll need real testing.

And since the kernel isn't making a huge use yet of fallthrough; we can
afford to look a little harder at things.

-- 
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/20200512/1f59ea71/attachment.sig>