On Wed, Jul 31, 2019 at 01:02:07PM -0700, Kees Cook wrote: > On Wed, Jul 31, 2019 at 08:48:32PM +0200, Peter Zijlstra wrote: > > On Wed, Jul 31, 2019 at 11:24:36AM -0700, h...@zytor.com wrote: > > > >> > +/* > > > >> > + * Add the pseudo keyword 'fallthrough' so case statement blocks > > > >> > + * must end with any of these keywords: > > > >> > + * break; > > > >> > + * fallthrough; > > > >> > + * goto <label>; > > > >> > + * return [expression]; > > > >> > + * > > > >> > + * gcc: > > > >> > >https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html#Statement-Attributes > > > >> > + */ > > > >> > +#if __has_attribute(__fallthrough__) > > > >> > +# define fallthrough > > > >> > __attribute__((__fallthrough__)) > > > >> > +#else > > > >> > +# define fallthrough do {} while (0) /* > > > >> > fallthrough */ > > > >> > +#endif > > > >> > + > > > > > If the comments are stripped, how would the compiler see them to be > > > able to issue a warning? I would guess that it is retained or replaced > > > with some other magic token. > > > > Everything that has the warning (GCC-7+/CLANG-9) has that attribute. > > I'd like to make sure we don't regress Coverity most of all. If the > recent updates to the Coverity scanner include support for the attribute > now, then I'm all for it. :)
I want to recant my position on Coverity coverage being a requirement here. While I was originally concerned about suddenly adding thousands more warnings to Coverity scans (if it doesn't support the flag -- I should know soon), it's been made clear to me we're now at the point where this is about to happen for Clang instead (since _it_ doesn't support the comment-style marking and never will but is about to gain C support[1] for the detection -- it only had C++ before). With that out of the way, yes, let's do a mass conversion. As mentioned before, I think "fallthrough;" should be used here (to match "break;"). Let's fork the C language. :) -Kees [1] https://reviews.llvm.org/D64838 -- Kees Cook
