On 10/19/20 12:42 PM, Nick Desaulniers wrote:
On Sat, Oct 17, 2020 at 10:43 PM Greg KH gregkh@linuxfoundation.org wrote:
On Sat, Oct 17, 2020 at 09:09:28AM -0700, trix@redhat.com wrote:
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.
I should have waited for Joe Perches's fixer addition to checkpatch :)
The easy fixes I did only cover about 1/2 of the problems.
Remaining are mostly nested switches, which from a complexity standpoint is bad.
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
In my simple add-a-cflag bot, i see there are about 250
issues for -Wunreachable-code-return.
I'll see about doing this one next.
... 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.
Yes, i think think these should be added.
Tom