public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Please fix or revert: [PATCH] checkpatch: add --strict tests for braces, comments and casts
@ 2012-04-16  9:17 Ingo Molnar
  2012-04-16 18:27 ` David Miller
  2012-04-16 19:44 ` Randy Dunlap
  0 siblings, 2 replies; 19+ messages in thread
From: Ingo Molnar @ 2012-04-16  9:17 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, Andrew Morton, Linus Torvalds, Andy Whitcroft


This recent checkpatch commit, added in v3.4-rc1:

  aad4f6149831 checkpatch: add --strict tests for braces, comments and casts

made the default checkpatch run complain about the following 
perfectly fine multi-line comment block:

+               rdp->qlen_lazy = 0;
+               rdp->qlen = 0;
+
+               /*
+                * Adopt all callbacks.  The outgoing CPU was in no shape
+                * to advance them, so make them all go through a full
+                * grace period.
+                */
+               *receive_rdp->nxttail[RCU_NEXT_TAIL] = rdp->nxtlist;

With:

 CHECK: Don't begin block comments with only a /* line, use /* comment...

#80: FILE: kernel/rcutree.c:1428:
+
+		/*

Which is an obviously bogus warning: the comment is perfectly 
fine and it has the form that Documentation/CodingStyle 
specifically recommends:

|                 Chapter 8: Commenting
|
| [...]
|
| The preferred style for long (multi-line) comments is:
|
|        /*
|         * This is the preferred style for multi-line
|         * comments in the Linux kernel source code.
|         * Please use it consistently.
|         *
|         * Description:  A column of asterisks on the left side,
|         * with beginning and ending almost-blank lines.
|         */
|

Please turn this new warning off by default, or fix the check, 
or revert it. (The revert below applies cleanly on top of latest 
-git and solves the warning for me.)

Thanks,

	Ingo

This reverts commit aad4f61498314d41d047ea2161b7bd7237b72d33.

Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 scripts/checkpatch.pl |   40 ++++++++--------------------------------
 1 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index de639ee..6217093 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1864,17 +1864,6 @@ sub process {
 			}
 		}
 
-		if ($line =~ /^\+.*\*[ \t]*\)[ \t]+/) {
-			CHK("SPACING",
-			    "No space is necessary after a cast\n" . $hereprev);
-		}
-
-		if ($rawline =~ /^\+[ \t]*\/\*[ \t]*$/ &&
-		    $prevrawline =~ /^\+[ \t]*$/) {
-			CHK("BLOCK_COMMENT_STYLE",
-			    "Don't begin block comments with only a /* line, use /* comment...\n" . $hereprev);
-		}
-
 # check for spaces at the beginning of a line.
 # Exceptions:
 #  1) within comments
@@ -2987,8 +2976,7 @@ sub process {
 			#print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
 			#print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n";
 			if ($#chunks > 0 && $level == 0) {
-				my @allowed = ();
-				my $allow = 0;
+				my $allowed = 0;
 				my $seen = 0;
 				my $herectx = $here . "\n";
 				my $ln = $linenr - 1;
@@ -2999,7 +2987,6 @@ sub process {
 					my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s);
 					my $offset = statement_rawlines($whitespace) - 1;
 
-					$allowed[$allow] = 0;
 					#print "COND<$cond> whitespace<$whitespace> offset<$offset>\n";
 
 					# We have looked at and allowed this specific line.
@@ -3012,34 +2999,23 @@ sub process {
 
 					$seen++ if ($block =~ /^\s*{/);
 
-					#print "cond<$cond> block<$block> allowed<$allowed[$allow]>\n";
+					#print "cond<$cond> block<$block> allowed<$allowed>\n";
 					if (statement_lines($cond) > 1) {
 						#print "APW: ALLOWED: cond<$cond>\n";
-						$allowed[$allow] = 1;
+						$allowed = 1;
 					}
 					if ($block =~/\b(?:if|for|while)\b/) {
 						#print "APW: ALLOWED: block<$block>\n";
-						$allowed[$allow] = 1;
+						$allowed = 1;
 					}
 					if (statement_block_size($block) > 1) {
 						#print "APW: ALLOWED: lines block<$block>\n";
-						$allowed[$allow] = 1;
+						$allowed = 1;
 					}
-					$allow++;
 				}
-				if ($seen) {
-					my $sum_allowed = 0;
-					foreach (@allowed) {
-						$sum_allowed += $_;
-					}
-					if ($sum_allowed == 0) {
-						WARN("BRACES",
-						     "braces {} are not necessary for any arm of this statement\n" . $herectx);
-					} elsif ($sum_allowed != $allow &&
-						 $seen != $allow) {
-						CHK("BRACES",
-						    "braces {} should be used on all arms of this statement\n" . $herectx);
-					}
+				if ($seen && !$allowed) {
+					WARN("BRACES",
+					     "braces {} are not necessary for any arm of this statement\n" . $herectx);
 				}
 			}
 		}

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

end of thread, other threads:[~2012-04-25 22:42 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-16  9:17 Please fix or revert: [PATCH] checkpatch: add --strict tests for braces, comments and casts Ingo Molnar
2012-04-16 18:27 ` David Miller
2012-04-16 19:09   ` Linus Torvalds
2012-04-16 19:13     ` David Miller
2012-04-16 20:26   ` Ingo Molnar
2012-04-16 20:34     ` Joe Perches
2012-04-16 20:34     ` Ingo Molnar
2012-04-16 21:00       ` David Miller
2012-04-17  3:30         ` Steven Rostedt
2012-04-25  8:33         ` Ingo Molnar
2012-04-16 20:58     ` David Miller
2012-04-25  8:42       ` Ingo Molnar
2012-04-16 19:44 ` Randy Dunlap
2012-04-16 19:32   ` [PATCH] checkpatch: revert --strict test for net/ and drivers/net block comment style Joe Perches
2012-04-16 19:35   ` Joe Perches
2012-04-16 20:28     ` Ingo Molnar
2012-04-16 20:33       ` Linus Torvalds
2012-04-17 10:07         ` Ingo Molnar
2012-04-25 22:42           ` Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox