[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 08/11] xen/compiler: import 'fallthrough' keyword from linux
On Thu, 14 Jan 2021, Jan Beulich wrote: > On 13.01.2021 00:30, Stefano Stabellini wrote: > > On Tue, 12 Jan 2021, Jan Beulich wrote: > >> On 08.01.2021 15:46, Rahul Singh wrote: > >>> -Wimplicit-fallthrough warns when a switch case falls through. Warning > >>> can be suppress by either adding a /* fallthrough */ comment, or by > >>> using a null statement: __attribute__ ((fallthrough)) > >> > >> Why is the comment variant (which we use in many places already, > >> albeit with varying wording) not the route of choice? > > > > See previous discussion: > > > > https://marc.info/?l=xen-devel&m=160707274517270 > > https://marc.info/?l=xen-devel&m=160733742810605 > > https://marc.info/?l=xen-devel&m=160733852011023 > > > > We thought it would be best to introduce "fallthrough" and only resort > > to comments as a plan B. The usage of the keyword should allow GCC to do > > better checks. > > Hmm, this earlier discussion was on an Arm-specific thread, and I > have to admit I can't see arguments there pro and/or con either > of the two alternatives. > > >>> Define the pseudo keyword 'fallthrough' for the ability to convert the > >>> various case block /* fallthrough */ style comments to null statement > >>> "__attribute__((__fallthrough__))" > >>> > >>> In C mode, GCC supports the __fallthrough__ attribute since 7.1, > >>> the same time the warning and the comment parsing were introduced. > >>> > >>> fallthrough devolves to an empty "do {} while (0)" if the compiler > >>> version (any version less than gcc 7) does not support the attribute. > >> > >> What about Coverity? It would be nice if we wouldn't need to add > >> two separate constructs everywhere to make both compiler and static > >> code checker happy. > > > > I don't think I fully understand your reply here: Coverity doesn't come > > into the picture. Given that GCC provides a special keyword to implement > > fallthrough, it makes sense to use it when available. When it is not > > available (e.g. clang or older GCC) we need to have an alternative to > > suppress the compiler warnings. Hence the need for this check: > > > > #if (!defined(__clang__) && (__GNUC__ >= 7)) > > I'm not sure how this interacts with Coverity. My point bringing up > that one is that whatever gets done here should _also_ result in > Coverity recognizing the fall-through as intentional, or else we'll > end up with many unwanted reports of new issues once the pseudo- > keyword gets made use of. The comment model is what we currently > use to "silence" Coverity; I'd like it to be clear up front that > any new alternative to be used is also going to "satisfy" it. That is a good point, and I agree with that. Rahul, do you have access to a Coverity instance to run a test?
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |