This is a pretty minor update, but some things which have been pending
for while now so it seems time to push these up. This version brings a
few new checks and the usual fixes for false positives. Of note:
- add checks for question mark and colon spacing, and
- fixes for the complex macro checks
Complete changelog below.
-apw
Andy Whitcroft (5):
checkpatch: add checks for question mark and colon spacing
checkpatch: variants -- move the main unary/binary operators to use
variants
checkpatch: complex macros need to ignore comments
checkpatch: types cannot start mid word for pointer tests
checkpatch: version 0.21
scripts/checkpatch.pl | 124 +++++++++++++++++++++++++++++++++++++------------
1 files changed, 94 insertions(+), 30 deletions(-)
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 022ee55..bc67793 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,7 +9,7 @@ use strict;
my $P = $0;
$P =~ s@.*/@@g;
-my $V = '0.20';
+my $V = '0.21';
use Getopt::Long qw(:config no_auto_abbrev);
--
1.5.6.1.201.g3e7d3
When checking spacing for pointer checks the type cannot start in the
middle of a word, ie. this is not 'int * bar':
x = fooint * bar;
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 96a762b..022ee55 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1435,11 +1435,11 @@ sub process {
ERROR("\"(foo $1 )\" should be \"(foo $1)\"\n" .
$herecurr);
- } elsif ($line =~ m{$NonptrType(\*+)(?:\s+(?:$Attribute|$Sparse))?\s+[A-Za-z\d_]+}) {
+ } elsif ($line =~ m{\b$NonptrType(\*+)(?:\s+(?:$Attribute|$Sparse))?\s+[A-Za-z\d_]+}) {
ERROR("\"foo$1 bar\" should be \"foo $1bar\"\n" .
$herecurr);
- } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+(?:$Attribute|$Sparse))\s+[A-Za-z\d_]+}) {
+ } elsif ($line =~ m{\b$NonptrType\s+(\*+)(?!\s+(?:$Attribute|$Sparse))\s+[A-Za-z\d_]+}) {
ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" .
$herecurr);
}
--
1.5.6.1.201.g3e7d3
Now that we have a variants system, move to using that to carry the
unary/binary designation for +, -, &, and *.
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 36 ++++++++++++++++++------------------
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 88027f2..8afa88a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -858,6 +858,19 @@ sub annotate_values {
print "CLOSE($1)\n" if ($dbg_values > 1);
$type = 'N';
+ } elsif ($cur =~ /^(-(?![->])|\+(?!\+)|\*|\&(?!\&))/o) {
+ my $variant;
+
+ print "OPV($1)\n" if ($dbg_values > 1);
+ if ($type eq 'V') {
+ $variant = 'B';
+ } else {
+ $variant = 'U';
+ }
+
+ substr($var, length($res), 1, $variant);
+ $type = 'N';
+
} elsif ($cur =~ /^($Operators)/o) {
print "OP($1)\n" if ($dbg_values > 1);
if ($1 ne '++' && $1 ne '--') {
@@ -1573,22 +1586,8 @@ sub process {
my $ptr = substr($blank, 0, $off) . "^";
my $hereptr = "$hereline$ptr\n";
- # Classify operators into binary, unary, or
- # definitions (* only) where they have more
- # than one mode.
+ # Pull out the value of this operator.
my $op_type = substr($curr_values, $off + 1, 1);
- my $op_left = substr($curr_values, $off, 1);
- my $is_unary;
- if ($op_type eq 'T') {
- $is_unary = 2;
- } elsif ($op_left eq 'V') {
- $is_unary = 0;
- } else {
- $is_unary = 1;
- }
- #if ($op eq '-' || $op eq '&' || $op eq '*') {
- # print "UNARY: <$op_left$op_type $is_unary $a:$op:$c> <$ca:$op:$cc> <$unary_ctx>\n";
- #}
# Get the full operator variant.
my $opv = $op . substr($curr_vars, $off, 1);
@@ -1625,18 +1624,19 @@ sub process {
}
# '*' as part of a type definition -- reported already.
- } elsif ($op eq '*' && $is_unary == 2) {
+ } elsif ($opv eq '*_') {
#warn "'*' is part of type\n";
# unary operators should have a space before and
# none after. May be left adjacent to another
# unary operator, or a cast
} elsif ($op eq '!' || $op eq '~' ||
- ($is_unary && ($op eq '*' || $op eq '-' || $op eq '&'))) {
+ $opv eq '*U' || $opv eq '-U' ||
+ $opv eq '&U') {
if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
ERROR("space required before that '$op' $at\n" . $hereptr);
}
- if ($op eq '*' && $cc =~/\s*const\b/) {
+ if ($op eq '*' && $cc =~/\s*const\b/) {
# A unary '*' may be const
} elsif ($ctx =~ /.xW/) {
--
1.5.6.1.201.g3e7d3
Ensure we ignore comments in complex macro detection else we incorrectly
report this:
#define PFM_GROUP_PERM_ANY -1 /* any user/group */
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 8afa88a..96a762b 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1972,6 +1972,7 @@ sub process {
} else {
$dstat =~ s/^.\s*\#\s*define\s+$Ident\s*//;
}
+ $dstat =~ s/$;//g;
$dstat =~ s/\\\n.//g;
$dstat =~ s/^\s*//s;
$dstat =~ s/\s*$//s;
--
1.5.6.1.201.g3e7d3
Add checks for the question mark colon operator spacing, and also check
the other uses of colon. Colon means a number of things:
- it introduces the else part of the ?: operator,
- it terminates a goto label,
- it terminates the case value,
- it separates the identifier from the bit size on bit fields, and
- it is used to introduce option types in asm().
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 81 +++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8a3b0fd..88027f2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -689,17 +689,20 @@ sub cat_vet {
my $av_preprocessor = 0;
my $av_pending;
my @av_paren_type;
+my $av_pend_colon;
sub annotate_reset {
$av_preprocessor = 0;
$av_pending = '_';
@av_paren_type = ('E');
+ $av_pend_colon = 'O';
}
sub annotate_values {
my ($stream, $type) = @_;
my $res;
+ my $var = '_' x length($stream);
my $cur = $stream;
print "$stream\n" if ($dbg_values > 1);
@@ -784,7 +787,12 @@ sub annotate_values {
$av_pending = 'N';
$type = 'N';
- } elsif ($cur =~/^(return|case|else|goto)/o) {
+ } elsif ($cur =~/^(case)/o) {
+ print "CASE($1)\n" if ($dbg_values > 1);
+ $av_pend_colon = 'C';
+ $type = 'N';
+
+ } elsif ($cur =~/^(return|else|goto)/o) {
print "KEYWORD($1)\n" if ($dbg_values > 1);
$type = 'N';
@@ -809,6 +817,15 @@ sub annotate_values {
$type = 'V';
$av_pending = 'V';
+ } elsif ($cur =~ /^($Ident\s*):/) {
+ if ($type eq 'E') {
+ $av_pend_colon = 'L';
+ } elsif ($type eq 'T') {
+ $av_pend_colon = 'B';
+ }
+ print "IDENT_COLON($1,$type>$av_pend_colon)\n" if ($dbg_values > 1);
+ $type = 'V';
+
} elsif ($cur =~ /^($Ident|$Constant)/o) {
print "IDENT($1)\n" if ($dbg_values > 1);
$type = 'V';
@@ -820,8 +837,24 @@ sub annotate_values {
} elsif ($cur =~/^(;|{|})/) {
print "END($1)\n" if ($dbg_values > 1);
$type = 'E';
+ $av_pend_colon = 'O';
+
+ } elsif ($cur =~ /^(\?)/o) {
+ print "QUESTION($1)\n" if ($dbg_values > 1);
+ $type = 'N';
+
+ } elsif ($cur =~ /^(:)/o) {
+ print "COLON($1,$av_pend_colon)\n" if ($dbg_values > 1);
+
+ substr($var, length($res), 1, $av_pend_colon);
+ if ($av_pend_colon eq 'C' || $av_pend_colon eq 'L') {
+ $type = 'E';
+ } else {
+ $type = 'N';
+ }
+ $av_pend_colon = 'O';
- } elsif ($cur =~ /^(;|\?|:|\[)/o) {
+ } elsif ($cur =~ /^(;|\[)/o) {
print "CLOSE($1)\n" if ($dbg_values > 1);
$type = 'N';
@@ -840,7 +873,7 @@ sub annotate_values {
}
}
- return $res;
+ return ($res, $var);
}
sub possible {
@@ -1294,12 +1327,14 @@ sub process {
# Track the 'values' across context and added lines.
my $opline = $line; $opline =~ s/^./ /;
- my $curr_values = annotate_values($opline . "\n", $prev_values);
+ my ($curr_values, $curr_vars) =
+ annotate_values($opline . "\n", $prev_values);
$curr_values = $prev_values . $curr_values;
if ($dbg_values) {
my $outline = $opline; $outline =~ s/\t/ /g;
print "$linenr > .$outline\n";
print "$linenr > $curr_values\n";
+ print "$linenr > $curr_vars\n";
}
$prev_values = substr($curr_values, -1);
@@ -1490,7 +1525,8 @@ sub process {
<<=|>>=|<=|>=|==|!=|
\+=|-=|\*=|\/=|%=|\^=|\|=|&=|
=>|->|<<|>>|<|>|=|!|~|
- &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%
+ &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%|
+ \?|:
}x;
my @elements = split(/($ops|;)/, $opline);
my $off = 0;
@@ -1554,6 +1590,9 @@ sub process {
# print "UNARY: <$op_left$op_type $is_unary $a:$op:$c> <$ca:$op:$cc> <$unary_ctx>\n";
#}
+ # Get the full operator variant.
+ my $opv = $op . substr($curr_vars, $off, 1);
+
# Ignore operators passed as parameters.
if ($op_type ne 'V' &&
$ca =~ /\s$/ && $cc =~ /^\s*,/) {
@@ -1571,8 +1610,10 @@ sub process {
# // is a comment
} elsif ($op eq '//') {
- # -> should have no spaces
- } elsif ($op eq '->') {
+ # No spaces for:
+ # ->
+ # : when part of a bitfield
+ } elsif ($op eq '->' || $opv eq ':B') {
if ($ctx =~ /Wx.|.xW/) {
ERROR("spaces prohibited around that '$op' $at\n" . $hereptr);
}
@@ -1628,11 +1669,33 @@ sub process {
$hereptr);
}
+ # 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("space prohibited before that '$op' $at\n" . $hereptr);
+ }
+
# All the others need spaces both sides.
} elsif ($ctx !~ /[EWC]x[CWE]/) {
+ my $ok = 0;
+
# Ignore email addresses <foo@bar>
- if (!($op eq '<' && $cb =~ /$;\S+\@\S+>/) &&
- !($op eq '>' && $cb =~ /<\S+\@\S+$;/)) {
+ if (($op eq '<' &&
+ $cc =~ /^\S+\@\S+>/) ||
+ ($op eq '>' &&
+ $ca =~ /<\S+\@\S+$/))
+ {
+ $ok = 1;
+ }
+
+ # Ignore ?:
+ if (($opv eq ':O' && $ca =~ /\?$/) ||
+ ($op eq '?' && $cc =~ /^:/)) {
+ $ok = 1;
+ }
+
+ if ($ok == 0) {
ERROR("spaces required around that '$op' $at\n" . $hereptr);
}
}
--
1.5.6.1.201.g3e7d3
On Wednesday 2008-07-02 13:31, Andy Whitcroft wrote:
>Ensure we ignore comments in complex macro detection else we incorrectly
>report this:
>
> #define PFM_GROUP_PERM_ANY -1 /* any user/group */
Does it also catch this ugly piece from dcache.c?
#define DCACHE_NFSFS_RENAMED 0x0002 /* this dentry has been "silly
* renamed" and has to be
* deleted on the last dput()
*/
On Sat, Jul 05, 2008 at 08:44:35PM +0200, Jan Engelhardt wrote:
>
> On Wednesday 2008-07-02 13:31, Andy Whitcroft wrote:
>
> >Ensure we ignore comments in complex macro detection else we incorrectly
> >report this:
> >
> > #define PFM_GROUP_PERM_ANY -1 /* any user/group */
>
> Does it also catch this ugly piece from dcache.c?
>
>
> #define DCACHE_NFSFS_RENAMED 0x0002 /* this dentry has been "silly
> * renamed" and has to be
> * deleted on the last dput()
> */
Yeah it should indeed. A quick check with the current version seems to
confirm that.
-apw