From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
To: Dan <dan.carpenter@oracle.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
kys@microsoft.com, olaf@aepfle.de, sthemmin@microsoft.com,
jasowang@redhat.com, linux-kernel <linux-kernel@vger.kernel.org>,
Stable@vger.kernel.org, Michael.H.Kelley@microsoft.com,
Robo Bot <apw@canonical.com>,
devel@linuxdriverproject.org, vkuznets@redhat.com,
haiyangz@microsoft.com
Subject: Re: [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up
Date: Sat, 20 Oct 2018 16:42:07 +0200 [thread overview]
Message-ID: <CANiq72kQwhOFC5=vaNZW9qpmohot3ATRJVnn95A8eJ2w2xCNMQ@mail.gmail.com> (raw)
In-Reply-To: <20181017062255.oiu44y4zuuwilan3@mwanda>
+On Wed, Oct 17, 2018 at 8:25 AM Dan Carpenter <dan.carpenter@oracle.com> 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.
* clang 7 supports -Wimplicit-fallthrough (not enabled in
-Wall/extra/pedantic like gcc, though) but *only* in C++ mode and with
the C++ syntax [[fallthrough]]. In other words, in C mode, no syntax
works and no diagnostics are emitted. It complains about
Wmissing-declarations. [IMO they should allow the __attribute__ syntax
for fallthrough (and enable it on C mode) to be compatible with gcc.
Maybe they are simply waiting for the C2x attributes... :-)]
* icc 19 does not know about -Wimplicit-fallthrough at all (but
seems to allow [[fallthrough]] on C++17 mode to comply with the
standard).
Therefore, the only improvement we could do right now is starting to
use the attribute for gcc > 7.1, and a comment for everybody else.
However, even if that was worth the trouble of changing the 2500+
instances of fall through markings that we have, comments are replaced
before the preprocessor stage, so we would need some (probably
non-portable) macro magic.
So, I would say, let's revisit this again in a few years. Possibly,
when we move the minimum version to gcc 7.1, clang and icc may both
support the fallthrough warning in C mode; so that we can always use
the __attribute__((fallthrough)) unconditionally under a __fallthrough
#define.
Cheers,
Miguel
next prev parent reply other threads:[~2018-10-20 14:42 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-17 3:12 [PATCH 0/5] Drivers: hv: Miscellaneous fixes kys
2018-10-17 3:14 ` [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context kys
2018-10-17 3:14 ` [PATCH 2/5] hv_utils: update name in struct hv_driver util_drv kys
2018-10-17 3:14 ` [PATCH 3/5] Drivers: hv: kvp: Fix the recent regression caused by incorrect clean-up kys
2018-10-17 5:07 ` Greg KH
2018-10-17 5:11 ` Gustavo A. R. Silva
2018-10-17 6:02 ` KY Srinivasan
2018-10-17 17:56 ` Dexuan Cui
2018-10-17 6:22 ` Dan Carpenter
2018-10-20 14:42 ` Miguel Ojeda [this message]
2018-10-20 19:22 ` Dan Carpenter
2018-10-21 4:15 ` Miguel Ojeda
2018-10-17 18:01 ` Dexuan Cui
2018-10-17 3:14 ` [PATCH 4/5] Drivers: hv: kvp: Use %u to print U32 kys
2018-10-17 5:04 ` Greg KH
2018-10-17 5:59 ` KY Srinivasan
2018-10-17 3:14 ` [PATCH 5/5] Tools: hv: kvp: Fix a warning of buffer overflow with gcc 8.0.1 kys
2018-10-17 5:07 ` Greg KH
2018-10-17 19:57 ` Dexuan Cui
2018-10-17 5:04 ` [PATCH 1/5] Drivers: hv: vmbus: Get rid of unnecessary state in hv_context Greg KH
2018-10-17 5:59 ` KY Srinivasan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANiq72kQwhOFC5=vaNZW9qpmohot3ATRJVnn95A8eJ2w2xCNMQ@mail.gmail.com' \
--to=miguel.ojeda.sandonis@gmail.com \
--cc=Michael.H.Kelley@microsoft.com \
--cc=Stable@vger.kernel.org \
--cc=apw@canonical.com \
--cc=dan.carpenter@oracle.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=haiyangz@microsoft.com \
--cc=jasowang@redhat.com \
--cc=kys@microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=olaf@aepfle.de \
--cc=sthemmin@microsoft.com \
--cc=vkuznets@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).