[PATCH] checkpatch: Reduce false positives for "Missing blank line after declarations" test

[PATCH] checkpatch: Reduce false positives for "Missing blank line after declarations" test

[Joe Perches]

Avoid some false positives for this test by adding
a few common macro types that declare variables.

	DECLARE_<FOO>
	DEFINE_<BAR>
	LIST_HEAD

Also allow extended bitfield declarations like

	int a:5,
	    b:2,
	    c:1;

and avoid emitting this warning on declarations like

	int foo[bar]
	       [baz];

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d44e440..98e0be7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -388,6 +388,11 @@ our @mode_permission_funcs = (
 	["(?:CLASS|DEVICE|SENSOR)_ATTR", 2],
 );
 
+our $declaration_macros = qr{(?x:
+	(?:$Storage\s+)?(?:DECLARE|DEFINE)_[A-Z]+\s*\(|
+	(?:$Storage\s+)?LIST_HEAD\s*\(
+)};
+
 our $allowed_asm_includes = qr{(?x:
 	irq|
 	memory
@@ -2251,14 +2256,18 @@ sub process {
 		}
 
 # check for missing blank lines after declarations
-		if ($prevline =~ /^\+\s+$Declare\s+$Ident/ &&
+		if ($sline =~ /^\+\s+\S/ &&			#Not at char 1
+		    ($prevline =~ /^\+\s+$Declare\s+$Ident/ ||
+		     $prevline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident/ ||
+		     $prevline =~ /^\+\s+$declaration_macros/) &&
 		    !($prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ ||
 		      $prevline =~ /(?:\{\s*|\\)$/) &&		#extended lines
-		    $sline =~ /^\+\s+/ &&			#Not at char 1
 		    !($sline =~ /^\+\s+$Declare/ ||
-		      $sline =~ /^\+\s+$Ident\s+$Ident/ ||	#eg: typedef foo
+		      $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident/ ||	#eg: typedef foo
+		      $sline =~ /^\+\s+$declaration_macros/ ||
 		      $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ ||
-		      $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(])/ ||
+		      $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ ||
+		      $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ ||	#bitfield
 		      $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) {
 			WARN("SPACING",
 			     "Missing a blank line after declarations\n" . $hereprev);

Re: [PATCH] checkpatch: Reduce false positives for "Missing blank line after declarations" test

[Joe Perches]

On Mon, 2014-03-17 at 11:45 -0700, Joe Perches wrote:
> Avoid some false positives for this test by adding
> a few common macro types that declare variables.

Self-NAK.  It causes too many other false positives.

I'll play with it awhile and test it better before
posting something else.

[PATCH V2] checkpatch: Reduce false positives for "Missing blank line after declarations" test

[Joe Perches]

Reduce the false positives of this test by checking
previous lines for known macros that declare variables.

Add c90 keywords and test those too.

Comment the specific tests done a bit better.

Signed-off-by: Joe Perches <joe@perches.com>
---

V2: I believe this one works better.
    Don't allow "else if" to be an $Ident $Ident match
    Skip bitfield continuations too

 scripts/checkpatch.pl | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index d44e440..93beee8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -303,6 +303,7 @@ our $Operators	= qr{
 			=>|->|<<|>>|<|>|!|~|
 			&&|\|\||,|\^|\+\+|--|&|\||$Arithmetic
 		  }x;
+our $c90_Keywords = qr{do|for|while|if|else|return|goto|continue|switch|default|case|break}x;
 
 our $NonptrType;
 our $NonptrTypeWithAttr;
@@ -388,6 +389,11 @@ our @mode_permission_funcs = (
 	["(?:CLASS|DEVICE|SENSOR)_ATTR", 2],
 );
 
+our $declaration_macros = qr{(?x:
+	(?:$Storage\s+)?(?:DECLARE|DEFINE)_[A-Z]+\s*\(|
+	(?:$Storage\s+)?LIST_HEAD\s*\(
+)};
+
 our $allowed_asm_includes = qr{(?x:
 	irq|
 	memory
@@ -2251,14 +2257,32 @@ sub process {
 		}
 
 # check for missing blank lines after declarations
-		if ($prevline =~ /^\+\s+$Declare\s+$Ident/ &&
-		    !($prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ ||
-		      $prevline =~ /(?:\{\s*|\\)$/) &&		#extended lines
-		    $sline =~ /^\+\s+/ &&			#Not at char 1
+		if ($sline =~ /^\+\s+\S/ &&			#Not at char 1
+			# actual declarations
+		    ($prevline =~ /^\+\s+$Declare\s+$Ident\s*[=,;\[]/ ||
+			# foo bar; where foo is some local typedef or #define
+		     $prevline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ ||
+			# known declaration macros
+		     $prevline =~ /^\+\s+$declaration_macros/) &&
+			# for "else if" which can look like "$Ident $Ident"
+		    !($prevline =~ /^\+\s+$c90_Keywords\b/ ||
+			# other possible extensions of declaration lines
+		      $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ ||
+			# not starting a section or a macro "\" extended line
+		      $prevline =~ /(?:\{\s*|\\)$/) &&
+			# looks like a declaration
 		    !($sline =~ /^\+\s+$Declare/ ||
-		      $sline =~ /^\+\s+$Ident\s+$Ident/ ||	#eg: typedef foo
+			# foo bar; where foo is some local typedef or #define
+		      $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ ||
+			# known declaration macros
+		      $sline =~ /^\+\s+$declaration_macros/ ||
+			# start of struct or union or enum
 		      $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ ||
-		      $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(])/ ||
+			# start or end of block or continuation of declaration
+		      $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ ||
+			# bitfield continuation
+		      $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ ||
+			# other possible extensions of declaration lines
 		      $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) {
 			WARN("SPACING",
 			     "Missing a blank line after declarations\n" . $hereprev);
-- 
1.8.1.2.459.gbcd45b4.dirty