* Re: [PATCH] checkpatch: skip macros when finding missing switch/case break
@ 2020-08-01 5:03 Cambda Zhu
2020-08-01 16:05 ` [PATCH] checkpatch: Remove missing switch/case break test Joe Perches
0 siblings, 1 reply; 4+ messages in thread
From: Cambda Zhu @ 2020-08-01 5:03 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel
I agree with you. Actually there are some false warnings not mentioned in my patch, such as:
case xxx: {
if () {
return;
} else {
if () {
return;
} else {
return;
}
}
}
case xxx:
...
Since compiler can do this now, I think this test should be removed.
Regards,
Cambda
> 在 2020年8月1日，02:05，Joe Perches <joe@perches.com> 写道：
>
> On Wed, 2020-07-29 at 20:59 +0800, Cambda Zhu wrote:
>> The checkpatch.pl only searches 3 previous lines when finding missing
>> switch/case break, and macros are treated as normal statements. If the
>> cases are surrounded with CONFIG, checkpatch.pl may report false
>> warnings. For example:
>
> Likely this test should be removed altogether as
> it's never really worked well and now compilers
> find this and emit warnings.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] checkpatch: Remove missing switch/case break test
2020-08-01 5:03 [PATCH] checkpatch: skip macros when finding missing switch/case break Cambda Zhu
@ 2020-08-01 16:05 ` Joe Perches
0 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2020-08-01 16:05 UTC (permalink / raw)
To: Andrew Morton, Andy Whitcroft; +Cc: linux-kernel, Cambda Zhu
This test doesn't work well and newer compilers are much better
at emitting this warning.
Signed-off-by: Joe Perches <joe@perches.com>
---
> 在 2020年8月1日，02:05，Joe Perches <joe@perches.com> 写道：
> > On Wed, 2020-07-29 at 20:59 +0800, Cambda Zhu wrote:
> > > The checkpatch.pl only searches 3 previous lines when finding missing
> > > switch/case break, and macros are treated as normal statements. If the
> > > cases are surrounded with CONFIG, checkpatch.pl may report false
> > > warnings. For example:
> >
> > Likely this test should be removed altogether as
> > it's never really worked well and now compilers
> > find this and emit warnings.
scripts/checkpatch.pl | 25 -------------------------
1 file changed, 25 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cc5542cc234f..4aa1d9d5e62c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6541,31 +6541,6 @@ sub process {
}
}
-# check for case / default statements not preceded by break/fallthrough/switch
- if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) {
- my $has_break = 0;
- my $has_statement = 0;
- my $count = 0;
- my $prevline = $linenr;
- while ($prevline > 1 && ($file || $count < 3) && !$has_break) {
- $prevline--;
- my $rline = $rawlines[$prevline - 1];
- my $fline = $lines[$prevline - 1];
- last if ($fline =~ /^\@\@/);
- next if ($fline =~ /^\-/);
- next if ($fline =~ /^.(?:\s*(?:case\s+(?:$Ident|$Constant)[\s$;]*|default):[\s$;]*)*$/);
- $has_break = 1 if ($rline =~ /fall[\s_-]*(through|thru)/i);
- next if ($fline =~ /^.[\s$;]*$/);
- $has_statement = 1;
- $count++;
- $has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|exit\s*\(\b|return\b|goto\b|continue\b)/);
- }
- if (!$has_break && $has_statement) {
- WARN("MISSING_BREAK",
- "Possible switch case/default not preceded by break or fallthrough comment\n" . $herecurr);
- }
- }
-
# check for /* fallthrough */ like comment, prefer fallthrough;
my @fallthroughs = (
'fallthrough',
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] checkpatch: skip macros when finding missing switch/case break
2020-07-29 12:59 [PATCH] checkpatch: skip macros when finding missing switch/case break Cambda Zhu
@ 2020-07-31 18:05 ` Joe Perches
0 siblings, 0 replies; 4+ messages in thread
From: Joe Perches @ 2020-07-31 18:05 UTC (permalink / raw)
To: Cambda Zhu; +Cc: linux-kernel
On Wed, 2020-07-29 at 20:59 +0800, Cambda Zhu wrote:
> The checkpatch.pl only searches 3 previous lines when finding missing
> switch/case break, and macros are treated as normal statements. If the
> cases are surrounded with CONFIG, checkpatch.pl may report false
> warnings. For example:
Likely this test should be removed altogether as
it's never really worked well and now compilers
find this and emit warnings.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] checkpatch: skip macros when finding missing switch/case break
@ 2020-07-29 12:59 Cambda Zhu
2020-07-31 18:05 ` Joe Perches
0 siblings, 1 reply; 4+ messages in thread
From: Cambda Zhu @ 2020-07-29 12:59 UTC (permalink / raw)
To: Joe Perches; +Cc: linux-kernel, Cambda Zhu
The checkpatch.pl only searches 3 previous lines when finding missing
switch/case break, and macros are treated as normal statements. If the
cases are surrounded with CONFIG, checkpatch.pl may report false
warnings. For example:
\+#if xxx
\+ case xxx: {
\+ ...
\+ break/return/...;
\+ }
\+#endif
\+#if xxx
\+ case xxx:
\+ ...
\+#endif
This patch skips lines starting with whitespaces and #, so the counter
of previous statements won't increase in these cases.
Signed-off-by: Cambda Zhu <cambda@linux.alibaba.com>
---
scripts/checkpatch.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4c820607540b..2c0a51ac82a7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -6503,6 +6503,7 @@ sub process {
next if ($fline =~ /^.(?:\s*(?:case\s+(?:$Ident|$Constant)[\s$;]*|default):[\s$;]*)*$/);
$has_break = 1 if ($rline =~ /fall[\s_-]*(through|thru)/i);
next if ($fline =~ /^.[\s$;]*$/);
+ next if ($fline =~ /^.\s*#/);
$has_statement = 1;
$count++;
$has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|exit\s*\(\b|return\b|goto\b|continue\b)/);
--
2.16.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-01 16:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01 5:03 [PATCH] checkpatch: skip macros when finding missing switch/case break Cambda Zhu
2020-08-01 16:05 ` [PATCH] checkpatch: Remove missing switch/case break test Joe Perches
-- strict thread matches above, loose matches on Subject: below --
2020-07-29 12:59 [PATCH] checkpatch: skip macros when finding missing switch/case break Cambda Zhu
2020-07-31 18:05 ` Joe Perches
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).