checkpatch: Improve missing break for switch/case tests

Commit Message

Joe Perches April 11, 2016, 2:26 a.m. UTC

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 <[e-mail]>
Reported-by: Lars-Peter Clausen <[e-mail]>
---
 scripts/checkpatch.pl | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Joe Perches April 11, 2016, 5:04 a.m. UTC | #1

On Sun, 2016-04-10 at 19:26 -0700, Joe Perches wrote:
> 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.

Please ignore, the patch is defective as
it has too many false positives.

Patch

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);
 			}
 		}