All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/12] checkpatch: collection of neatenings and new "--fix"
@ 2013-06-10 22:20 Joe Perches
  2013-06-10 22:20 ` [PATCH 01/12] checkpatch: Remove quote from CamelCase test Joe Perches
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

A few neatenings and cleanups and an EXPERIMENTAL option to
fix various whitespace/formatting defects in patches.

Joe Perches (12):
  checkpatch: Remove quote from CamelCase test
  checkpatch: Improve network block comment test and message
  checkpatch: Warn when networking block comment lines don't start with *
  checkpatch: Warn on comparisons to jiffies
  checkpatch: Warn on comparisons to get_jiffies_64()
  checkpatch: Reduce false positive rate of "complex macros"
  checkpatch: Add a placeholder to check blank lines before declarations
  checkpatch: Don't warn on blank lines before/after braces as often
  checkpatch: Add a --strict test for comparison to true/false
  checkpatch: Improve "no space after cast" test
  checkpatch: Create an EXPERIMENTAL --fix option to correct patches
  checkpatch: Move test for space before semicolon after operator spacing

 scripts/checkpatch.pl | 524 ++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 423 insertions(+), 101 deletions(-)

-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/12] checkpatch: Remove quote from CamelCase test
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 02/12] checkpatch: Improve network block comment test and message Joe Perches
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

commit be987d9f80 ("checkpatch: improve CamelCase test for Page")
added it but it shouldn't be there.  Must have been my fault.

Make sure that the tested variable doesn't contain a constant.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6185eb6..2f61b5c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2949,9 +2949,9 @@ sub process {
 			}
 
 #CamelCase
-			if ($var !~ /$Constant/ &&
+			if ($var !~ /^$Constant$/ &&
 			    $var =~ /[A-Z][a-z]|[a-z][A-Z]/ &&
-			    $var !~ /"^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
+			    $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ &&
 			    !defined $camelcase{$var}) {
 				$camelcase{$var} = 1;
 				CHK("CAMELCASE",
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/12] checkpatch: Improve network block comment test and message
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
  2013-06-10 22:20 ` [PATCH 01/12] checkpatch: Remove quote from CamelCase test Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 03/12] checkpatch: Warn when networking block comment lines don't start with * Joe Perches
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Show the first line of the comment after a line
with just /* to better show where the defective
comment style is in the file.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2f61b5c..a3922d0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1891,8 +1891,8 @@ sub process {
 		}
 
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&
-		    $rawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
-		    $prevrawline =~ /^\+[ \t]*$/) {
+		    $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
+		    $rawline =~ /^\+[ \t]*\*/) {
 			WARN("NETWORKING_BLOCK_COMMENT_STYLE",
 			     "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev);
 		}
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/12] checkpatch: Warn when networking block comment lines don't start with *
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
  2013-06-10 22:20 ` [PATCH 01/12] checkpatch: Remove quote from CamelCase test Joe Perches
  2013-06-10 22:20 ` [PATCH 02/12] checkpatch: Improve network block comment test and message Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 04/12] checkpatch: Warn on comparisons to jiffies Joe Perches
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Some block comments in network are written as:

	/* block comment line 1
	   block comment line 2
	 */

Emit a warning on the "block comment line 2" because it should be

	/* block comment line 1
	 * block comment line 2
	 */

This warning is only emitted on the second line of a block comment.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index a3922d0..576139a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1898,6 +1898,14 @@ sub process {
 		}
 
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&
+		    $prevrawline =~ /^\+[ \t]*\/\*/ &&		#starting /*
+		    $prevrawline !~ /\*\/[ \t]*$/ &&		#no trailing */
+		    $rawline !~ /^\+[ \t]*\*/) {		#no leading *
+			WARN("NETWORKING_BLOCK_COMMENT_STYLE",
+			     "networking block comments start with * on subsequent lines\n" . $hereprev);
+		}
+
+		if ($realfile =~ m@^(drivers/net/|net/)@ &&
 		    $rawline !~ m@^\+[ \t]*\*/[ \t]*$@ &&	#trailing */
 		    $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ &&	#inline /*...*/
 		    $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ &&	#trailing **/
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/12] checkpatch: Warn on comparisons to jiffies
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (2 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 03/12] checkpatch: Warn when networking block comment lines don't start with * Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 05/12] checkpatch: Warn on comparisons to get_jiffies_64() Joe Perches
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Comparing jiffies is almost always wrong and
time_before and time_after should be used instead.

Warn on any comparison to jiffies.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 576139a..c274e1d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3299,6 +3299,12 @@ sub process {
 			}
 		}
 
+# check for comparisons of jiffies
+		if ($line =~ /\bjiffies\s*$Compare|$Compare\s*jiffies\b/) {
+			WARN("JIFFIES_COMPARISON",
+			     "Comparing jiffies is almost always wrong; prefer time_after, time_before and friends\n" . $herecurr);
+		}
+
 # warn about #ifdefs in C files
 #		if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) {
 #			print "#ifdef in C files should be avoided\n";
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/12] checkpatch: Warn on comparisons to get_jiffies_64()
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (3 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 04/12] checkpatch: Warn on comparisons to jiffies Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 06/12] checkpatch: Reduce false positive rate of "complex macros" Joe Perches
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Comparing get_jiffies_64() is almost always wrong and
time_before64 and time_after64 should be used instead.

Warn on any comparison to get_jiffies_64().

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c274e1d..f4e247b2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3305,6 +3305,12 @@ sub process {
 			     "Comparing jiffies is almost always wrong; prefer time_after, time_before and friends\n" . $herecurr);
 		}
 
+# check for comparisons of get_jiffies_64()
+		if ($line =~ /\bget_jiffies_64\s*\(\s*\)\s*$Compare|$Compare\s*get_jiffies_64\s*\(\s*\)/) {
+			WARN("JIFFIES_COMPARISON",
+			     "Comparing get_jiffies_64() is almost always wrong; prefer time_after64, time_before64 and friends\n" . $herecurr);
+		}
+
 # warn about #ifdefs in C files
 #		if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) {
 #			print "#ifdef in C files should be avoided\n";
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/12] checkpatch: Reduce false positive rate of "complex macros"
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (4 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 05/12] checkpatch: Warn on comparisons to get_jiffies_64() Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 07/12] checkpatch: Add a placeholder to check blank lines before declarations Joe Perches
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Allow "#define foo struct.member" without bleating a warning.

This also allows "#define foo bar.baz->qux" and so on.

Signed-off-by: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index f4e247b2..93b8e66 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3041,7 +3041,7 @@ sub process {
 			if ($dstat ne '' &&
 			    $dstat !~ /^(?:$Ident|-?$Constant),$/ &&			# 10, // foo(),
 			    $dstat !~ /^(?:$Ident|-?$Constant);$/ &&			# foo();
-			    $dstat !~ /^[!~-]?(?:$Ident|$Constant)$/ &&		# 10 // foo() // !foo // ~foo // -foo
+			    $dstat !~ /^[!~-]?(?:$Lval|$Constant)$/ &&		# 10 // foo() // !foo // ~foo // -foo // foo->bar // foo.bar->baz
 			    $dstat !~ /^'X'$/ &&					# character constants
 			    $dstat !~ /$exceptions/ &&
 			    $dstat !~ /^\.$Ident\s*=/ &&				# .foo =
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/12] checkpatch: Add a placeholder to check blank lines before declarations
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (5 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 06/12] checkpatch: Reduce false positive rate of "complex macros" Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 08/12] checkpatch: Don't warn on blank lines before/after braces as often Joe Perches
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Figure out first how to determine if this is
in a struct declaration or in a function body
before enabling this.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 93b8e66..4ad4052 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2750,6 +2750,14 @@ sub process {
 			      "space required before the open brace '{'\n" . $herecurr);
 		}
 
+## # check for blank lines before declarations
+##		if ($line =~ /^.\t+$Type\s+$Ident(?:\s*=.*)?;/ &&
+##		    $prevrawline =~ /^.\s*$/) {
+##			WARN("SPACING",
+##			     "No blank lines before declarations\n" . $hereprev);
+##		}
+##
+
 # closing brace should have a space following it when it has anything
 # on the line
 		if ($line =~ /}(?!(?:,|;|\)))\S/) {
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/12] checkpatch: Don't warn on blank lines before/after braces as often
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (6 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 07/12] checkpatch: Add a placeholder to check blank lines before declarations Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 09/12] checkpatch: Add a --strict test for comparison to true/false Joe Perches
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Check to make sure the blank lines aren't comment lines like:

bool foo(bool bar)
{
	/* Don't warn on a leading comment */
	return !bar;
	/* Don't warn on a trailing comment either */
}

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 4ad4052..c43be81 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3258,11 +3258,11 @@ sub process {
 		}
 
 # check for unnecessary blank lines around braces
-		if (($line =~ /^.\s*}\s*$/ && $prevline =~ /^.\s*$/)) {
+		if (($line =~ /^.\s*}\s*$/ && $prevrawline =~ /^.\s*$/)) {
 			CHK("BRACES",
 			    "Blank lines aren't necessary before a close brace '}'\n" . $hereprev);
 		}
-		if (($line =~ /^.\s*$/ && $prevline =~ /^..*{\s*$/)) {
+		if (($rawline =~ /^.\s*$/ && $prevline =~ /^..*{\s*$/)) {
 			CHK("BRACES",
 			    "Blank lines aren't necessary after an open brace '{'\n" . $hereprev);
 		}
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/12] checkpatch: Add a --strict test for comparison to true/false
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (7 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 08/12] checkpatch: Don't warn on blank lines before/after braces as often Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 10/12] checkpatch: Improve "no space after cast" test Joe Perches
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Comparing to true or false is error prone.

Add tests for the various forms of (foo == true) && (false != bar)
that are only reported with --strict.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c43be81..c2d223c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3588,6 +3588,33 @@ sub process {
 			     "Using yield() is generally wrong. See yield() kernel-doc (sched/core.c)\n"  . $herecurr);
 		}
 
+# check for comparisons against true and false
+		if ($line =~ /\+\s*(.*?)\b(true|false|$Lval)\s*(==|\!=)\s*(true|false|$Lval)\b(.*)$/i) {
+			my $lead = $1;
+			my $arg = $2;
+			my $test = $3;
+			my $otype = $4;
+			my $trail = $5;
+			my $op = "!";
+
+			($arg, $otype) = ($otype, $arg) if ($arg =~ /^(?:true|false)$/i);
+
+			my $type = lc($otype);
+			if ($type =~ /^(?:true|false)$/) {
+				if (("$test" eq "==" && "$type" eq "true") ||
+				    ("$test" eq "!=" && "$type" eq "false")) {
+					$op = "";
+				}
+
+				CHK("BOOL_COMPARISON",
+				    "Using comparison to $otype is error prone\n" . $herecurr);
+
+## maybe suggesting a correct construct would better
+##				    "Using comparison to $otype is error prone.  Perhaps use '${lead}${op}${arg}${trail}'\n" . $herecurr);
+
+			}
+		}
+
 # check for semaphores initialized locked
 		if ($line =~ /^.\s*sema_init.+,\W?0\W?\)/) {
 			WARN("CONSIDER_COMPLETION",
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/12] checkpatch: Improve "no space after cast" test
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (8 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 09/12] checkpatch: Add a --strict test for comparison to true/false Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 11/12] checkpatch: Create an EXPERIMENTAL --fix option to correct patches Joe Perches
  2013-06-10 22:20 ` [PATCH 12/12] checkpatch: Move test for space before semicolon after operator spacing Joe Perches
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Some false positives exist on this test.

For instance:
	*va_arg(args, signed char *) = val.s;
or
	memset(foo, 0, sizeof(struct bar *) * baz));

Ignore lines that have an arithmetic operator or assignment
after what appears to be a cast to a pointer "(foo *)".

Add $Arithmetic convenience variable.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c2d223c..ab39ceb 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -241,10 +241,11 @@ our $Float	= qr{$Float_hex|$Float_dec|$Float_int};
 our $Constant	= qr{$Float|$Binary|$Hex|$Int};
 our $Assignment	= qr{\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=};
 our $Compare    = qr{<=|>=|==|!=|<|>};
+our $Arithmetic = qr{\+|-|\*|\/|%};
 our $Operators	= qr{
 			<=|>=|==|!=|
 			=>|->|<<|>>|<|>|!|~|
-			&&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%
+			&&|\|\||,|\^|\+\+|--|&|\||$Arithmetic
 		  }x;
 
 our $NonptrType;
@@ -1885,7 +1886,7 @@ sub process {
 			}
 		}
 
-		if ($line =~ /^\+.*\*[ \t]*\)[ \t]+/) {
+		if ($line =~ /^\+.*\*[ \t]*\)[ \t]+(?!$Assignment|$Arithmetic)/) {
 			CHK("SPACING",
 			    "No space is necessary after a cast\n" . $hereprev);
 		}
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/12] checkpatch: Create an EXPERIMENTAL --fix option to correct patches
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (9 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 10/12] checkpatch: Improve "no space after cast" test Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  2013-06-10 22:20 ` [PATCH 12/12] checkpatch: Move test for space before semicolon after operator spacing Joe Perches
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Some patches have simple defects in whitespace and formatting that
checkpatch could correct automatically.  Attempt to do so.

Add a --fix option to create a "<inputfile>.EXPERIMENTAL-checkpatch-fixes"
file that tries to use normal kernel style for some of these formatting
errors.

Add warnings against using this file without verifying the changes.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ab39ceb..9696be5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -27,6 +27,7 @@ my $summary = 1;
 my $mailback = 0;
 my $summary_file = 0;
 my $show_types = 0;
+my $fix = 0;
 my $root;
 my %debug;
 my %ignore_type = ();
@@ -63,6 +64,11 @@ Options:
                              is all off)
   --test-only=WORD           report only warnings/errors containing WORD
                              literally
+  --fix                      EXPERIMENTAL - may create horrible results
+                             If correctable single-line errors exist, create
+                             "<inputfile>.EXPERIMENTAL-checkpatch-fixes"
+                             with potential errors corrected to the preferred
+                             checkpatch style
   -h, --help, --version      display this help and exit
 
 When FILE is - read standard input.
@@ -114,7 +120,7 @@ GetOptions(
 	'summary!'	=> \$summary,
 	'mailback!'	=> \$mailback,
 	'summary-file!'	=> \$summary_file,
-
+	'fix!'		=> \$fix,
 	'debug=s'	=> \%debug,
 	'test-only=s'	=> \$tst_only,
 	'h|help'	=> \$help,
@@ -367,6 +373,7 @@ $chk_signoff = 0 if ($file);
 
 my @rawlines = ();
 my @lines = ();
+my @fixed = ();
 my $vname;
 for my $filename (@ARGV) {
 	my $FILE;
@@ -394,6 +401,7 @@ for my $filename (@ARGV) {
 	}
 	@rawlines = ();
 	@lines = ();
+	@fixed = ();
 }
 
 exit($exit);
@@ -434,7 +442,7 @@ sub parse_email {
 		$comment = $2 if defined $2;
 		$formatted_email =~ s/$address.*$//;
 		$name = $formatted_email;
-		$name =~ s/^\s+|\s+$//g;
+		$name = trim($name);
 		$name =~ s/^\"|\"$//g;
 		# If there's a name left after stripping spaces and
 		# leading quotes, and the address doesn't have both
@@ -449,9 +457,9 @@ sub parse_email {
 		}
 	}
 
-	$name =~ s/^\s+|\s+$//g;
+	$name = trim($name);
 	$name =~ s/^\"|\"$//g;
-	$address =~ s/^\s+|\s+$//g;
+	$address = trim($address);
 	$address =~ s/^\<|\>$//g;
 
 	if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
@@ -467,9 +475,9 @@ sub format_email {
 
 	my $formatted_email;
 
-	$name =~ s/^\s+|\s+$//g;
+	$name = trim($name);
 	$name =~ s/^\"|\"$//g;
-	$address =~ s/^\s+|\s+$//g;
+	$address = trim($address);
 
 	if ($name =~ /[^\w \-]/i) { ##has "must quote" chars
 		$name =~ s/(?<!\\)"/\\"/g; ##escape quotes
@@ -1291,19 +1299,25 @@ sub ERROR {
 	if (report("ERROR", $_[0], $_[1])) {
 		our $clean = 0;
 		our $cnt_error++;
+		return 1;
 	}
+	return 0;
 }
 sub WARN {
 	if (report("WARNING", $_[0], $_[1])) {
 		our $clean = 0;
 		our $cnt_warn++;
+		return 1;
 	}
+	return 0;
 }
 sub CHK {
 	if ($check && report("CHECK", $_[0], $_[1])) {
 		our $clean = 0;
 		our $cnt_chk++;
+		return 1;
 	}
+	return 0;
 }
 
 sub check_absolute_file {
@@ -1334,6 +1348,29 @@ sub check_absolute_file {
 	}
 }
 
+sub trim {
+	my ($string) = @_;
+
+	$string =~ s/(^\s+|\s+$)//g;
+
+	return $string;
+}
+
+sub tabify {
+	my ($leading) = @_;
+
+	my $source_indent = 8;
+	my $max_spaces_before_tab = $source_indent - 1;
+	my $spaces_to_tab = " " x $source_indent;
+
+	#convert leading spaces to tabs
+	1 while $leading =~ s@^([\t]*)$spaces_to_tab@$1\t@g;
+	#Remove spaces before a tab
+	1 while $leading =~ s@^([\t]*)( {1,$max_spaces_before_tab})\t@$1\t@g;
+
+	return "$leading";
+}
+
 sub pos_last_openparen {
 	my ($line) = @_;
 
@@ -1425,6 +1462,8 @@ sub process {
 		$linenr++;
 		$line = $rawline;
 
+		push(@fixed, $rawline) if ($fix);
+
 		if ($rawline=~/^\+\+\+\s+(\S+)/) {
 			$setup_docs = 0;
 			if ($1 =~ m@Documentation/kernel-parameters.txt$@) {
@@ -1616,16 +1655,29 @@ sub process {
 				     "Non-standard signature: $sign_off\n" . $herecurr);
 			}
 			if (defined $space_before && $space_before ne "") {
-				WARN("BAD_SIGN_OFF",
-				     "Do not use whitespace before $ucfirst_sign_off\n" . $herecurr);
+				if (WARN("BAD_SIGN_OFF",
+					 "Do not use whitespace before $ucfirst_sign_off\n" . $herecurr) &&
+				    $fix) {
+					$fixed[$linenr - 1] =
+					    "$ucfirst_sign_off $email";
+				}
 			}
 			if ($sign_off =~ /-by:$/i && $sign_off ne $ucfirst_sign_off) {
-				WARN("BAD_SIGN_OFF",
-				     "'$ucfirst_sign_off' is the preferred signature form\n" . $herecurr);
+				if (WARN("BAD_SIGN_OFF",
+					 "'$ucfirst_sign_off' is the preferred signature form\n" . $herecurr) &&
+				    $fix) {
+					$fixed[$linenr - 1] =
+					    "$ucfirst_sign_off $email";
+				}
+
 			}
 			if (!defined $space_after || $space_after ne " ") {
-				WARN("BAD_SIGN_OFF",
-				     "Use a single space after $ucfirst_sign_off\n" . $herecurr);
+				if (WARN("BAD_SIGN_OFF",
+					 "Use a single space after $ucfirst_sign_off\n" . $herecurr) &&
+				    $fix) {
+					$fixed[$linenr - 1] =
+					    "$ucfirst_sign_off $email";
+				}
 			}
 
 			my ($email_name, $email_address, $comment) = parse_email($email);
@@ -1715,8 +1767,12 @@ sub process {
 
 		} elsif ($rawline =~ /^\+.*\S\s+$/ || $rawline =~ /^\+\s+$/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			ERROR("TRAILING_WHITESPACE",
-			      "trailing whitespace\n" . $herevet);
+			if (ERROR("TRAILING_WHITESPACE",
+				  "trailing whitespace\n" . $herevet) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~ s/^(\+.*?)\s+$/$1/;
+			}
+
 			$rpt_cleaners = 1;
 		}
 
@@ -1811,8 +1867,12 @@ sub process {
 
 # check for spaces before a quoted newline
 		if ($rawline =~ /^.*\".*\s\\n/) {
-			WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
-			     "unnecessary whitespace before a quoted newline\n" . $herecurr);
+			if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE",
+				 "unnecessary whitespace before a quoted newline\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~ s/^(\+.*\".*)\s+\\n/$1\\n/;
+			}
+
 		}
 
 # check for adding lines without a newline.
@@ -1843,16 +1903,23 @@ sub process {
 		if ($rawline =~ /^\+\s* \t\s*\S/ ||
 		    $rawline =~ /^\+\s*        \s*/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			ERROR("CODE_INDENT",
-			      "code indent should use tabs where possible\n" . $herevet);
 			$rpt_cleaners = 1;
+			if (ERROR("CODE_INDENT",
+				  "code indent should use tabs where possible\n" . $herevet) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~ s/^\+([ \t]+)/"\+" . tabify($1)/e;
+			}
 		}
 
 # check for space before tabs.
 		if ($rawline =~ /^\+/ && $rawline =~ / \t/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			WARN("SPACE_BEFORE_TAB",
-			     "please, no space before tabs\n" . $herevet);
+			if (WARN("SPACE_BEFORE_TAB",
+				"please, no space before tabs\n" . $herevet) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/(^\+.*) +\t/$1\t/;
+			}
 		}
 
 # check for && or || at the start of a line
@@ -1880,15 +1947,24 @@ sub process {
 
 				if ($newindent ne $goodtabindent &&
 				    $newindent ne $goodspaceindent) {
-					CHK("PARENTHESIS_ALIGNMENT",
-					    "Alignment should match open parenthesis\n" . $hereprev);
+
+					if (CHK("PARENTHESIS_ALIGNMENT",
+						"Alignment should match open parenthesis\n" . $hereprev) &&
+					    $fix && $line =~ /^\+/) {
+						$fixed[$linenr - 1] =~
+						    s/^\+[ \t]*/\+$goodtabindent/;
+					}
 				}
 			}
 		}
 
 		if ($line =~ /^\+.*\*[ \t]*\)[ \t]+(?!$Assignment|$Arithmetic)/) {
-			CHK("SPACING",
-			    "No space is necessary after a cast\n" . $hereprev);
+			if (CHK("SPACING",
+				"No space is necessary after a cast\n" . $hereprev) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/^(\+.*\*[ \t]*\))[ \t]+/$1/;
+			}
 		}
 
 		if ($realfile =~ m@^(drivers/net/|net/)@ &&
@@ -1920,10 +1996,13 @@ sub process {
 #  1) within comments
 #  2) indented preprocessor commands
 #  3) hanging labels
-		if ($rawline =~ /^\+ / && $line !~ /\+ *(?:$;|#|$Ident:)/)  {
+		if ($rawline =~ /^\+ / && $line !~ /^\+ *(?:$;|#|$Ident:)/)  {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			WARN("LEADING_SPACE",
-			     "please, no spaces at the start of a line\n" . $herevet);
+			if (WARN("LEADING_SPACE",
+				 "please, no spaces at the start of a line\n" . $herevet) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~ s/^\+([ \t]+)/"\+" . tabify($1)/e;
+			}
 		}
 
 # check we are in a valid C source file if not then ignore this hunk
@@ -2213,7 +2292,7 @@ sub process {
 		$prev_values = substr($curr_values, -1);
 
 #ignore lines not being added
-		if ($line=~/^[^\+]/) {next;}
+		next if ($line =~ /^[^\+]/);
 
 # TEST: allow direct testing of the type matcher.
 		if ($dbg_type) {
@@ -2264,8 +2343,15 @@ sub process {
 
 # no C99 // comments
 		if ($line =~ m{//}) {
-			ERROR("C99_COMMENTS",
-			      "do not use C99 // comments\n" . $herecurr);
+			if (ERROR("C99_COMMENTS",
+				  "do not use C99 // comments\n" . $herecurr) &&
+			    $fix) {
+				my $line = $fixed[$linenr - 1];
+				if ($line =~ /\/\/(.*)$/) {
+					my $comment = trim($1);
+					$fixed[$linenr - 1] =~ s@\/\/(.*)$@/\* $comment \*/@;
+				}
+			}
 		}
 		# Remove C99 comments.
 		$line =~ s@//.*@@;
@@ -2364,7 +2450,7 @@ sub process {
 		# (char*[ const])
 		while ($line =~ m{(\($NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)\))}g) {
 			#print "AA<$1>\n";
-			my ($from, $to) = ($2, $2);
+			my ($ident, $from, $to) = ($1, $2, $2);
 
 			# Should start with a space.
 			$to =~ s/^(\S)/ $1/;
@@ -2374,15 +2460,22 @@ sub process {
 			while ($to =~ s/\*\s+\*/\*\*/) {
 			}
 
-			#print "from<$from> to<$to>\n";
+##			print "1: from<$from> to<$to> ident<$ident>\n";
 			if ($from ne $to) {
-				ERROR("POINTER_LOCATION",
-				      "\"(foo$from)\" should be \"(foo$to)\"\n" .  $herecurr);
+				if (ERROR("POINTER_LOCATION",
+					  "\"(foo$from)\" should be \"(foo$to)\"\n" .  $herecurr) &&
+				    $fix) {
+					my $sub_from = $ident;
+					my $sub_to = $ident;
+					$sub_to =~ s/\Q$from\E/$to/;
+					$fixed[$linenr - 1] =~
+					    s@\Q$sub_from\E@$sub_to@;
+				}
 			}
 		}
 		while ($line =~ m{(\b$NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)($Ident))}g) {
 			#print "BB<$1>\n";
-			my ($from, $to, $ident) = ($2, $2, $3);
+			my ($match, $from, $to, $ident) = ($1, $2, $2, $3);
 
 			# Should start with a space.
 			$to =~ s/^(\S)/ $1/;
@@ -2394,10 +2487,18 @@ sub process {
 			# Modifiers should have spaces.
 			$to =~ s/(\b$Modifier$)/$1 /;
 
-			#print "from<$from> to<$to> ident<$ident>\n";
+##			print "2: from<$from> to<$to> ident<$ident>\n";
 			if ($from ne $to && $ident !~ /^$Modifier$/) {
-				ERROR("POINTER_LOCATION",
-				      "\"foo${from}bar\" should be \"foo${to}bar\"\n" .  $herecurr);
+				if (ERROR("POINTER_LOCATION",
+					  "\"foo${from}bar\" should be \"foo${to}bar\"\n" .  $herecurr) &&
+				    $fix) {
+
+					my $sub_from = $match;
+					my $sub_to = $match;
+					$sub_to =~ s/\Q$from\E/$to/;
+					$fixed[$linenr - 1] =~
+					    s@\Q$sub_from\E@$sub_to@;
+				}
 			}
 		}
 
@@ -2483,9 +2584,13 @@ sub process {
 		}
 
 # missing space after union, struct or enum definition
-		if ($line =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?(?:\s+$Ident)?[=\{]/) {
-		    WARN("SPACING",
-			 "missing space after $1 definition\n" . $herecurr);
+		if ($line =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident){1,2}[=\{]/) {
+			if (WARN("SPACING",
+				 "missing space after $1 definition\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/^(.\s*(?:typedef\s+)?(?:enum|union|struct)(?:\s+$Ident){1,2})([=\{])/$1 $2/;
+			}
 		}
 
 # check for spacing round square brackets; allowed:
@@ -2497,8 +2602,12 @@ sub process {
 			if ($prefix !~ /$Type\s+$/ &&
 			    ($where != 0 || $prefix !~ /^.\s+$/) &&
 			    $prefix !~ /[{,]\s+$/) {
-				ERROR("BRACKET_SPACE",
-				      "space prohibited before open square bracket '['\n" . $herecurr);
+				if (ERROR("BRACKET_SPACE",
+					  "space prohibited before open square bracket '['\n" . $herecurr) &&
+				    $fix) {
+				    $fixed[$linenr - 1] =~
+					s/^(\+.*?)\s+\[/$1\[/;
+				}
 			}
 		}
 
@@ -2515,7 +2624,6 @@ sub process {
 				__attribute__|format|__extension__|
 				asm|__asm__)$/x)
 			{
-
 			# cpp #define statements have non-optional spaces, ie
 			# if there is a space between the name and the open
 			# parenthesis it is simply not a parameter group.
@@ -2529,19 +2637,30 @@ sub process {
 			} elsif ($ctx =~ /$Type$/) {
 
 			} else {
-				WARN("SPACING",
-				     "space prohibited between function name and open parenthesis '('\n" . $herecurr);
+				if (WARN("SPACING",
+					 "space prohibited between function name and open parenthesis '('\n" . $herecurr) &&
+					     $fix) {
+					$fixed[$linenr - 1] =~
+					    s/\b$name\s+\(/$name\(/;
+				}
 			}
 		}
 
 # check for whitespace before a non-naked semicolon
 		if ($line =~ /^\+.*\S\s+;/) {
-			WARN("SPACING",
-			     "space prohibited before semicolon\n" . $herecurr);
+			if (WARN("SPACING",
+				 "space prohibited before semicolon\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/^(\+.*\S)\s+;/$1;/;
+			}
 		}
 
 # Check operator spacing.
 		if (!($line=~/\#\s*include/)) {
+			my $fixed_line = "";
+			my $line_fixed = 0;
+
 			my $ops = qr{
 				<<=|>>=|<=|>=|==|!=|
 				\+=|-=|\*=|\/=|%=|\^=|\|=|&=|
@@ -2550,11 +2669,30 @@ sub process {
 				\?|:
 			}x;
 			my @elements = split(/($ops|;)/, $opline);
+
+##			print("element count: <" . $#elements . ">\n");
+##			foreach my $el (@elements) {
+##				print("el: <$el>\n");
+##			}
+
+			my @fix_elements = ();
 			my $off = 0;
 
+			foreach my $el (@elements) {
+				push(@fix_elements, substr($rawline, $off, length($el)));
+				$off += length($el);
+			}
+
+			$off = 0;
+
 			my $blank = copy_spacing($opline);
 
 			for (my $n = 0; $n < $#elements; $n += 2) {
+
+				my $good = $fix_elements[$n] . $fix_elements[$n + 1];
+
+##				print("n: <$n> good: <$good>\n");
+
 				$off += length($elements[$n]);
 
 				# Pick up the preceding and succeeding characters.
@@ -2611,8 +2749,11 @@ sub process {
 				} elsif ($op eq ';') {
 					if ($ctx !~ /.x[WEBC]/ &&
 					    $cc !~ /^\\/ && $cc !~ /^;/) {
-						ERROR("SPACING",
-						      "space required after that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space required after that '$op' $at\n" . $hereptr)) {
+							$good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+							$line_fixed = 1;
+						}
 					}
 
 				# // is a comment
@@ -2623,15 +2764,24 @@ sub process {
 				#   :   when part of a bitfield
 				} elsif ($op eq '->' || $opv eq ':B') {
 					if ($ctx =~ /Wx.|.xW/) {
-						ERROR("SPACING",
-						      "spaces prohibited around that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "spaces prohibited around that '$op' $at\n" . $hereptr)) {
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+							$line_fixed = 1;
+							if (defined $fix_elements[$n + 2]) {
+								$fix_elements[$n + 2] =~ s/^\s+//;
+							}
+						}
 					}
 
 				# , must have a space on the right.
 				} elsif ($op eq ',') {
 					if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) {
-						ERROR("SPACING",
-						      "space required after that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space required after that '$op' $at\n" . $hereptr)) {
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]) . " ";
+							$line_fixed = 1;
+						}
 					}
 
 				# '*' as part of a type definition -- reported already.
@@ -2645,34 +2795,58 @@ sub process {
 					 $opv eq '*U' || $opv eq '-U' ||
 					 $opv eq '&U' || $opv eq '&&U') {
 					if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
-						ERROR("SPACING",
-						      "space required before that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space required before that '$op' $at\n" . $hereptr)) {
+							$good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]);
+							$line_fixed = 1;
+						}
 					}
 					if ($op eq '*' && $cc =~/\s*$Modifier\b/) {
 						# A unary '*' may be const
 
 					} elsif ($ctx =~ /.xW/) {
-						ERROR("SPACING",
-						      "space prohibited after that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space prohibited after that '$op' $at\n" . $hereptr)) {
+							$fixed_line =~ s/\s+$//;
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+							$line_fixed = 1;
+							if (defined $fix_elements[$n + 2]) {
+								$fix_elements[$n + 2] =~ s/^\s+//;
+							}
+						}
 					}
 
 				# unary ++ and unary -- are allowed no space on one side.
 				} elsif ($op eq '++' or $op eq '--') {
 					if ($ctx !~ /[WEOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) {
-						ERROR("SPACING",
-						      "space required one side of that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space required one side of that '$op' $at\n" . $hereptr)) {
+							$fixed_line =~ s/\s+$//;
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]) . " ";
+							$line_fixed = 1;
+						}
 					}
 					if ($ctx =~ /Wx[BE]/ ||
 					    ($ctx =~ /Wx./ && $cc =~ /^;/)) {
-						ERROR("SPACING",
-						      "space prohibited before that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space prohibited before that '$op' $at\n" . $hereptr)) {
+							$fixed_line =~ s/\s+$//;
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+							$line_fixed = 1;
+						}
 					}
 					if ($ctx =~ /ExW/) {
-						ERROR("SPACING",
-						      "space prohibited after that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space prohibited after that '$op' $at\n" . $hereptr)) {
+							$fixed_line =~ s/\s+$//;
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+							$line_fixed = 1;
+							if (defined $fix_elements[$n + 2]) {
+								$fix_elements[$n + 2] =~ s/^\s+//;
+							}
+						}
 					}
 
-
 				# << and >> may either have or not have spaces both sides
 				} elsif ($op eq '<<' or $op eq '>>' or
 					 $op eq '&' or $op eq '^' or $op eq '|' or
@@ -2681,17 +2855,23 @@ sub process {
 					 $op eq '%')
 				{
 					if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
-						ERROR("SPACING",
-						      "need consistent spacing around '$op' $at\n" .
-							$hereptr);
+						if (ERROR("SPACING",
+							  "need consistent spacing around '$op' $at\n" . $hereptr)) {
+							$fixed_line =~ s/\s+$//;
+							$good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+							$line_fixed = 1;
+						}
 					}
 
 				# A colon needs no spaces before when it is
 				# terminating a case value or a label.
 				} elsif ($opv eq ':C' || $opv eq ':L') {
 					if ($ctx =~ /Wx./) {
-						ERROR("SPACING",
-						      "space prohibited before that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "space prohibited before that '$op' $at\n" . $hereptr)) {
+							$good = trim($fix_elements[$n]) . trim($fix_elements[$n + 1]);
+							$line_fixed = 1;
+						}
 					}
 
 				# All the others need spaces both sides.
@@ -2714,12 +2894,30 @@ sub process {
 					}
 
 					if ($ok == 0) {
-						ERROR("SPACING",
-						      "spaces required around that '$op' $at\n" . $hereptr);
+						if (ERROR("SPACING",
+							  "spaces required around that '$op' $at\n" . $hereptr)) {
+							$good = trim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " ";
+							$good = $fix_elements[$n] . " " . trim($fix_elements[$n + 1]) . " ";
+							$line_fixed = 1;
+						}
 					}
 				}
 				$off += length($elements[$n + 1]);
+
+##				print("n: <$n> GOOD: <$good>\n");
+
+				$fixed_line = $fixed_line . $good;
+			}
+
+			if (($#elements % 2) == 0) {
+				$fixed_line = $fixed_line . $fix_elements[$#elements];
 			}
+
+			if ($fix && $line_fixed && $fixed_line ne $fixed[$linenr - 1]) {
+				$fixed[$linenr - 1] = $fixed_line;
+			}
+
+
 		}
 
 # check for multiple assignments
@@ -2747,8 +2945,12 @@ sub process {
 #need space before brace following if, while, etc
 		if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) ||
 		    $line =~ /do{/) {
-			ERROR("SPACING",
-			      "space required before the open brace '{'\n" . $herecurr);
+			if (ERROR("SPACING",
+				  "space required before the open brace '{'\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/^(\+.*(?:do|\))){/$1 {/;
+			}
 		}
 
 ## # check for blank lines before declarations
@@ -2768,32 +2970,52 @@ sub process {
 
 # check spacing on square brackets
 		if ($line =~ /\[\s/ && $line !~ /\[\s*$/) {
-			ERROR("SPACING",
-			      "space prohibited after that open square bracket '['\n" . $herecurr);
+			if (ERROR("SPACING",
+				  "space prohibited after that open square bracket '['\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/\[\s+/\[/;
+			}
 		}
 		if ($line =~ /\s\]/) {
-			ERROR("SPACING",
-			      "space prohibited before that close square bracket ']'\n" . $herecurr);
+			if (ERROR("SPACING",
+				  "space prohibited before that close square bracket ']'\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/\s+\]/\]/;
+			}
 		}
 
 # check spacing on parentheses
 		if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ &&
 		    $line !~ /for\s*\(\s+;/) {
-			ERROR("SPACING",
-			      "space prohibited after that open parenthesis '('\n" . $herecurr);
+			if (ERROR("SPACING",
+				  "space prohibited after that open parenthesis '('\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/\(\s+/\(/;
+			}
 		}
 		if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ &&
 		    $line !~ /for\s*\(.*;\s+\)/ &&
 		    $line !~ /:\s+\)/) {
-			ERROR("SPACING",
-			      "space prohibited before that close parenthesis ')'\n" . $herecurr);
+			if (ERROR("SPACING",
+				  "space prohibited before that close parenthesis ')'\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/\s+\)/\)/;
+			}
 		}
 
 #goto labels aren't indented, allow a single space however
 		if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and
 		   !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) {
-			WARN("INDENTED_LABEL",
-			     "labels should not be indented\n" . $herecurr);
+			if (WARN("INDENTED_LABEL",
+				 "labels should not be indented\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/^(.)\s+/$1/;
+			}
 		}
 
 # Return is not a function.
@@ -2830,8 +3052,13 @@ sub process {
 		}
 
 # Need a space before open parenthesis after if, while etc
-		if ($line=~/\b(if|while|for|switch)\(/) {
-			ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr);
+		if ($line =~ /\b(if|while|for|switch)\(/) {
+			if (ERROR("SPACING",
+				  "space required before the open parenthesis '('\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/\b(if|while|for|switch)\(/$1 \(/;
+			}
 		}
 
 # Check for illegal assignment in if conditional -- and check for trailing
@@ -3329,8 +3556,13 @@ sub process {
 
 # warn about spacing in #ifdefs
 		if ($line =~ /^.\s*\#\s*(ifdef|ifndef|elif)\s\s+/) {
-			ERROR("SPACING",
-			      "exactly one space required after that #$1\n" . $herecurr);
+			if (ERROR("SPACING",
+				  "exactly one space required after that #$1\n" . $herecurr) &&
+			    $fix) {
+				$fixed[$linenr - 1] =~
+				    s/^(.\s*\#\s*(ifdef|ifndef|elif))\s{2,}/$1 /;
+			}
+
 		}
 
 # check for spinlock_t definitions without a comment.
@@ -3793,6 +4025,40 @@ sub process {
 	    print "\n\n";
 	}
 
+	if ($clean == 0 && $fix && "@rawlines" ne "@fixed") {
+		my $newfile = $filename . ".EXPERIMENTAL-checkpatch-fixes";
+		my $linecount = 0;
+		my $f;
+
+		open($f, '>', $newfile)
+		    or die "$P: Can't open $newfile for write\n";
+		foreach my $fixed_line (@fixed) {
+			$linecount++;
+			if ($file) {
+				if ($linecount > 3) {
+					$fixed_line =~ s/^\+//;
+					print $f $fixed_line. "\n";
+				}
+			} else {
+				print $f $fixed_line . "\n";
+			}
+		}
+		close($f);
+
+		if (!$quiet) {
+			print << "EOM";
+Wrote EXPERIMENTAL --fix correction(s) to '$newfile'
+
+Do _NOT_ trust the results written to this file.
+Do _NOT_ submit these changes without inspecting them for correctness.
+
+This EXPERIMENTAL file is simply a convenience to help rewrite patches.
+No warranties, expressed or implied...
+
+EOM
+		}
+	}
+
 	if ($clean == 1 && $quiet == 0) {
 		print "$vname has no obvious style problems and is ready for submission.\n"
 	}
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 12/12] checkpatch: Move test for space before semicolon after operator spacing
  2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
                   ` (10 preceding siblings ...)
  2013-06-10 22:20 ` [PATCH 11/12] checkpatch: Create an EXPERIMENTAL --fix option to correct patches Joe Perches
@ 2013-06-10 22:20 ` Joe Perches
  11 siblings, 0 replies; 13+ messages in thread
From: Joe Perches @ 2013-06-10 22:20 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Andy Whitcroft, linux-kernel

Moving this test allows the --fix option to work better.

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9696be5..5989415 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2646,16 +2646,6 @@ sub process {
 			}
 		}
 
-# check for whitespace before a non-naked semicolon
-		if ($line =~ /^\+.*\S\s+;/) {
-			if (WARN("SPACING",
-				 "space prohibited before semicolon\n" . $herecurr) &&
-			    $fix) {
-				$fixed[$linenr - 1] =~
-				    s/^(\+.*\S)\s+;/$1;/;
-			}
-		}
-
 # Check operator spacing.
 		if (!($line=~/\#\s*include/)) {
 			my $fixed_line = "";
@@ -2920,6 +2910,16 @@ sub process {
 
 		}
 
+# check for whitespace before a non-naked semicolon
+		if ($line =~ /^\+.*\S\s+;/) {
+			if (WARN("SPACING",
+				 "space prohibited before semicolon\n" . $herecurr) &&
+			    $fix) {
+				1 while $fixed[$linenr - 1] =~
+				    s/^(\+.*\S)\s+;/$1;/;
+			}
+		}
+
 # check for multiple assignments
 		if ($line =~ /^.\s*$Lval\s*=\s*$Lval\s*=(?!=)/) {
 			CHK("MULTIPLE_ASSIGNMENTS",
-- 
1.8.1.2.459.gbcd45b4.dirty


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-06-10 22:22 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-10 22:20 [PATCH 00/12] checkpatch: collection of neatenings and new "--fix" Joe Perches
2013-06-10 22:20 ` [PATCH 01/12] checkpatch: Remove quote from CamelCase test Joe Perches
2013-06-10 22:20 ` [PATCH 02/12] checkpatch: Improve network block comment test and message Joe Perches
2013-06-10 22:20 ` [PATCH 03/12] checkpatch: Warn when networking block comment lines don't start with * Joe Perches
2013-06-10 22:20 ` [PATCH 04/12] checkpatch: Warn on comparisons to jiffies Joe Perches
2013-06-10 22:20 ` [PATCH 05/12] checkpatch: Warn on comparisons to get_jiffies_64() Joe Perches
2013-06-10 22:20 ` [PATCH 06/12] checkpatch: Reduce false positive rate of "complex macros" Joe Perches
2013-06-10 22:20 ` [PATCH 07/12] checkpatch: Add a placeholder to check blank lines before declarations Joe Perches
2013-06-10 22:20 ` [PATCH 08/12] checkpatch: Don't warn on blank lines before/after braces as often Joe Perches
2013-06-10 22:20 ` [PATCH 09/12] checkpatch: Add a --strict test for comparison to true/false Joe Perches
2013-06-10 22:20 ` [PATCH 10/12] checkpatch: Improve "no space after cast" test Joe Perches
2013-06-10 22:20 ` [PATCH 11/12] checkpatch: Create an EXPERIMENTAL --fix option to correct patches Joe Perches
2013-06-10 22:20 ` [PATCH 12/12] checkpatch: Move test for space before semicolon after operator spacing Joe Perches

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.