[Qemu-devel] [PATCH v2 3/3] checkpatch: adjust to QEMUisms

[Qemu-devel] [PATCH v2 3/3] checkpatch: adjust to QEMUisms

[Blue Swirl]

[-- Attachment #1: Type: text/plain, Size: 5840 bytes --]

Change checkpatch.pl for QEMU use:
 - Root directory detection
 - Forbid tabs
 - Indent at 4 spaces
 - Allow typedefs
 - Enforce brace use even for single statement blocks
 - Don't suggest nonexistent cleanup tools

Mention the script in CODING_STYLE.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
v2: fix false positives for #if directives.

 CODING_STYLE          |    3 ++
 scripts/checkpatch.pl |   64 +++++++++++++-----------------------------------
 2 files changed, 21 insertions(+), 46 deletions(-)
 mode change 100644 => 100755 scripts/checkpatch.pl

diff --git a/CODING_STYLE b/CODING_STYLE
index 2c8268d..5ecfa22 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -1,6 +1,9 @@
 Qemu Coding Style
 =================

+Please use the script checkpatch.pl in the scripts directory to check
+patches before submitting.
+
 1. Whitespace

 Of course, the most important aspect in any coding style is whitespace.
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
old mode 100644
new mode 100755
index 2a8a0e2..16a0822
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -328,9 +328,9 @@ sub top_of_kernel_tree {
 	my ($root) = @_;

 	my @tree_check = (
-		"COPYING", "CREDITS", "Kbuild", "MAINTAINERS", "Makefile",
-		"README", "Documentation", "arch", "include", "drivers",
-		"fs", "init", "ipc", "kernel", "lib", "scripts",
+		"COPYING", "MAINTAINERS", "Makefile",
+		"README", "docs", "VERSION",
+		"vl.c"
 	);

 	foreach my $check (@tree_check) {
@@ -1494,31 +1494,13 @@ sub process {
 # check we are in a valid source file C or perl if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|pl)$/);

-# at the beginning of a line any tabs must come first and anything
-# more than 8 must use tabs.
-		if ($rawline =~ /^\+\s* \t\s*\S/ ||
-		    $rawline =~ /^\+\s*        \s*/) {
+# in QEMU, no tabs are allowed
+		if ($rawline =~ /\t/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			ERROR("code indent should use tabs where possible\n" . $herevet);
+			ERROR("code indent should never use tabs\n" . $herevet);
 			$rpt_cleaners = 1;
 		}

-# check for space before tabs.
-		if ($rawline =~ /^\+/ && $rawline =~ / \t/) {
-			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			WARN("please, no space before tabs\n" . $herevet);
-		}
-
-# check for spaces at the beginning of a line.
-# Exceptions:
-#  1) within comments
-#  2) indented preprocessor commands
-#  3) hanging labels
-		if ($rawline =~ /^\+ / && $line !~ /\+ *(?:$;|#|$Ident:)/)  {
-			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-			WARN("please, no spaces at the start of a line\n" . $herevet);
-		}
-
 # check we are in a valid C source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c)$/);

@@ -1746,7 +1728,7 @@ sub process {

 			#print "line<$line> prevline<$prevline> indent<$indent>
sindent<$sindent> check<$check> continuation<$continuation> s<$s>
cond_lines<$cond_lines> stat_real<$stat_real> stat<$stat>\n";

-			if ($check && (($sindent % 8) != 0 ||
+			if ($check && (($sindent % 4) != 0 ||
 			    ($sindent <= $indent && $s ne ''))) {
 				WARN("suspect code indent for conditional statements ($indent,
$sindent)\n" . $herecurr . "$stat_real\n");
 			}
@@ -1869,16 +1851,6 @@ sub process {
 				$herecurr);
 		}

-# check for new typedefs, only function parameters and sparse annotations
-# make sense.
-		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);
-		}
-
 # * goes on variable not on type
 		# (char*[ const])
 		if ($line =~ m{\($NonptrType(\s*(?:$Modifier\b\s*|\*\s*)+)\)}) {
@@ -2514,13 +2486,13 @@ sub process {
 			WARN("vmlinux.lds.h needs VMLINUX_SYMBOL() around C-visible
symbols\n" . $herecurr);
 		}

-# check for redundant bracing round if etc
-		if ($line =~ /(^.*)\bif\b/ && $1 !~ /else\s*$/) {
+# check for missing bracing round if etc
+		if ($line =~ /(^.*)\bif\b/ && $line !~ /\#\s*if/) {
 			my ($level, $endln, @chunks) =
 				ctx_statement_full($linenr, $realcnt, 1);
 			#print "chunks<$#chunks> linenr<$linenr> endln<$endln> level<$level>\n";
 			#print "APW: <<$chunks[1][0]>><<$chunks[1][1]>>\n";
-			if ($#chunks > 0 && $level == 0) {
+			if ($#chunks >= 0 && $level == 0) {
 				my $allowed = 0;
 				my $seen = 0;
 				my $herectx = $here . "\n";
@@ -2558,8 +2530,8 @@ sub process {
 						$allowed = 1;
 					}
 				}
-				if ($seen && !$allowed) {
-					WARN("braces {} are not necessary for any arm of this
statement\n" . $herectx);
+				if (!$seen) {
+					WARN("braces {} are necessary for all arms of this statement\n"
. $herectx);
 				}
 			}
 		}
@@ -2605,7 +2577,7 @@ sub process {
 					$allowed = 1;
 				}
 			}
-			if ($level == 0 && $block =~ /^\s*\{/ && !$allowed) {
+			if ($level == 0 && !$block =~ /^\s*\{/ && !$allowed) {
 				my $herectx = $here . "\n";;
 				my $cnt = statement_rawlines($block);

@@ -2613,7 +2585,7 @@ sub process {
 					$herectx .= raw_line($linenr, $n) . "\n";;
 				}

-				WARN("braces {} are not necessary for single statement blocks\n"
. $herectx);
+				WARN("braces {} are necessary even for single statement blocks\n"
. $herectx);
 			}
 		}

@@ -2918,10 +2890,10 @@ sub process {
 	if ($quiet == 0) {
 		# If there were whitespace errors which cleanpatch can fix
 		# then suggest that.
-		if ($rpt_cleaners) {
-			print "NOTE: whitespace errors detected, you may wish to use
scripts/cleanpatch or\n";
-			print "      scripts/cleanfile\n\n";
-		}
+#		if ($rpt_cleaners) {
+#			print "NOTE: whitespace errors detected, you may wish to use
scripts/cleanpatch or\n";
+#			print "      scripts/cleanfile\n\n";
+#		}
 	}

 	if ($clean == 1 && $quiet == 0) {
-- 
1.6.2.4

[-- Attachment #2: 0003-checkpatch-adjust-to-QEMUisms.patch --]
[-- Type: application/mbox, Size: 6009 bytes --]

[Qemu-devel] Re: [PATCH v2 3/3] checkpatch: adjust to QEMUisms

[Paolo Bonzini]

On 01/15/2011 06:45 PM, Blue Swirl wrote:
> +			if ($level == 0 && !$block =~ /^\s*\{/ && !$allowed) {

I'm not a Perl expert at all, but I think you need parentheses for the 
argument of "!":

  if ($level == 0 && !($block =~ /^\s*\{/) && !$allowed) {

Paolo

[Qemu-devel] Re: [PATCH v2 3/3] checkpatch: adjust to QEMUisms

[Blue Swirl]

On Mon, Jan 17, 2011 at 7:40 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 01/15/2011 06:45 PM, Blue Swirl wrote:
>>
>> +                       if ($level == 0 && !$block =~ /^\s*\{/ &&
>> !$allowed) {
>
> I'm not a Perl expert at all, but I think you need parentheses for the
> argument of "!":

! has higher precedence than =~:
http://perldoc.perl.org/perlop.html#Operator-Precedence-and-Associativity

>  if ($level == 0 && !($block =~ /^\s*\{/) && !$allowed) {

Maybe instead:
if ($level == 0 && $block !~ /^\s*\{/ && !$allowed) {

[Qemu-devel] Re: [PATCH v2 3/3] checkpatch: adjust to QEMUisms

[Paolo Bonzini]

On 01/17/2011 08:37 PM, Blue Swirl wrote:
> On Mon, Jan 17, 2011 at 7:40 AM, Paolo Bonzini<pbonzini@redhat.com>  wrote:
>> On 01/15/2011 06:45 PM, Blue Swirl wrote:
>>>
>>> +                       if ($level == 0&&  !$block =~ /^\s*\{/&&
>>> !$allowed) {
>>
>> I'm not a Perl expert at all, but I think you need parentheses for the
>> argument of "!":
>
> ! has higher precedence than =~:
> http://perldoc.perl.org/perlop.html#Operator-Precedence-and-Associativity

I think that's what I meant. :)

>>   if ($level == 0&&  !($block =~ /^\s*\{/)&&  !$allowed) {
>
> Maybe instead:
> if ($level == 0&&  $block !~ /^\s*\{/&&  !$allowed) {

Yes, this too.

Paolo