Thread (19 messages) 19 messages, 11 authors, 2020-10-20

Re: [RFC] treewide: cleanup unreachable breaks

From: Joe Perches <joe@perches.com>
Date: 2020-10-20 18:42:58
Also in: alsa-devel, amd-gfx, bpf, dri-devel, intel-wired-lan, keyrings, linux-acpi, linux-amlogic, linux-block, linux-can, linux-crypto, linux-edac, linux-gpio, linux-iio, linux-integrity, linux-media, linux-pci, linux-pm, linux-samsung-soc, linux-scsi, linux-security-module, linux-serial, linux-usb, linux-watchdog, linux-wireless, lkml, nouveau, nvdimm, ocfs2-devel, platform-driver-x86, virtualization, xen-devel

On Mon, 2020-10-19 at 12:42 -0700, Nick Desaulniers wrote:
On Sat, Oct 17, 2020 at 10:43 PM Greg KH [off-list ref] wrote:
quoted
On Sat, Oct 17, 2020 at 09:09:28AM -0700, trix@redhat.com wrote:
quoted
From: Tom Rix <trix@redhat.com>

This is a upcoming change to clean up a new warning treewide.
I am wondering if the change could be one mega patch (see below) or
normal patch per file about 100 patches or somewhere half way by collecting
early acks.
Please break it up into one-patch-per-subsystem, like normal, and get it
merged that way.

Sending us a patch, without even a diffstat to review, isn't going to
get you very far...
Tom,
If you're able to automate this cleanup, I suggest checking in a
script that can be run on a directory.  Then for each subsystem you
can say in your commit "I ran scripts/fix_whatever.py on this subdir."
 Then others can help you drive the tree wide cleanup.  Then we can
enable -Wunreachable-code-break either by default, or W=2 right now
might be a good idea.

Ah, George (gbiv@, cc'ed), did an analysis recently of
`-Wunreachable-code-loop-increment`, `-Wunreachable-code-break`, and
`-Wunreachable-code-return` for Android userspace.  From the review:
Spoilers: of these, it seems useful to turn on
-Wunreachable-code-loop-increment and -Wunreachable-code-return by
default for Android
...
While these conventions about always having break arguably became
obsolete when we enabled -Wfallthrough, my sample turned up zero
potential bugs caught by this warning, and we'd need to put a lot of
effort into getting a clean tree. So this warning doesn't seem to be
worth it.
Looks like there's an order of magnitude of `-Wunreachable-code-break`
than the other two.

We probably should add all 3 to W=2 builds (wrapped in cc-option).
I've filed https://github.com/ClangBuiltLinux/linux/issues/1180 to
follow up on.
I suggest using W=1 as people that are doing cleanups
generally use that and not W=123 or any other style.

Every other use of W= is still quite noisy and these
code warnings are relatively trivially to fix up.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help