[PATCH] checkpatch: Improve missing break for switch/case tests
From: Joe Perches
Date: Sun Apr 10 2016 - 22:26:40 EST
The current switch/case test doesn't handle ... case labels like:
switch (foo) {
case bar ... baz:
etc...
}
Add a specific regex for that form and the default label.
Use the regex where a case label is tested.
Improve the missing break/fall-through test by only reporting
the first case label missing the break not every case label
where there isn't a preceding break/fall-through.
Show the line above the case label when reporting
the possible defect.
Signed-off-by: Joe Perches <joe@xxxxxxxxxxx>
Reported-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
---
scripts/checkpatch.pl | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e3d9c34..216e4a1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -331,6 +331,11 @@ our $Operators = qr{
}x;
our $c90_Keywords = qr{do|for|while|if|else|return|goto|continue|switch|default|case|break}x;
+our $case_label = qr{
+ (?:case\s+(?:$Ident|$Constant)
+ (?:\s*\.\.\.\s*\s*(?:$Ident|$Constant))? |
+ default)\s*:
+ }x;
our $BasicType;
our $NonptrType;
@@ -4417,7 +4422,7 @@ sub process {
$herecurr);
}
# case and default should not have general statements after them
- if ($line =~ /^.\s*(?:case\s*.*|default\s*):/g &&
+ if ($line =~ /^.\s*$case_label/g &&
$line !~ /\G(?:
(?:\s*$;*)(?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$|
\s*return\s+
@@ -5648,27 +5653,28 @@ sub process {
}
# check for case / default statements not preceded by break/fallthrough/switch
- if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) {
+ if ($line =~ /^.\s*$case_label/ &&
+ $prevline !~ /^.\s*$case_label/) {
my $has_break = 0;
my $has_statement = 0;
my $count = 0;
my $prevline = $linenr;
- while ($prevline > 1 && ($file || $count < 3) && !$has_break) {
+ while ($prevline > 1 && ($file || $count < 3) && !$has_break && !$has_statement) {
$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$;]*)*$/);
+ next if ($fline =~ /^.(?:\s*${case_label}[\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$;]*$|return\b|goto\b|continue\b)/);
+ $has_statement = 1;
}
if (!$has_break && $has_statement) {
WARN("MISSING_BREAK",
- "Possible switch case/default not preceeded by break or fallthrough comment\n" . $herecurr);
+ "Possible switch case/default not preceeded by break or fallthrough comment\n" . $hereprev);
}
}