[PATCH 00/13] checkpatch: update to version 0.24

[PATCH 00/13] checkpatch: update to version 0.24

[Andy Whitcroft]

This update brings a large number of small fixes to existing checks.
Of note:

 - fixes up a number of false type detections,
 - code indent checks handle macros and continuation,
 - complex macro detection handles square bracket nesting correctly, and
 - better comment handling for case.

Complete changelog below.

*** BLURB HERE ***

Andy Whitcroft (13):
  checkpatch: do is not a possible type
  checkpatch: labels are not possible types
  checkpatch: handle do without braces if we have enough context
  checkpatch: macros which define structure members are not complex
  checkpatch: accept any sized le/be type
  checkpatch: pull out known acceptable typedefs
  checkpatch: suspect code indent must stop at #else/#elif
  checkpatch: complex macros checks miss square brackets
  checkpatch: DEFINE_ macros are real definitions for exports
  checkpatch: trailing statements ensure we report the end of the line
  checkpatch: suspect indent handle macro continuation
  checkpatch: allow for comments either side of a brace on case
  checkpatch: version: 0.24

 scripts/checkpatch.pl |  110 +++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 85 insertions(+), 25 deletions(-)

[PATCH 01/13] checkpatch: do is not a possible type

[Andy Whitcroft]

A do without braces '{' may trigger a false possible type 'do' and then this
may be interpreted as an external definition of foo():

	do
		foo();
	while (bar);

Add do to the type exclusions.  Fix up tests so we can check for them.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |   24 ++++++++++++++++++------
 1 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 118fe1f..6b21188 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -913,12 +913,22 @@ sub annotate_values {
 sub possible {
 	my ($possible, $line) = @_;
 
-	print "CHECK<$possible> ($line)\n" if ($dbg_possible > 1);
-	if ($possible !~ /^(?:$Modifier|$Storage|$Type|DEFINE_\S+)$/ &&
-	    $possible ne 'goto' && $possible ne 'return' &&
-	    $possible ne 'case' && $possible ne 'else' &&
-	    $possible ne 'asm' && $possible ne '__asm__' &&
-	    $possible !~ /^(typedef|struct|enum)\b/) {
+	print "CHECK<$possible> ($line)\n" if ($dbg_possible > 2);
+	if ($possible !~ /(?:
+		^(?:
+			$Modifier|
+			$Storage|
+			$Type|
+			DEFINE_\S+|
+			goto|
+			return|
+			case|
+			else|
+			asm|__asm__|
+			do
+		)$|
+		^(?:typedef|struct|enum)\b
+	    )/x) {
 		# Check for modifiers.
 		$possible =~ s/\s*$Storage\s*//g;
 		$possible =~ s/\s*$Sparse\s*//g;
@@ -936,6 +946,8 @@ sub possible {
 			push(@typeList, $possible);
 		}
 		build_types();
+	} else {
+		warn "NOTPOSS: $possible ($line)\n" if ($dbg_possible > 1);
 	}
 }
 
-- 
1.6.0.1.451.gc8d31

[PATCH 02/13] checkpatch: labels are not possible types

[Andy Whitcroft]

A label is not a candidate for a possible type.  Exclude them.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6b21188..17e1d94 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1324,7 +1324,7 @@ sub process {
 				possible($type, "A:" . $s);
 
 			# definitions in global scope can only start with types
-			} elsif ($s =~ /^.(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?($Ident)\b/s) {
+			} elsif ($s =~ /^.(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?($Ident)\b\s*(?!:)/s) {
 				possible($1, "B:" . $s);
 			}
 
-- 
1.6.0.1.451.gc8d31

[PATCH 03/13] checkpatch: handle do without braces if we have enough context

[Andy Whitcroft]

If we have sufficient context detect and handle do without braces ({).
Else these incorrectly trigger a trailing statements error for the
associated while.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 17e1d94..19690a2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1051,6 +1051,7 @@ sub process {
 
 	# suppression flags
 	my %suppress_ifbraces;
+	my %suppress_whiletrailers;
 
 	# Pre-scan the patch sanitizing the lines.
 	# Pre-scan the patch looking for any __setup documentation.
@@ -1156,6 +1157,7 @@ sub process {
 			$prev_values = 'E';
 
 			%suppress_ifbraces = ();
+			%suppress_whiletrailers = ();
 			next;
 
 # track the line number as we move through the hunk, note that
@@ -1301,9 +1303,9 @@ sub process {
 		}
 
 # Check for potential 'bare' types
-		my ($stat, $cond, $line_nr_next, $remain_next);
+		my ($stat, $cond, $line_nr_next, $remain_next, $off_next);
 		if ($realcnt && $line =~ /.\s*\S/) {
-			($stat, $cond, $line_nr_next, $remain_next) =
+			($stat, $cond, $line_nr_next, $remain_next, $off_next) =
 				ctx_statement_block($linenr, $realcnt, 0);
 			$stat =~ s/\n./\n /g;
 			$cond =~ s/\n./\n /g;
@@ -1952,7 +1954,26 @@ sub process {
 
 # Check for illegal assignment in if conditional -- and check for trailing
 # statements after the conditional.
-		if ($line =~ /\b(?:if|while|for)\s*\(/ && $line !~ /^.\s*#/) {
+		if ($line =~ /do\s*(?!{)/) {
+			my ($stat_next) = ctx_statement_block($line_nr_next,
+						$remain_next, $off_next);
+			$stat_next =~ s/\n./\n /g;
+			##print "stat<$stat> stat_next<$stat_next>\n";
+
+			if ($stat_next =~ /^\s*while\b/) {
+				# If the statement carries leading newlines,
+				# then count those as offsets.
+				my ($whitespace) =
+					($stat_next =~ /^((?:\s*\n[+-])*\s*)/s);
+				my $offset =
+					statement_rawlines($whitespace) - 1;
+
+				$suppress_whiletrailers{$line_nr_next +
+								$offset} = 1;
+			}
+		}
+		if (!defined $suppress_whiletrailers{$linenr} &&
+		    $line =~ /\b(?:if|while|for)\s*\(/ && $line !~ /^.\s*#/) {
 			my ($s, $c) = ($stat, $cond);
 
 			if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/) {
-- 
1.6.0.1.451.gc8d31

[PATCH 04/13] checkpatch: macros which define structure members are not complex

[Andy Whitcroft]

We often see macros which define structure members, these are not
complex and necessarily do not have braces or brackets.  For example:

    #define _PLIST_HEAD_INIT(head)                      \
        .prio_list = LIST_HEAD_INIT((head).prio_list),  \
        .node_list = LIST_HEAD_INIT((head).node_list)

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 19690a2..6eceda7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2155,6 +2155,7 @@ sub process {
 				if ($dstat ne '' &&
 				    $dstat !~ /^(?:$Ident|-?$Constant)$/ &&
 				    $dstat !~ /$exceptions/ &&
+				    $dstat !~ /^\.$Ident\s*=/ &&
 				    $dstat =~ /$Operators/)
 				{
 					ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n");
-- 
1.6.0.1.451.gc8d31

[PATCH 05/13] checkpatch: accept any sized le/be type

[Andy Whitcroft]

We are likely going to have 24 bit types.  Expand the type matcher to match
any size.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6eceda7..bb88df2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -159,7 +159,7 @@ our @typeList = (
 	qr{float},
 	qr{double},
 	qr{bool},
-	qr{(?:__)?(?:u|s|be|le)(?:8|16|32|64)},
+	qr{(?:__)?(?:u|s|be|le)(?:\d|\d\d)},
 	qr{struct\s+$Ident},
 	qr{union\s+$Ident},
 	qr{enum\s+$Ident},
-- 
1.6.0.1.451.gc8d31

[PATCH 06/13] checkpatch: pull out known acceptable typedefs

[Andy Whitcroft]

Within the type checker we have a number of common kernel types which
must be implemented as typedefs.  Pull those out so that we can use
the same expressions to trigger exclusions.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bb88df2..4680ccf 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -146,6 +146,11 @@ our $UTF8	= qr {
 	|  \xF4[\x80-\x8F][\x80-\xBF]{2}     # plane 16
 }x;
 
+our $typeTypedefs = qr{(?x:
+	(?:__)?(?:u|s|be|le)(?:\d|\d\d)|
+	atomic_t
+)};
+
 our @typeList = (
 	qr{void},
 	qr{(?:unsigned\s+)?char},
@@ -159,7 +164,6 @@ our @typeList = (
 	qr{float},
 	qr{double},
 	qr{bool},
-	qr{(?:__)?(?:u|s|be|le)(?:\d|\d\d)},
 	qr{struct\s+$Ident},
 	qr{union\s+$Ident},
 	qr{enum\s+$Ident},
@@ -179,6 +183,7 @@ sub build_types {
 			(?:$Modifier\s+|const\s+)*
 			(?:
 				(?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\)|
+				(?:$typeTypedefs\b)|
 				(?:${all}\b)
 			)
 			(?:\s+$Modifier|\s+const)*
@@ -1589,6 +1594,7 @@ sub process {
 		if ($line =~ /\btypedef\s/ &&
 		    $line !~ /\btypedef\s+$Type\s+\(\s*\*?$Ident\s*\)\s*\(/ &&
 		    $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ &&
+		    $line !~ /\b$typeTypedefs\b/ &&
 		    $line !~ /\b__bitwise(?:__|)\b/) {
 			WARN("do not add new typedefs\n" . $herecurr);
 		}
-- 
1.6.0.1.451.gc8d31

[PATCH 07/13] checkpatch: suspect code indent must stop at #else/#elif

[Andy Whitcroft]

When we hit and #else or #elif we know we are meeting an alternative
piece of code.  All bets are off on indent if we did not see the open of
the control so stop checking.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4680ccf..c479bde 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1468,6 +1468,12 @@ sub process {
 			while ($cond_ptr != $cond_lines) {
 				$cond_ptr = $cond_lines;
 
+				# If we see an #else/#elif then the code
+				# is not linear.
+				if ($s =~ /^\s*\#\s*(?:else|elif)/) {
+					$check = 0;
+				}
+
 				# Ignore:
 				#  1) blank lines, they should be at 0,
 				#  2) preprocessor lines, and
-- 
1.6.0.1.451.gc8d31

[PATCH 08/13] checkpatch: complex macros checks miss square brackets

[Andy Whitcroft]

We are missing 'simple' values which include square brackets.  Refactor
to ensure we handle nesting correctly and detect these simple forms.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c479bde..54dfa2b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2142,9 +2142,10 @@ sub process {
 			$dstat =~ s/\s*$//s;
 
 			# Flatten any parentheses and braces
-			while ($dstat =~ s/\([^\(\)]*\)/1/) {
-			}
-			while ($dstat =~ s/\{[^\{\}]*\}/1/) {
+			while ($dstat =~ s/\([^\(\)]*\)/1/ ||
+			       $dstat =~ s/\{[^\{\}]*\}/1/ ||
+			       $dstat =~ s/\[[^\{\}]*\]/1/)
+			{
 			}
 
 			my $exceptions = qr{
-- 
1.6.0.1.451.gc8d31

[PATCH 09/13] checkpatch: DEFINE_ macros are real definitions for exports

[Andy Whitcroft]

When we want to confirm an export is directly after its definition we need
to allow for DEFINE_ style macros.  Add these to the execeptions.  Refactor
the exceptions.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 54dfa2b..a675f06 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1573,13 +1573,14 @@ sub process {
 		if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) ||
 		    ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
 			my $name = $1;
-			if (($prevline !~ /^}/) &&
-			   ($prevline !~ /^\+}/) &&
-			   ($prevline !~ /^ }/) &&
-			   ($prevline !~ /^.DECLARE_$Ident\(\Q$name\E\)/) &&
-			   ($prevline !~ /^.LIST_HEAD\(\Q$name\E\)/) &&
-			   ($prevline !~ /^.$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(/) &&
-			   ($prevline !~ /\b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=|\[)/)) {
+			if ($prevline !~ /(?:
+				^.}|
+				^.DEFINE_$Ident\(\Q$name\E\)|
+				^.DECLARE_$Ident\(\Q$name\E\)|
+				^.LIST_HEAD\(\Q$name\E\)|
+				^.$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
+				\b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=|\[)
+			    )/x) {
 				WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
 			}
 		}
-- 
1.6.0.1.451.gc8d31

[PATCH 10/13] checkpatch: trailing statements ensure we report the end of the line

[Andy Whitcroft]

When reporting some complex trailing statements we report only the
starting line of the error, that tends to imply the shown line is in
error and confuse the reader.  As we do know where the actual error is
report that line too with an appropriate gap marker where applicable.

    #ERROR: trailing statements should be on next line
    #1: FILE: Z202.c:1:
    +       for (pbh = page_buffers(bh->b_page); pbh != bh;
    +               pbh = pbh->b_this_page, key++);
    #ERROR: trailing statements should be on next line
    #4: FILE: Z202.c:4:
    +       for (pbh = page_buffers(bh->b_page);
    [...]
    +               pbh = pbh->b_this_page, key++);

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a675f06..2c1afba 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2001,7 +2001,16 @@ sub process {
 			if (length($c) && $s !~ /^\s*{?\s*\\*\s*$/ &&
 			    $c !~ /}\s*while\s*/)
 			{
-				ERROR("trailing statements should be on next line\n" . $herecurr);
+				# Find out how long the conditional actually is.
+				my @newlines = ($c =~ /\n/gs);
+				my $cond_lines = 1 + $#newlines;
+
+				my $stat_real = raw_line($linenr, $cond_lines);
+				if (defined($stat_real) && $cond_lines > 1) {
+					$stat_real = "[...]\n$stat_real";
+				}
+
+				ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real);
 			}
 		}
 
-- 
1.6.0.1.451.gc8d31

[PATCH 11/13] checkpatch: suspect indent handle macro continuation

[Andy Whitcroft]

When ignoring a macro in the middle of a conditional, we need to ignore
the macro start and any continuation lines.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2c1afba..862e8e0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1465,6 +1465,7 @@ sub process {
 			}
 
 			my $cond_ptr = -1;
+			$continuation = 0;
 			while ($cond_ptr != $cond_lines) {
 				$cond_ptr = $cond_lines;
 
@@ -1478,9 +1479,11 @@ sub process {
 				#  1) blank lines, they should be at 0,
 				#  2) preprocessor lines, and
 				#  3) labels.
-				if ($s =~ /^\s*?\n/ ||
+				if ($continuation ||
+				    $s =~ /^\s*?\n/ ||
 				    $s =~ /^\s*#\s*?/ ||
 				    $s =~ /^\s*$Ident\s*:/) {
+					$continuation = ($s =~ /^.*?\\\n/) ? 1 : 0;
 					$s =~ s/^.*?\n//;
 					$cond_lines++;
 				}
-- 
1.6.0.1.451.gc8d31

[PATCH 12/13] checkpatch: allow for comments either side of a brace on case

[Andy Whitcroft]

When specifying case we may have comments and/or braces at the end without
actually having a 'statement'.  Allow for these to occur in any order.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 862e8e0..ada27c4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2043,7 +2043,7 @@ sub process {
 # case and default should not have general statements after them
 		if ($line =~ /^.\s*(?:case\s*.*|default\s*):/g &&
 		    $line !~ /\G(?:
-			(?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$|
+			(?:\s*$;*)(?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$|
 			\s*return\s+
 		    )/xg)
 		{
-- 
1.6.0.1.451.gc8d31

[PATCH 13/13] checkpatch: version: 0.24

[Andy Whitcroft]

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 scripts/checkpatch.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ada27c4..e30bac1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,7 +9,7 @@ use strict;
 my $P = $0;
 $P =~ s@.*/@@g;
 
-my $V = '0.23';
+my $V = '0.24';
 
 use Getopt::Long qw(:config no_auto_abbrev);
 
-- 
1.6.0.1.451.gc8d31