On Sat, 2020-10-17 at 20:21 +0200, Julia Lawall wrote:
On Sat, 17 Oct 2020, Joe Perches wrote:
On Sat, 2020-10-17 at 09:09 -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.
clang has a number of useful, new warnings see https://clang.llvm.org/docs/DiagnosticsReference.html
This change cleans up -Wunreachable-code-break https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-brea... for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
Early acks/individual patches by subsystem would be good. Better still would be an automated cocci script.
Coccinelle is not especially good at this, because it is based on control flow, and a return or goto diverts the control flow away from the break. A hack to solve the problem is to put an if around the return or goto, but that gives the break a meaningless file name and line number. I collected the following list, but it only has 439 results, so fewer than clang. But maybe there are some files that are not considered by clang in the x86 allyesconfig configuration.
Probably checkpatch is the best solution here, since it is not configuration sensitive and doesn't care about control flow.
Likely the clang compiler is the best option here.
It might be useful to add -Wunreachable-code-break to W=1 or just always enable it if it isn't already enabled.
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 95e4cdb94fe9..3819787579d5 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) +KBUILD_CFLAGS += $(call cc-option, -Wunreachable-code-break) # The following turn off the warnings enabled by -Wextra KBUILD_CFLAGS += -Wno-missing-field-initializers KBUILD_CFLAGS += -Wno-sign-compare
(and thank you Tom for pushing this forward)
checkpatch can't find instances like:
case FOO: if (foo) return 1; else return 2; break;
As it doesn't track flow and relies on the number of tabs to be the same for any goto/return and break;
checkpatch will warn on:
case FOO: ... goto bar; break;