2008-08-11 20:16:27

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 00/17] checkpatch: update to version 0.22

This update brings some bigger fixes to indent checking, some fixes
for false positives, and some tightening of some tests. Of note:

- major fixes to indent checking including selection of the right output
lines,
- comment detection improved, and
- reporting absolute filenames.

Complete changelog below.

-apw

Andy Whitcroft (17):
checkpatch: square brackets -- exemption for array slices in braces
checkpatch: values: double ampersand may be unary
checkpatch: conditional indent -- labels have different indent rules
checkpatch: switch indent allow plain return
checkpatch: add tests for the attribute matcher
checkpatch: ____cacheline_aligned et al are modifiers
checkpatch: complex macros -- fix up extension handling
checkpatch: fix up comment checks search to scan the entire block
checkpatch: include/asm checks should be anchored
checkpatch: reduce warnings for #include of asm/foo.h to check from
arch/bar.c
checkpatch: report any absolute references to kernel source files
checkpatch: report the real first line of all suspect indents
checkpatch: suspect indent -- skip over preprocessor, label and blank
lines
checkpatch: %Lx tests should hand %% as a literal
checkpatch: report the correct lines for single statement blocks
checkpatch: perform indent checks on perl
checkpatch: version: 0.22


2008-08-11 20:13:31

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 02/17] checkpatch: values: double ampersand may be unary

It is possible to use double ampersand (&&) in unary context where it
means the address of a goto label. Handle spacing for it.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6f821a0..1148213 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -858,7 +858,7 @@ sub annotate_values {
print "CLOSE($1)\n" if ($dbg_values > 1);
$type = 'N';

- } elsif ($cur =~ /^(-(?![->])|\+(?!\+)|\*|\&(?!\&))/o) {
+ } elsif ($cur =~ /^(-(?![->])|\+(?!\+)|\*|\&\&|\&)/o) {
my $variant;

print "OPV($1)\n" if ($dbg_values > 1);
@@ -1634,7 +1634,7 @@ sub process {
# unary operator, or a cast
} elsif ($op eq '!' || $op eq '~' ||
$opv eq '*U' || $opv eq '-U' ||
- $opv eq '&U') {
+ $opv eq '&U' || $opv eq '&&U') {
if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
ERROR("space required before that '$op' $at\n" . $hereptr);
}
--
1.6.0.rc1.258.g80295

2008-08-11 20:14:02

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 05/17] checkpatch: add tests for the attribute matcher

Add support for direct testing of the attribute matcher, add basic tests
for it.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c6782ac..1c032b1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -66,6 +66,7 @@ if ($#ARGV < 0) {
my $dbg_values = 0;
my $dbg_possible = 0;
my $dbg_type = 0;
+my $dbg_attr = 0;
for my $key (keys %debug) {
eval "\${dbg_$key} = '$debug{$key}';"
}
@@ -1367,6 +1368,15 @@ sub process {
}
next;
}
+# TEST: allow direct testing of the attribute matcher.
+ if ($dbg_attr) {
+ if ($line =~ /^.\s*$Attribute\s*$/) {
+ ERROR("TEST: is attr\n" . $herecurr);
+ } elsif ($dbg_attr > 1 && $line =~ /^.+($Attribute)/) {
+ ERROR("TEST: is not attr ($1 is)\n". $herecurr);
+ }
+ next;
+ }

# check for initialisation to aggregates open brace on the next line
if ($prevline =~ /$Declare\s*$Ident\s*=\s*$/ &&
--
1.6.0.rc1.258.g80295

2008-08-11 20:13:45

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 06/17] checkpatch: ____cacheline_aligned et al are modifiers

Add the cacheline alignment modifiers to the attribute lists.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1c032b1..303c363 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -113,7 +113,10 @@ our $Attribute = qr{
const|
__read_mostly|
__kprobes|
- __(?:mem|cpu|dev|)(?:initdata|init)
+ __(?:mem|cpu|dev|)(?:initdata|init)|
+ ____cacheline_aligned|
+ ____cacheline_aligned_in_smp|
+ ____cacheline_internodealigned_in_smp
}x;
our $Modifier;
our $Inline = qr{inline|__always_inline|noinline};
--
1.6.0.rc1.258.g80295

2008-08-11 20:14:22

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 04/17] checkpatch: switch indent allow plain return

It is a common and sane idiom to allow a single return on the end
of a case statement:

switch (...) {
case foo: return bar;
}

Add an exception for this.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1e7d2cd..c6782ac 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1295,7 +1295,11 @@ sub process {
}
}
if ($line =~ /^.\s*(?:case\s*.*|default\s*):/g &&
- $line !~ /\G(?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$/g) {
+ $line !~ /\G(?:
+ (?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$|
+ \s*return\s+
+ )/xg)
+ {
ERROR("trailing statements should be on next line\n" . $herecurr);
}

--
1.6.0.rc1.258.g80295

2008-08-11 20:14:53

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 07/17] checkpatch: complex macros -- fix up extension handling

Only pull in new extension lines where the current contents ends
with a \.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 303c363..586f9a4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1977,8 +1977,8 @@ sub process {
# Extract the remainder of the define (if any) and
# rip off surrounding spaces, and trailing \'s.
$rest = '';
- while ($off != 0 || ($cnt > 0 && $rest =~ /(?:^|\\)\s*$/)) {
- #print "ADDING $off <" . substr($lines[$ln - 1], $off) . ">\n";
+ while ($off != 0 || ($cnt > 0 && $rest =~ /\\\s*$/)) {
+ #print "ADDING cnt<$cnt> $off <" . substr($lines[$ln - 1], $off) . "> rest<$rest>\n";
if ($off != 0 || $lines[$ln - 1] !~ /^-/) {
$rest .= substr($lines[$ln - 1], $off) . "\n";
$cnt--;
--
1.6.0.rc1.258.g80295

2008-08-11 20:14:37

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 01/17] checkpatch: square brackets -- exemption for array slices in braces

It is wholy reasonable to have square brackets representing array slices
in braces on the same line. These should be spaced.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bc67793..6f821a0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1493,11 +1493,13 @@ sub process {

# check for spacing round square brackets; allowed:
# 1. with a type on the left -- int [] a;
-# 2. at the beginning of a line for slice initialisers -- [0..10] = 5,
+# 2. at the beginning of a line for slice initialisers -- [0...10] = 5,
+# 3. inside a curly brace -- = { [0...10] = 5 }
while ($line =~ /(.*?\s)\[/g) {
my ($where, $prefix) = ($-[1], $1);
if ($prefix !~ /$Type\s+$/ &&
- ($where != 0 || $prefix !~ /^.\s+$/)) {
+ ($where != 0 || $prefix !~ /^.\s+$/) &&
+ $prefix !~ /{\s+$/) {
ERROR("space prohibited before open square bracket '['\n" . $herecurr);
}
}
--
1.6.0.rc1.258.g80295

2008-08-11 20:15:22

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 03/17] checkpatch: conditional indent -- labels have different indent rules

Labels have different indent rules and must be ignored when checking
the conditional indent levels. Also correct identify labels in
single statement conditionals.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1148213..1e7d2cd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -782,9 +782,9 @@ sub annotate_values {
}
$type = 'N';

- } elsif ($cur =~ /^(if|while|typeof|__typeof__|for)\b/o) {
+ } elsif ($cur =~ /^(if|while|for)\b/o) {
print "COND($1)\n" if ($dbg_values > 1);
- $av_pending = 'N';
+ $av_pending = 'E';
$type = 'N';

} elsif ($cur =~/^(case)/o) {
@@ -792,7 +792,7 @@ sub annotate_values {
$av_pend_colon = 'C';
$type = 'N';

- } elsif ($cur =~/^(return|else|goto)/o) {
+ } elsif ($cur =~/^(return|else|goto|typeof|__typeof__)\b/o) {
print "KEYWORD($1)\n" if ($dbg_values > 1);
$type = 'N';

@@ -1846,6 +1846,11 @@ sub process {
$check = 0;
}

+ # Ignore the current line if it is label.
+ if ($s =~ /^\s*$Ident\s*:/) {
+ $check = 0;
+ }
+
my (undef, $sindent) = line_stats("+" . $s);

##print "line<$line> prevline<$prevline> indent<$indent> sindent<$sindent> check<$check> continuation<$continuation> s<$s>\n";
--
1.6.0.rc1.258.g80295

2008-08-11 20:15:43

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 09/17] checkpatch: include/asm checks should be anchored

It is possible to have other include/asm paths within the tree which
are not subject to the do not edit checks. Ignore those.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 482768c..0e5af8e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1134,7 +1134,7 @@ sub process {
$realfile = $1;
$realfile =~ s@^[^/]*/@@;

- if ($realfile =~ m@include/asm/@) {
+ if ($realfile =~ m@^include/asm/@) {
ERROR("do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n");
}
next;
--
1.6.0.rc1.258.g80295

2008-08-11 20:15:58

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 08/17] checkpatch: fix up comment checks search to scan the entire block

We are not counting the lines in the block correctly which causes the
comment scan to stop prematurly and thus miss comments which end at the
end of the block. Fix this up.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 586f9a4..482768c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1029,9 +1029,14 @@ sub process {
# edge is a close comment then we must be in a comment
# at context start.
my $edge;
- for (my $ln = $linenr + 1; $ln < ($linenr + $realcnt); $ln++) {
- next if ($line =~ /^-/);
- ($edge) = ($rawlines[$ln - 1] =~ m@(/\*|\*/)@);
+ my $cnt = $realcnt;
+ for (my $ln = $linenr + 1; $cnt > 0; $ln++) {
+ next if (defined $rawlines[$ln - 1] &&
+ $rawlines[$ln - 1] =~ /^-/);
+ $cnt--;
+ #print "RAW<$rawlines[$ln - 1]>\n";
+ ($edge) = (defined $rawlines[$ln - 1] &&
+ $rawlines[$ln - 1] =~ m@(/\*|\*/)@);
last if (defined $edge);
}
if (defined $edge && $edge eq '*/') {
--
1.6.0.rc1.258.g80295

2008-08-11 20:18:22

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 13/17] checkpatch: suspect indent -- skip over preprocessor, label and blank lines

We should skip over and check the lines which follow preprocessor
statements, labels, and blank lines. These all have legitimate reasons
to be indented differently.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 27 ++++++++++++++++-----------
1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e3ae79a..6ddae89 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1434,7 +1434,7 @@ sub process {
if ($s =~ s/^\s*\\//) {
$continuation = 1;
}
- if ($s =~ s/^\s*\n//) {
+ if ($s =~ s/^\s*?\n//) {
$check = 1;
$cond_lines++;
}
@@ -1446,15 +1446,20 @@ sub process {
$check = 0;
}

- # Ignore the current line if its is a preprocessor
- # line.
- if ($s =~ /^\s*#\s*/) {
- $check = 0;
- }
+ my $cond_ptr = -1;
+ while ($cond_ptr != $cond_lines) {
+ $cond_ptr = $cond_lines;

- # Ignore the current line if it is label.
- if ($s =~ /^\s*$Ident\s*:/) {
- $check = 0;
+ # Ignore:
+ # 1) blank lines, they should be at 0,
+ # 2) preprocessor lines, and
+ # 3) labels.
+ if ($s =~ /^\s*?\n/ ||
+ $s =~ /^\s*#\s*?/ ||
+ $s =~ /^\s*$Ident\s*:/) {
+ $s =~ s/^.*?\n//;
+ $cond_lines++;
+ }
}

my (undef, $sindent) = line_stats("+" . $s);
@@ -1470,7 +1475,7 @@ sub process {
$stat_real = "[...]\n$stat_real";
}

- ##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";
+ #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 ||
($sindent <= $indent && $s ne ''))) {
--
1.6.0.rc1.258.g80295

2008-08-11 20:18:38

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 10/17] checkpatch: reduce warnings for #include of asm/foo.h to check from arch/bar.c

It is much more likely that an architecture file will want to directly
include asm header files. Reduce this WARNING to a CHECK when the referencing
file is in the arch directory.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0e5af8e..9e7e9d1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1942,12 +1942,17 @@ sub process {

#warn if <asm/foo.h> is #included and <linux/foo.h> is available (uses RAW line)
if ($tree && $rawline =~ m{^.\s*\#\s*include\s*\<asm\/(.*)\.h\>}) {
- my $checkfile = "include/linux/$1.h";
- if (-f "$root/$checkfile" && $realfile ne $checkfile &&
+ my $file = "$1.h";
+ my $checkfile = "include/linux/$file";
+ if (-f "$root/$checkfile" &&
+ $realfile ne $checkfile &&
$1 ne 'irq')
{
- WARN("Use #include <linux/$1.h> instead of <asm/$1.h>\n" .
- $herecurr);
+ if ($realfile =~ m{^arch/}) {
+ CHK("Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
+ } else {
+ WARN("Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr);
+ }
}
}

--
1.6.0.rc1.258.g80295

2008-08-11 20:18:52

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 11/17] checkpatch: report any absolute references to kernel source files

Absolute references to kernel source files are generally only useful
locally to the originator of the patch. Check for any such references
and report them.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 41 +++++++++++++++++++++++++++++++++++++++++
1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9e7e9d1..cc61cf7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -958,6 +958,33 @@ sub CHK {
}
}

+sub check_absolute_file {
+ my ($absolute, $herecurr) = @_;
+ my $file = $absolute;
+
+ ##print "absolute<$absolute>\n";
+
+ # See if any suffix of this path is a path within the tree.
+ while ($file =~ s@^[^/]*/@@) {
+ if (-f "$root/$file") {
+ ##print "file<$file>\n";
+ last;
+ }
+ }
+ if (! -f _) {
+ return 0;
+ }
+
+ # It is, so see if the prefix is acceptable.
+ my $prefix = $absolute;
+ substr($prefix, -length($file)) = '';
+
+ ##print "prefix<$prefix>\n";
+ if ($prefix ne ".../") {
+ WARN("use relative pathname instead of absolute in changelog text\n" . $herecurr);
+ }
+}
+
sub process {
my $filename = shift;

@@ -1168,6 +1195,20 @@ sub process {
$herecurr) if (!$emitted_corrupt++);
}

+# Check for absolute kernel paths.
+ if ($tree) {
+ while ($line =~ m{(?:^|\s)(/\S*)}g) {
+ my $file = $1;
+
+ if ($file =~ m{^(.*?)(?::\d+)+:?$} &&
+ check_absolute_file($1, $herecurr)) {
+ #
+ } else {
+ check_absolute_file($file, $herecurr);
+ }
+ }
+ }
+
# UTF-8 regex found at http://www.w3.org/International/questions/qa-forms-utf-8.en.php
if (($realfile =~ /^$/ || $line =~ /^\+/) &&
$rawline !~ m/^$UTF8*$/) {
--
1.6.0.rc1.258.g80295

2008-08-11 20:19:20

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 12/17] checkpatch: report the real first line of all suspect indents

We are currently only reporting syspect indents if the conditional is
modified but the indent missmatch could be generated by the body changing,
make sure we catch both. Also only report the first line of the body,
and more importantly make sure we report the raw copy of the line. Finally
report the indent levels to make it easier to understand what is wrong.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 141 ++++++++++++++++++++++++++++++-------------------
1 files changed, 86 insertions(+), 55 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cc61cf7..e3ae79a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -673,6 +673,22 @@ sub ctx_has_comment {
return ($cmt ne '');
}

+sub raw_line {
+ my ($linenr, $cnt) = @_;
+
+ my $offset = $linenr - 1;
+ $cnt++;
+
+ my $line;
+ while ($cnt) {
+ $line = $rawlines[$offset++];
+ next if (defined($line) && $line =~ /^-/);
+ $cnt--;
+ }
+
+ return $line;
+}
+
sub cat_vet {
my ($vet) = @_;
my ($res, $coded);
@@ -1392,6 +1408,76 @@ sub process {
}
}

+# Check relative indent for conditionals and blocks.
+ if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) {
+ my ($s, $c) = ($stat, $cond);
+
+ substr($s, 0, length($c), '');
+
+ # Make sure we remove the line prefixes as we have
+ # none on the first line, and are going to readd them
+ # where necessary.
+ $s =~ s/\n./\n/gs;
+
+ # Find out how long the conditional actually is.
+ my $cond_lines = 0 + $c =~ /\n/gs;
+
+ # We want to check the first line inside the block
+ # starting at the end of the conditional, so remove:
+ # 1) any blank line termination
+ # 2) any opening brace { on end of the line
+ # 3) any do (...) {
+ my $continuation = 0;
+ my $check = 0;
+ $s =~ s/^.*\bdo\b//;
+ $s =~ s/^\s*{//;
+ if ($s =~ s/^\s*\\//) {
+ $continuation = 1;
+ }
+ if ($s =~ s/^\s*\n//) {
+ $check = 1;
+ $cond_lines++;
+ }
+
+ # Also ignore a loop construct at the end of a
+ # preprocessor statement.
+ if (($prevline =~ /^.\s*#\s*define\s/ ||
+ $prevline =~ /\\\s*$/) && $continuation == 0) {
+ $check = 0;
+ }
+
+ # Ignore the current line if its is a preprocessor
+ # line.
+ if ($s =~ /^\s*#\s*/) {
+ $check = 0;
+ }
+
+ # Ignore the current line if it is label.
+ if ($s =~ /^\s*$Ident\s*:/) {
+ $check = 0;
+ }
+
+ my (undef, $sindent) = line_stats("+" . $s);
+ my $stat_real = raw_line($linenr, $cond_lines);
+
+ # Check if either of these lines are modified, else
+ # this is not this patch's fault.
+ if (!defined($stat_real) ||
+ $stat !~ /^\+/ && $stat_real !~ /^\+/) {
+ $check = 0;
+ }
+ if (defined($stat_real) && $cond_lines > 1) {
+ $stat_real = "[...]\n$stat_real";
+ }
+
+ ##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 ||
+ ($sindent <= $indent && $s ne ''))) {
+ WARN("suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n");
+ }
+ }
+
# Track the 'values' across context and added lines.
my $opline = $line; $opline =~ s/^./ /;
my ($curr_values, $curr_vars) =
@@ -1869,61 +1955,6 @@ sub process {
}
}

-# Check relative indent for conditionals and blocks.
- if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) {
- my ($s, $c) = ($stat, $cond);
-
- substr($s, 0, length($c), '');
-
- # Make sure we remove the line prefixes as we have
- # none on the first line, and are going to readd them
- # where necessary.
- $s =~ s/\n./\n/gs;
-
- # We want to check the first line inside the block
- # starting at the end of the conditional, so remove:
- # 1) any blank line termination
- # 2) any opening brace { on end of the line
- # 3) any do (...) {
- my $continuation = 0;
- my $check = 0;
- $s =~ s/^.*\bdo\b//;
- $s =~ s/^\s*{//;
- if ($s =~ s/^\s*\\//) {
- $continuation = 1;
- }
- if ($s =~ s/^\s*\n//) {
- $check = 1;
- }
-
- # Also ignore a loop construct at the end of a
- # preprocessor statement.
- if (($prevline =~ /^.\s*#\s*define\s/ ||
- $prevline =~ /\\\s*$/) && $continuation == 0) {
- $check = 0;
- }
-
- # Ignore the current line if its is a preprocessor
- # line.
- if ($s =~ /^\s*#\s*/) {
- $check = 0;
- }
-
- # Ignore the current line if it is label.
- if ($s =~ /^\s*$Ident\s*:/) {
- $check = 0;
- }
-
- my (undef, $sindent) = line_stats("+" . $s);
-
- ##print "line<$line> prevline<$prevline> indent<$indent> sindent<$sindent> check<$check> continuation<$continuation> s<$s>\n";
-
- if ($check && (($sindent % 8) != 0 ||
- ($sindent <= $indent && $s ne ''))) {
- WARN("suspect code indent for conditional statements\n" . $herecurr);
- }
- }
-
# Check for bitwise tests written as boolean
if ($line =~ /
(?:
--
1.6.0.rc1.258.g80295

2008-08-11 20:19:43

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 14/17] checkpatch: %Lx tests should hand %% as a literal

Ensure that we handle literal %'s correctly when adjacent to a %Lx.

%Lx bad
%%Lx good
%%%Lx bad

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6ddae89..c7980ff 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2392,6 +2392,7 @@ sub process {
my $string;
while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) {
$string = substr($rawline, $-[1], $+[1] - $-[1]);
+ $string =~ s/%%/__/g;
if ($string =~ /(?<!%)%L[udi]/) {
WARN("\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr);
last;
--
1.6.0.rc1.258.g80295

2008-08-11 20:19:58

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 15/17] checkpatch: report the correct lines for single statement blocks

Report the correct lines for single statement blocks. Currently we are
reporting the right number of lines, but not skipping the negative lines.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c7980ff..36825c3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2214,10 +2214,10 @@ sub process {
}
if ($level == 0 && $block =~ /^\s*\{/ && !$allowed) {
my $herectx = $here . "\n";;
- my $end = $linenr + statement_rawlines($block) - 1;
+ my $cnt = statement_rawlines($block);

- for (my $ln = $linenr - 1; $ln < $end; $ln++) {
- $herectx .= $rawlines[$ln] . "\n";;
+ for (my $n = 0; $n < $cnt; $n++) {
+ $herectx .= raw_line($linenr, $n) . "\n";;
}

WARN("braces {} are not necessary for single statement blocks\n" . $herectx);
--
1.6.0.rc1.258.g80295

2008-08-11 20:20:26

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 16/17] checkpatch: perform indent checks on perl

So that we eat our own dog food ensure the indent checks apply to perl too.

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 36825c3..ba677c1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1266,8 +1266,8 @@ sub process {
WARN("adding a line without newline at end of file\n" . $herecurr);
}

-# check we are in a valid source file *.[hc] if not then ignore this hunk
- next if ($realfile !~ /\.[hc]$/);
+# 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.
@@ -1277,6 +1277,9 @@ sub process {
ERROR("code indent should use tabs where possible\n" . $herevet);
}

+# check we are in a valid C source file if not then ignore this hunk
+ next if ($realfile !~ /\.(h|c)$/);
+
# check for RCS/CVS revision markers
if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr);
--
1.6.0.rc1.258.g80295

2008-08-11 20:20:47

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 17/17] checkpatch: version: 0.22

Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ba677c1..66bcedc 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,7 +9,7 @@ use strict;
my $P = $0;
$P =~ s@.*/@@g;

-my $V = '0.21';
+my $V = '0.22';

use Getopt::Long qw(:config no_auto_abbrev);

--
1.6.0.rc1.258.g80295

2008-10-08 13:39:22

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 09/17] checkpatch: include/asm checks should be anchored

On Mon, 11 Aug 2008, Andy Whitcroft wrote:
> It is possible to have other include/asm paths within the tree which
> are not subject to the do not edit checks. Ignore those.
>
> Signed-off-by: Andy Whitcroft <[email protected]>
> ---
> scripts/checkpatch.pl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 482768c..0e5af8e 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -1134,7 +1134,7 @@ sub process {
> $realfile = $1;
> $realfile =~ s@^[^/]*/@@;
>
> - if ($realfile =~ m@include/asm/@) {
> + if ($realfile =~ m@^include/asm/@) {
> ERROR("do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n");
> }
> next;

I just stumbled on the same false positive, and found your patch.
Shouldn't the error message be updated, too?

--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1134,8 +1134,8 @@ sub process {
$realfile = $1;
$realfile =~ s@^[^/]*/@@;

- if ($realfile =~ m@include/asm/@) {
- ERROR("do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n");
+ if ($realfile =~ m@^include/asm/@) {
+ ERROR("do not modify files in include/asm, change architecture specific files in arch/<architecture>/include/asm or include/asm-<architecture>\n" . "$here$rawline\n");
}
next;
}


With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010