Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up

From: Dan Carpenter
Date: Sat Oct 20 2018 - 15:22:50 EST


On Sat, Oct 20, 2018 at 04:42:07PM +0200, Miguel Ojeda wrote:
> +On Wed, Oct 17, 2018 at 8:25 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >
> > It's not common at all. It should be wrapped in a macro and put into
> > compiler.h.
> >
> > But I hope it does become adopted. It's better than randomly grepping
> > for non-standard comments.
>
> Using an attribute is indeed better whenever possible. In C++17 it is
> an standard attribute and there have been proposals to include some of
> them for C as well since 2016 at least, e.g. the latest for
> fallthrough at:
>
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf
>
> I have taken a quick look into supporting it (typing it here to save
> it on the mailing list :-), and we have:
>
> * gcc >= 7.1 supports -Wimplicit-fallthrough with
> __attribute__((fallthrough)), the comment syntax and the C++
> [[fallthrough]] syntax.
> * gcc < 7.1 complains about empty declarations (it does not know
> about attributes for null statements) and also
> -Wdeclaration-after-statement.

I'm not sure I understand about empty decalarations. The idea is that
we would do:

case 3:
frob();
__fall_through();
case 4:
frob();

#if GCC > 7.1
#define __fall_through() __attribute__((fallthrough))
#else
#define __fall_through()
#endif

So long as GCC and static analysis tools understand about the attribute
that's enought to catch the bugs. No one cares what clang and icc do.
We would just disable the fall through warnings on those until they are
updated to support the fallthrough attribute.

We wouldn't update all the fall through comments until later, but going
forward people could use the __fall_through() macro if they want.

regards,
dan carpenter