Following this email is the next batch of updates to checkpatch.
This marks a change in how I intend to send updates. The previous
"single delta" update caused us to lose a patch from Greg KH (which is
re-included in this update), bad. By sending the actual patches we avoid
regressing anything I have missed, it also has the advantage of maintaining
individual changes to carry signoff more accurately and generally being
more transparent.
This version brings a few new checks and the usual slew of fixes for
false positives mostly in type detection. Of note:
- allow printk text strings to bust the width restrictions for
grepability,
- major fixes for possible type/modifier handling,
- trailing statement checks for case and default,
- trailing statement checks for for and while,
- spacing checks for square brackets '[' ']',
- conditions/loops indent checks, and
- checks for new use of __initcall.
Complete changelog below.
-apw
Andy Whitcroft (20):
checkpatch: Version: 0.20
checkpatch: return is not a function -- parentheses for casts are ok too
checkpatch: types: some types may also be identifiers
checkpatch: possible types: __asm__ is never a type
checkpatch: comment detection: ignore macro continuation when
detecting associated comments
checkpatch: types: unary -- goto introduces unary context
checkpatch: macros: fix statement counting block end detection
checkpatch: trailing statement indent: fix end of statement location
checkpatch: allow printk strings to exceed 80 characters to
maintain their searchability
checkpatch: switch -- report trailing statements on case and default
checkpatch: check spacing for square brackets
checkpatch: toughen trailing if statement checks and extend them
to while and for
checkpatch: condition/loop indent checks
checkpatch: allow for type modifiers on multiple declarations
checkpatch: improve type matcher ...Fix end of statement location. Where the last line of the statement is
replaced we are miss reporting the newly added replacement an incorrectly
indented trailing statement for the negative context. We are also
incorrectly reporting negative statements generally.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 15 ++++++++++-----
1 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index add8686..89177c3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1249,17 +1249,22 @@ sub process {
my $pre_ctx = "$1$2";
my ($level, @ctx) = ctx_statement_level($linenr, $realcnt, 0);
- my $ctx_ln = $linenr + $#ctx + 1;
my $ctx_cnt = $realcnt - $#ctx - 1;
my $ctx = join("\n", @ctx);
- ##warn "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
+ my $ctx_ln = $linenr;
+ my $ctx_skip = $realcnt;
- # Skip over any removed lines in the context following statement.
- while (defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^-/) {
+ while ($ctx_skip > $ctx_cnt || ($ctx_skip == $ctx_cnt &&
+ defined $lines[$ctx_ln - 1] &&
+ $lines[$ctx_ln - 1] =~ /^-/)) {
+ ##print "SKIP<$ctx_skip> CNT<$ctx_cnt>\n";
+ $ctx_skip-- if (!defined $lines[$ctx_ln - 1] || $lines[$ctx_ln - 1] !~ /^-/);
$ctx_ln++;
}
- ##warn "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
+
+ ##print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
+ ##print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
ERROR("that open brace { should be on the previous line\n" .
--
1.5.6.rc0.140.ga9675
--
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 6971bf0..66f060e 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,7 +9,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.19'; +my $V = '0.20'; use Getopt::Long qw(:config no_auto_abbrev); -- 1.5.6.rc0.140.ga9675 --
We are incorrectly counting the lines in a block while accumulating
the trailing lines in a macro statement, leading to false positives.
Fix end of block handling and general counting for negative context lines.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 23 +++++++++++------------
1 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b2b0648..add8686 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -470,7 +470,9 @@ sub ctx_statement_block {
}
$off++;
}
+ # We are truly at the end, so shuffle to the next line.
if ($off == $len) {
+ $loff = $len + 1;
$line++;
$remain--;
}
@@ -1793,30 +1795,26 @@ sub process {
$lines[$ln - 1] =~ /^(?:-|..*\\$)/)
{
$ctx .= $rawlines[$ln - 1] . "\n";
+ $cnt-- if ($lines[$ln - 1] !~ /^-/);
$ln++;
- $cnt--;
}
$ctx .= $rawlines[$ln - 1];
($dstat, $dcond, $ln, $cnt, $off) =
ctx_statement_block($linenr, $ln - $linenr + 1, 0);
#print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n";
- #print "LINE<$lines[$ln]> len<" . length($lines[$ln]) . "\n";
+ #print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n";
# Extract the remainder of the define (if any) and
# rip off surrounding spaces, and trailing \'s.
$rest = '';
- if (defined $lines[$ln - 1] &&
- $off > length($lines[$ln - 1]))
- {
- $ln++;
- $cnt--;
- $off = 0;
- }
- while ($cnt > 0) {
- $rest .= substr($lines[$ln - 1], $off) . "\n";
+ while ($off != 0 || ($cnt > 0 && $rest =~ /(?:^|\\)\s*$/)) {
+ #print "ADDING $off <" . substr($lines[$ln - 1], $off) . ">\n";
+ if ($off != 0 || $lines[$ln - 1] !~ /^-/) {
+ $rest .= substr($lines[$ln - 1], $off) . "\n";
+ $cnt--;
+ }
$ln++;
- $cnt--;
$off = 0;
}
$rest =~ s/\\\n.//g;
@@ -1847,6 +1845,7 @@ sub process {
DEFINE_PER_CPU|
__typeof__\(
...When looking for an associated comment they may be suffixed by a macro
continuation. Ignore this.
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 fd597a4..94250d1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -631,7 +631,7 @@ sub ctx_locate_comment {
my ($first_line, $end_line) = @_;
# Catch a comment on the end of the line itself.
- my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*$@);
+ my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@);
return $current_comment if (defined $current_comment);
# Look through the context and try and figure out if there is a
--
1.5.6.rc0.140.ga9675
--
Casts require parentheses so it is possible to have something like this:
return (int)(*a);
This miss trips the complexity function. Ensure that the two separate
parenthesised sections are not coelesced.
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 66f060e..83ae37b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1670,6 +1670,7 @@ sub process {
my $value = $2;
# Flatten any parentheses and braces
+ $value =~ s/\)\(/\) \(/g;
while ($value =~ s/\([^\(\)]*\)/1/) {
}
--
1.5.6.rc0.140.ga9675
--
From: Michael Ellerman <michael@ellerman.id.au>
[apw@shadowen.org: generalise pattern and add tests]
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5420db6..cf70f12 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2108,6 +2108,10 @@ sub process {
if ($line =~ /\bsimple_(strto.*?)\s*\(/) {
WARN("consider using strict_$1 in preference to simple_$1\n" . $herecurr);
}
+# check for __initcall(), use device_initcall() explicitly please
+ if ($line =~ /^.\s*__initcall\s*\(/) {
+ WARN("please use device_initcall() instead of __initcall()\n" . $herecurr);
+ }
# use of NR_CPUS is usually wrong
# ignore definitions of NR_CPUS and usage to define arrays as likely right
--
1.5.6.rc0.140.ga9675
--
We are false matching __asm__ as a type, and then tripping the external
function checks. Squash.
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 cf70f12..fd597a4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -846,7 +846,7 @@ sub possible {
if ($possible !~ /^(?:$Storage|$Type|DEFINE_\S+)$/ &&
$possible ne 'goto' && $possible ne 'return' &&
$possible ne 'case' && $possible ne 'else' &&
- $possible ne 'asm' &&
+ $possible ne 'asm' && $possible ne '__asm__' &&
$possible !~ /^(typedef|struct|enum)\b/) {
# Check for modifiers.
$possible =~ s/\s*$Storage\s*//g;
--
1.5.6.rc0.140.ga9675
--
When we see a goto we enter unary context. For example:
goto *h;
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 94250d1..b2b0648 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -780,7 +780,7 @@ sub annotate_values {
$av_pending = 'N';
$type = 'N';
- } elsif ($cur =~/^(return|case|else)/o) {
+ } elsif ($cur =~/^(return|case|else|goto)/o) {
print "KEYWORD($1)\n" if ($dbg_values > 1);
$type = 'N';
--
1.5.6.rc0.140.ga9675
--
Some types such as typedefs may overlap real identifiers. Be more
targetted about when a type can really exist. Where it cannot let it be
an identifier. This prevents false reporting of the minus '-' in unary
context in the following:
foo[bar->bool - 1];
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 83ae37b..5420db6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -171,6 +171,7 @@ our @modifierList = (
sub build_types {
my $mods = "(?: \n" . join("|\n ", @modifierList) . "\n)";
my $all = "(?: \n" . join("|\n ", @typeList) . "\n)";
+ $Modifier = qr{(?:$Attribute|$Sparse|$mods)};
$NonptrType = qr{
(?:const\s+)?
(?:$mods\s+)?
@@ -178,15 +179,14 @@ sub build_types {
(?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\)|
(?:${all}\b)
)
- (?:\s+$Sparse|\s+const)*
+ (?:\s+$Modifier|\s+const)*
}x;
$Type = qr{
$NonptrType
(?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)?
- (?:\s+$Inline|\s+$Sparse|\s+$Attribute|\s+$mods)*
+ (?:\s+$Inline|\s+$Modifier)*
}x;
$Declare = qr{(?:$Storage\s+)?$Type};
- $Modifier = qr{(?:$Attribute|$Sparse|$mods)};
}
build_types();
@@ -715,7 +715,7 @@ sub annotate_values {
$av_preprocessor = 0;
}
- } elsif ($cur =~ /^($Type)/) {
+ } elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\))/) {
print "DECLARE($1)\n" if ($dbg_values > 1);
$type = 'T';
@@ -800,8 +800,9 @@ sub annotate_values {
print "PAREN('$1')\n" if ($dbg_values > 1);
}
- } elsif ($cur =~ /^($Ident)\(/o) {
+ } elsif ($cur =~ /^($Ident)\s*\(/o) {
print "FUNC($1)\n" if ($dbg_values > 1);
+ $type = 'V';
$av_pending = 'V';
} elsif ($cur =~ /^($Ident|$Constant)/o) {
--
1.5.6.rc0.140.ga9675
--
Add support for multiple modifiers such as:
int __one __two foo;
Also handle trailing known modifiers when defecting modifiers:
int __one foo __read_mostly;
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c20916..8a3b0fd 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -859,8 +859,10 @@ sub possible {
} elsif ($possible =~ /\s/) {
$possible =~ s/\s*$Type\s*//g;
- warn "MODIFIER: $possible ($line)\n" if ($dbg_possible);
- push(@modifierList, $possible);
+ for my $modifier (split(' ', $possible)) {
+ warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible);
+ push(@modifierList, $modifier);
+ }
} else {
warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible);
@@ -1186,7 +1188,7 @@ sub process {
} elsif ($s =~ /^.\s*$Ident\s*\(/s) {
# declarations always start with types
- } elsif ($prev_values eq 'E' && $s =~ /^.\s*(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?((?:\s*$Ident)+)\b(?:\s+$Sparse)?\s*\**\s*(?:$Ident|\(\*[^\)]*\))\s*(?:;|=|,|\()/s) {
+ } elsif ($prev_values eq 'E' && $s =~ /^.\s*(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?((?:\s*$Ident)+?)\b(?:\s+$Sparse)?\s*\**\s*(?:$Ident|\(\*[^\)]*\))(?:\s*$Modifier)?\s*(?:;|=|,|\()/s) {
my $type = $1;
$type =~ s/\s+/ /g;
possible($type, "A:" . $s);
--
1.5.6.rc0.140.ga9675
--
Exclude vmlinux.lds.h from the macro complexity checks. They will never
apply sanely here.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 53ec394..775f2b1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1860,7 +1860,8 @@ sub process {
# multi-statement macros should be enclosed in a do while loop, grab the
# first statement and ensure its the whole macro if its not enclosed
# in a known good container
- if ($line =~ /^.\s*\#\s*define\s*$Ident(\()?/) {
+ if ($realfile !~ m@/vmlinux.lds.h$@ &&
+ $line =~ /^.\s*\#\s*define\s*$Ident(\()?/) {
my $ln = $linenr;
my $cnt = $realcnt;
my ($off, $dstat, $dcond, $rest);
--
1.5.6.rc0.140.ga9675
--
Ensure we do not inadvertantly load known modifiers up as possible types.
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 6d07b67..9c20916 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -847,7 +847,7 @@ sub possible {
my ($possible, $line) = @_;
print "CHECK<$possible> ($line)\n" if ($dbg_possible > 1);
- if ($possible !~ /^(?:$Storage|$Type|DEFINE_\S+)$/ &&
+ 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__' &&
--
1.5.6.rc0.140.ga9675
--
From: Greg Kroah-Hartman <gregkh@suse.de>
usb_free_urb() can take a NULL, so let's check and warn about that.
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 13d7a33..a4e8087 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2078,6 +2078,13 @@ sub process {
WARN("kfree(NULL) is safe this check is probabally not required\n" . $hereprev);
}
}
+# check for needless usb_free_urb() checks
+ if ($prevline =~ /\bif\s*\(([^\)]*)\)/) {
+ my $expr = $1;
+ if ($line =~ /\busb_free_urb\(\Q$expr\E\);/) {
+ WARN("usb_free_urb(NULL) is safe this check is probably not required\n" . $hereprev);
+ }
+ }
# warn about #ifdefs in C files
# if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) {
--
1.5.6.rc0.140.ga9675
--
Make sure we correctly mark the return type of the pointer to a function
declaration.
const void *(*sb_tag)(struct sysfs_tag_info *info);
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 775f2b1..6d07b67 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -715,7 +715,7 @@ sub annotate_values {
$av_preprocessor = 0;
}
- } elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\))/) {
+ } elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\)|\()/) {
print "DECLARE($1)\n" if ($dbg_values > 1);
$type = 'T';
--
1.5.6.rc0.140.ga9675
--
Although we are finding the added modifier in the declaration below
we are not correctly matching it as a type. Fix the declaration.
static void __ref *vmem_alloc_pages(unsigned int order)
{
}
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 7 +++----
1 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 077a2ca..53ec394 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -168,12 +168,11 @@ our @modifierList = (
);
sub build_types {
- my $mods = "(?: \n" . join("|\n ", @modifierList) . "\n)";
- my $all = "(?: \n" . join("|\n ", @typeList) . "\n)";
+ my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)";
+ my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)";
$Modifier = qr{(?:$Attribute|$Sparse|$mods)};
$NonptrType = qr{
- (?:const\s+)?
- (?:$mods\s+)?
+ (?:$Modifier\s+|const\s+)*
(?:
(?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\)|
(?:${all}\b)
--
1.5.6.rc0.140.ga9675
--
Allow for type modifiers mid declaration on multiple declarations:
struct mxser_mstatus ms, __user *msu = argp;
Reported by Jiri Slaby.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 3961e75..bcfb8ef 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -721,6 +721,10 @@ sub annotate_values {
print "DECLARE($1)\n" if ($dbg_values > 1);
$type = 'T';
+ } elsif ($cur =~ /^($Modifier)\s*/) {
+ print "MODIFIER($1)\n" if ($dbg_values > 1);
+ $type = 'T';
+
} elsif ($cur =~ /^(\#\s*define\s*$Ident)(\(?)/o) {
print "DEFINE($1,$2)\n" if ($dbg_values > 1);
$av_preprocessor = 1;
--
1.5.6.rc0.140.ga9675
--
Check to see if the block/statement which a condition or loop introduces
is indented correctly.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 59 +++++++++++++++++++++++++++++++++++++++++++++---
1 files changed, 55 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8616bae..13d7a33 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1167,10 +1167,10 @@ sub process {
}
# Check for potential 'bare' types
- my ($stat, $cond);
+ my ($stat, $cond, $line_nr_next, $remain_next);
if ($realcnt && $line =~ /.\s*\S/) {
- ($stat, $cond) = ctx_statement_block($linenr,
- $realcnt, 0);
+ ($stat, $cond, $line_nr_next, $remain_next) =
+ ctx_statement_block($linenr, $realcnt, 0);
$stat =~ s/\n./\n /g;
$cond =~ s/\n./\n /g;
@@ -1712,7 +1712,8 @@ sub process {
ERROR("space required before the open parenthesis '('\n" . $herecurr);
}
-# Check for illegal assignment in if conditional.
+# Check for illegal assignment in if conditional -- and check for trailing
+# statements after the conditional.
if ($line =~ /\b(?:if|while|for)\s*\(/ && $line !~ /^.\s*#/) {
my ($s, $c) = ($stat, $cond);
@@ -1732,6 +1733,56 @@ 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 =~ ...Extend the trailing statement checks to report a trailing semi-colon ';'
as we really want it on the next line and indented so it is really really
obvious. Also extend the tests to include while and for.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e7c8ab1..8616bae 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1269,8 +1269,8 @@ sub process {
$ctx_ln++;
}
- ##print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
- ##print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
+ #print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
+ #print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
ERROR("that open brace { should be on the previous line\n" .
@@ -1713,7 +1713,7 @@ sub process {
}
# Check for illegal assignment in if conditional.
- if ($line =~ /\bif\s*\(/) {
+ if ($line =~ /\b(?:if|while|for)\s*\(/ && $line !~ /^.\s*#/) {
my ($s, $c) = ($stat, $cond);
if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/) {
@@ -1725,8 +1725,8 @@ sub process {
substr($s, 0, length($c), '');
$s =~ s/\n.*//g;
$s =~ s/$;//g; # Remove any comments
- if (length($c) && $s !~ /^\s*({|;|)\s*\\*\s*$/ &&
- $c !~ /^.\s*\#\s*if/)
+ if (length($c) && $s !~ /^\s*{?\s*\\*\s*$/ &&
+ $c !~ /}\s*while\s*/)
{
ERROR("trailing statements should be on next line\n" . $herecurr);
}
--
1.5.6.rc0.140.ga9675
--
Check on the spacing before square brackets. We should only allow spaces
there if this is part of a type definition or an initialialiser.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5f71b30..e7c8ab1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1435,6 +1435,17 @@ sub process {
ERROR("open brace '{' following $1 go on the same line\n" . $hereprev);
}
+# 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,
+ while ($line =~ /(.*?\s)\[/g) {
+ my ($where, $prefix) = ($-[1], $1);
+ if ($prefix !~ /$Type\s+$/ &&
+ ($where != 0 || $prefix !~ /^.\s+$/)) {
+ ERROR("space prohibited before open square bracket '['\n" . $herecurr);
+ }
+ }
+
# check for spaces between functions and their parentheses.
while ($line =~ /($Ident)\s+\(/g) {
my $name = $1;
--
1.5.6.rc0.140.ga9675
--
Report trailing statements on case and default lines.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 614999f..5f71b30 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1244,6 +1244,10 @@ sub process {
ERROR("switch and case should be at the same indent\n$hereline$err");
}
}
+ if ($line =~ /^.\s*(?:case\s*.*|default\s*):/g &&
+ $line !~ /\G(?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$/g) {
+ ERROR("trailing statements should be on next line\n" . $herecurr);
+ }
# if/while/etc brace do not go on next line, unless defining a do while loop,
# or if that brace on the next line is for something else
--
1.5.6.rc0.140.ga9675
--
Allow printk strings to break the 80 character width limits, thus keeping
them complete and searchable.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 89177c3..614999f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1138,7 +1138,9 @@ sub process {
}
#80 column limit
if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ &&
- $rawline !~ /^.\s*\*\s*\@$Ident\s/ && $length > 80)
+ $rawline !~ /^.\s*\*\s*\@$Ident\s/ &&
+ $line !~ /^\+\s*printk\s*\(\s*(?:KERN_\S+\s*)?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ &&
+ $length > 80)
{
WARN("line over 80 characters\n" . $herecurr);
}
--
1.5.6.rc0.140.ga9675
--
From: Wolfram Sang <w.sang@pengutronix.de>
Correct spelling in the kfree reports.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
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 a4e8087..3961e75 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2075,7 +2075,7 @@ sub process {
if ($prevline =~ /\bif\s*\(([^\)]*)\)/) {
my $expr = $1;
if ($line =~ /\bkfree\(\Q$expr\E\);/) {
- WARN("kfree(NULL) is safe this check is probabally not required\n" . $hereprev);
+ WARN("kfree(NULL) is safe this check is probably not required\n" . $hereprev);
}
}
# check for needless usb_free_urb() checks
--
1.5.6.rc0.140.ga9675
--
Improve type matcher debug so we can see what it does match. As part
of this move us to to using the common debug framework.
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
scripts/checkpatch.pl | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bcfb8ef..077a2ca 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -17,7 +17,6 @@ my $quiet = 0;
my $tree = 1;
my $chk_signoff = 1;
my $chk_patch = 1;
-my $tst_type = 0;
my $tst_only;
my $emacs = 0;
my $terse = 0;
@@ -44,7 +43,6 @@ GetOptions(
'summary-file!' => \$summary_file,
'debug=s' => \%debug,
- 'test-type!' => \$tst_type,
'test-only=s' => \$tst_only,
) or exit;
@@ -67,6 +65,7 @@ if ($#ARGV < 0) {
my $dbg_values = 0;
my $dbg_possible = 0;
+my $dbg_type = 0;
for my $key (keys %debug) {
eval "\${dbg_$key} = '$debug{$key}';"
}
@@ -1307,8 +1306,12 @@ sub process {
if ($line=~/^[^\+]/) {next;}
# TEST: allow direct testing of the type matcher.
- if ($tst_type && $line =~ /^.$Declare$/) {
- ERROR("TEST: is type $Declare\n" . $herecurr);
+ if ($dbg_type) {
+ if ($line =~ /^.\s*$Declare\s*$/) {
+ ERROR("TEST: is type\n" . $herecurr);
+ } elsif ($dbg_type > 1 && $line =~ /^.+($Declare)/) {
+ ERROR("TEST: is not type ($1 is)\n". $herecurr);
+ }
next;
}
--
1.5.6.rc0.140.ga9675
--
