linux-kernel - [PATCH] checkpatch: Improve missing break for switch/case tests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <1460341582.1800.66.camel@perches.com>
Date:	Sun, 10 Apr 2016 19:26:22 -0700
From:	Joe Perches <joe@...ches.com>
To:	Andrew Morton <akpm@...ux-foundation.org>,
	Andy Whitcroft <apw@...dowen.org>
Cc:	"alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Takashi Iwai <tiwai@...e.de>,
	"Harvey, Ryan" <Ryan.Harvey@...rus.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: [PATCH] checkpatch: Improve missing break for switch/case tests

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@...ches.com>
Reported-by: Lars-Peter Clausen <lars@...afoo.de>
---
 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);
 			}
 		}