[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 08/11] xen/compiler: import 'fallthrough' keyword from linux

  • To: Stefano Stabellini <sstabellini@xxxxxxxxxx>
  • From: Rahul Singh <Rahul.Singh@xxxxxxx>
  • Date: Fri, 15 Jan 2021 12:14:17 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=LLLvG005XmfcSyFhb1Tq+qVKrm0fXQ65rYQRcjoLWcE=; b=OH7xSaj+CjyyDaG6uVv82JW/jX1CuukF/tpH9SoJhPEDHP6y7pRpMqv0gkixY2XZcaQBzzXVdnWdbDmRaY2ELCW1WWCnxsPnNllKIPNOJYQri9q+P5Y+yzX9b86GBWPtnI+EsiJARq6PrySfNb9L/cB2BOFARv5wUQLZ+qxfZxnoDdkcATwU48+ds8dsCH+iAEj4nAUL51Q6+2wJKXsy4clL+GbKFNS4H8Gpuyerpi2uiQ8X23YRL9NGYwcosAPBn7MGOaan7p9ibBvKoxw4UfegAoI+8eX1+bkJLqUR/8xuuvINinUYr9LL0PwV73blQLAKIrQ3uJmYSzPIYlDGCw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UtWPhPsDFMMOrp8Tsba4+8++Pr/XobWMq59SOAKK2ANypgaDtznU2YS4gTW0+d1nPG2sHYlUhOxTMH/MwsJsXZzSSL0F3ysntZCK8Ptmsel4oQSk9QdGEulYYjrvaQ/j4RmFtS7C1c4RQ9Gt3KI0efrj6BJr/0KQ5yRvo2NYy8JuYUmBmJ9TQ4VVTeMjdn6K4U3dfSfGGfHpqzxjIzjGdcRUNrj/0Aeq8BIR/OzMzsakiHwIiX+Y8/sldvGfQ/UPE0bte2D5ATPTNtHDaasMZ5DSbq+wcAfmmSpRr05Z4nV3DoLY46btu8VvfC4k9m9qIR20tCGm7UADizbPp0C7lg==
  • Authentication-results-original: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Cc: Jan Beulich <jbeulich@xxxxxxxx>, Bertrand Marquis <Bertrand.Marquis@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Fri, 15 Jan 2021 12:14:48 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: kernel.org; dkim=none (message not signed) header.d=none;kernel.org; dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHW5c272wDpW7M2fUCol5kPEXm0sqoj5EGAgADGGoCAAjJ8AIAA9tOAgADQnoA=
  • Thread-topic: [PATCH v4 08/11] xen/compiler: import 'fallthrough' keyword from linux


> On 14 Jan 2021, at 11:47 pm, Stefano Stabellini <sstabellini@xxxxxxxxxx> 
> wrote:
> 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? 

No I don’t have access to Coverity to run a test.What I found out that from the 
Linux kernel mailing list Coverity understand the 
"__attribute__((__fallthrough__))” keyword.
If someone else can run a Coverity test than it will be very helpful.

[1] https://lore.kernel.org/lkml/20181021182926.GB6683@xxxxxxxxx/
[2] https://lore.kernel.org/patchwork/patch/1108577/




Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.