From: Edward Cree <ecree.xilinx@gmail.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: Kees Cook <keescook@chromium.org>,
Jakub Kicinski <kuba@kernel.org>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
alsa-devel@alsa-project.org, amd-gfx@lists.freedesktop.org,
bridge@lists.linux-foundation.org, ceph-devel@vger.kernel.org,
cluster-devel@redhat.com, coreteam@netfilter.org,
devel@driverdev.osuosl.org, dm-devel@redhat.com,
drbd-dev@lists.linbit.com, dri-devel@lists.freedesktop.org,
GR-everest-linux-l2@marvell.com, GR-Linux-NIC-Dev@marvell.com,
intel-gfx@lists.freedesktop.org,
intel-wired-lan@lists.osuosl.org, keyrings@vger.kernel.org,
linux1394-devel@lists.sourceforge.net,
linux-acpi@vger.kernel.org, linux-afs@lists.infradead.org,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
linux-arm-msm@vger.kernel.org,
linux-atm-general@lists.sourceforge.net,
linux-block@vger.kernel.org, linux-can@vger.kernel.org,
linux-cifs@vger.kernel.org,
Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
linux-decnet-user@lists.sourceforge.net,
Ext4 Developers List <linux-ext4@vger.kernel.org>,
linux-fbdev@vger.kernel.org, linux-geode@lists.infradead.org,
linux-gpio@vger.kernel.org, linux-hams@vger.kernel.org,
linux-hwmon@vger.kernel.org, linux-i3c@lists.infradead.org,
linux-ide@vger.kernel.org, linux-iio@vger.kernel.org,
linux-input <linux-input@vger.kernel.org>,
linux-integrity@vger.kernel.org,
linux-mediatek@lists.infradead.org,
Linux Media Mailing List <linux-media@vger.kernel.org>,
linux-mmc@vger.kernel.org, Linux-MM <linux-mm@kvack.org>,
linux-mtd@lists.infradead.org, linux-nfs@vger.kernel.org,
linux-rdma@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-sctp@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-usb@vger.kernel.org, linux-watchdog@vger.kernel.org,
linux-wireless <linux-wireless@vger.kernel.org>,
Network Development <netdev@vger.kernel.org>,
netfilter-devel@vger.kernel.org, nouveau@lists.freedesktop.org,
op-tee@lists.trustedfirmware.org, oss-drivers@netronome.com,
patches@opensource.cirrus.com, rds-devel@oss.oracle.com,
reiserfs-devel@vger.kernel.org, samba-technical@lists.samba.org,
selinux@vger.kernel.org, target-devel@vger.kernel.org,
tipc-discussion@lists.sourceforge.net,
usb-storage@lists.one-eyed-alien.net,
virtualization@lists.linux-foundation.org,
wcn36xx@lists.infradead.org,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<x86@kernel.org>,
xen-devel@lists.xenproject.org, linux-hardening@vger.kernel.org,
Nick Desaulniers <ndesaulniers@google.com>,
Nathan Chancellor <natechancellor@gmail.com>,
Miguel Ojeda <ojeda@kernel.org>, Joe Perches <joe@perches.com>
Subject: Re: [PATCH 000/141] Fix fall-through warnings for Clang
Date: Wed, 25 Nov 2020 22:44:35 +0000 [thread overview]
Message-ID: <44005bde-f6d4-5eaa-39b8-1a5efeedb2d3@gmail.com> (raw)
In-Reply-To: <CANiq72kqO=bYMJnFS2uYRpgWATJ=uXxZuNUsTXT+3aLtrpnzvQ@mail.gmail.com>
On 25/11/2020 00:32, Miguel Ojeda wrote:
> I have said *authoring* lines of *this* kind takes a minute per line.
> Specifically: lines fixing the fallthrough warning mechanically and
> repeatedly where the compiler tells you to, and doing so full-time for
> a month.
<snip>
> It is useful since it makes intent clear.
To make the intent clear, you have to first be certain that you
understand the intent; otherwise by adding either a break or a
fallthrough to suppress the warning you are just destroying the
information that "the intent of this code is unknown".
Figuring out the intent of a piece of unfamiliar code takes more
than 1 minute; just because
case foo:
thing;
case bar:
break;
produces identical code to
case foo:
thing;
break;
case bar:
break;
doesn't mean that *either* is correct — maybe the author meant
to write
case foo:
return thing;
case bar:
break;
and by inserting that break you've destroyed the marker that
would direct someone who knew what the code was about to look
at that point in the code and spot the problem.
Thus, you *always* have to look at more than just the immediate
mechanical context of the code, to make a proper judgement that
yes, this was the intent. If you think that that sort of thing
can be done in an *average* time of one minute, then I hope you
stay away from code I'm responsible for!
One minute would be an optimistic target for code that, as the
maintainer, one is already somewhat familiar with. For code
that you're seeing for the first time, as is usually the case
with the people doing these mechanical fix-a-warning patches,
it's completely unrealistic.
A warning is only useful because it makes you *think* about the
code. If you suppress the warning without doing that thinking,
then you made the warning useless; and if the warning made you
think about code that didn't *need* it, then the warning was
useless from the start.
So make your mind up: does Clang's stricter -Wimplicit-fallthrough
flag up code that needs thought (in which case the fixes take
effort both to author and to review) or does it flag up code
that can be mindlessly "fixed" (in which case the warning is
worthless)? Proponents in this thread seem to be trying to
have it both ways.
-ed
next prev parent reply other threads:[~2020-11-25 22:44 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-20 18:21 [PATCH 000/141] Fix fall-through warnings for Clang Gustavo A. R. Silva
2020-11-20 18:28 ` Joe Perches
2020-11-20 19:02 ` Gustavo A. R. Silva
2020-11-20 18:30 ` [PATCH 041/141] mm: " Gustavo A. R. Silva
2020-11-20 18:53 ` [PATCH 000/141] " Jakub Kicinski
2020-11-20 19:04 ` Gustavo A. R. Silva
2020-11-20 19:30 ` Kees Cook
2020-11-20 19:51 ` Jakub Kicinski
2020-11-20 20:48 ` Kees Cook
2020-11-22 16:17 ` Kees Cook
2020-11-22 18:21 ` James Bottomley
2020-11-22 18:25 ` Joe Perches
2020-11-22 19:12 ` James Bottomley
2020-11-22 19:22 ` Joe Perches
2020-11-22 19:53 ` James Bottomley
2020-11-23 13:03 ` [Intel-wired-lan] " Gustavo A. R. Silva
2020-11-23 16:31 ` James Bottomley
2020-11-24 21:32 ` Kees Cook
2020-11-24 22:24 ` Finn Thain
2020-11-24 23:15 ` Miguel Ojeda
2020-11-24 23:53 ` Finn Thain
2020-11-25 1:05 ` Miguel Ojeda
2020-11-25 7:05 ` James Bottomley
2020-11-25 12:24 ` Nick Desaulniers
2020-11-25 16:24 ` Jakub Kicinski
2020-11-25 17:04 ` Miguel Ojeda
2020-11-25 22:09 ` Nick Desaulniers
2020-11-25 21:33 ` Finn Thain
2020-11-25 22:09 ` Nick Desaulniers
2020-11-25 23:21 ` Finn Thain
2020-11-26 0:30 ` Finn Thain
2020-11-25 21:10 ` Kees Cook
2020-11-22 20:35 ` Miguel Ojeda
2020-11-22 22:36 ` James Bottomley
2020-11-23 14:19 ` Miguel Ojeda
2020-11-23 15:58 ` James Bottomley
2020-11-23 16:24 ` Rafael J. Wysocki
2020-11-23 16:32 ` Joe Perches
2020-11-23 18:56 ` Miguel Ojeda
2020-11-23 20:37 ` James Bottomley
2020-11-25 0:32 ` Miguel Ojeda
2020-11-25 22:44 ` Edward Cree [this message]
2020-11-26 14:53 ` Miguel Ojeda
2020-11-26 15:28 ` Geert Uytterhoeven
2020-11-26 16:18 ` Karol Herbst
2020-11-26 17:05 ` Miguel Ojeda
2020-11-25 10:38 ` Andy Shevchenko
2020-11-25 9:01 ` Sean Young
2020-11-22 22:54 ` Finn Thain
2020-11-22 23:04 ` James Bottomley
2020-11-23 14:05 ` Miguel Ojeda
2020-11-24 0:58 ` Finn Thain
2020-11-24 1:05 ` Joe Perches
2020-11-24 2:48 ` Finn Thain
2020-11-24 23:46 ` Miguel Ojeda
2020-11-24 1:32 ` Nick Desaulniers
2020-11-24 1:46 ` Jakub Kicinski
2020-11-24 21:25 ` Kees Cook
2020-11-25 23:02 ` Edward Cree
2020-12-01 14:08 ` Dan Carpenter
2020-12-01 14:04 ` Dan Carpenter
2020-11-20 22:21 ` Miguel Ojeda
2020-11-23 20:03 ` Jason Gunthorpe
2020-11-24 14:47 ` Gustavo A. R. Silva
2020-11-23 20:38 ` Mark Brown
2020-11-24 14:47 ` Gustavo A. R. Silva
2020-12-01 5:52 ` Martin K. Petersen
2020-12-01 8:20 ` Gustavo A. R. Silva
2020-12-08 4:52 ` (subset) " Martin K. Petersen
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=44005bde-f6d4-5eaa-39b8-1a5efeedb2d3@gmail.com \
--to=ecree.xilinx@gmail.com \
--cc=GR-Linux-NIC-Dev@marvell.com \
--cc=GR-everest-linux-l2@marvell.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=alsa-devel@alsa-project.org \
--cc=amd-gfx@lists.freedesktop.org \
--cc=bridge@lists.linux-foundation.org \
--cc=ceph-devel@vger.kernel.org \
--cc=cluster-devel@redhat.com \
--cc=coreteam@netfilter.org \
--cc=devel@driverdev.osuosl.org \
--cc=dm-devel@redhat.com \
--cc=drbd-dev@lists.linbit.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=gustavoars@kernel.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=joe@perches.com \
--cc=keescook@chromium.org \
--cc=keyrings@vger.kernel.org \
--cc=kuba@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-afs@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-atm-general@lists.sourceforge.net \
--cc=linux-block@vger.kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-decnet-user@lists.sourceforge.net \
--cc=linux-ext4@vger.kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-geode@lists.infradead.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-hams@vger.kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-i3c@lists.infradead.org \
--cc=linux-ide@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-mm@kvack.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=linux-sctp@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux-usb@vger.kernel.org \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=linux1394-devel@lists.sourceforge.net \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=natechancellor@gmail.com \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=nouveau@lists.freedesktop.org \
--cc=ojeda@kernel.org \
--cc=op-tee@lists.trustedfirmware.org \
--cc=oss-drivers@netronome.com \
--cc=patches@opensource.cirrus.com \
--cc=rds-devel@oss.oracle.com \
--cc=reiserfs-devel@vger.kernel.org \
--cc=samba-technical@lists.samba.org \
--cc=selinux@vger.kernel.org \
--cc=target-devel@vger.kernel.org \
--cc=tipc-discussion@lists.sourceforge.net \
--cc=usb-storage@lists.one-eyed-alien.net \
--cc=virtualization@lists.linux-foundation.org \
--cc=wcn36xx@lists.infradead.org \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/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).