This update brings a couple of new checks, and a number of fixes. Of note:
- adds checks for missuse of in_atomic(),
- checks for spacing within stars in pointer types, and
- fixes some comment detection corner cases.
Complete changelog below.
-apw
Andy Whitcroft (9):
checkpatch: add checks for in_atomic()
checkpatch: comment detection may miss an implied comment on the last
hunk
checkpatch: widen implied comment detection to allow multiple stars
checkpatch: structure member assignments are not complex
checkpatch: __weak is an official attribute
checkpatch: detect multiple bitfield declarations
checkpatch: comment ends inside strings is most likely not an open
comment
checkpatch: dissallow spaces between stars in pointer types
checkpatch: version: 0.25
scripts/checkpatch.pl | 92 ++++++++++++++++++++++++++++++++++---------------
1 files changed, 64 insertions(+), 28 deletions(-)
Some people use double star '**' as a comment continuation, and start
comments with complete lines of stars. Widen the implied comment detection
to pick these up.
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 beae539..c28c20c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1109,7 +1109,7 @@ sub process {
# is the start of a diff block and this line starts
# ' *' then it is very likely a comment.
if (!defined $edge &&
- $rawlines[$linenr] =~ m@^.\s* \*(?:\s|$)@)
+ $rawlines[$linenr] =~ m@^.\s*(?:\*\*+| \*)(?:\s|$)@)
{
$in_comment = 1;
}
--
1.6.0.2.711.gf1ba4
Ensure we do not trigger the complex macros checks on structure member
assignment, for example:
#define foo .bar = 10
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c28c20c..5551eb1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2167,9 +2167,10 @@ sub process {
MODULE_PARAM_DESC|
DECLARE_PER_CPU|
DEFINE_PER_CPU|
- __typeof__\(
+ __typeof__\(|
+ \.$Ident\s*=\s*
}x;
- #print "REST<$rest>\n";
+ #print "REST<$rest> dstat<$dstat>\n";
if ($rest ne '') {
if ($rest !~ /while\s*\(/ &&
$dstat !~ /$exceptions/)
--
1.6.0.2.711.gf1ba4
When detecting implied comments from leading stars we may incorrectly
think we have detected an edge one way or the other when we have not
if we drop off the end of the last hunk. Fix this up.
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 dbbf96f..beae539 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1097,8 +1097,8 @@ sub process {
$rawlines[$ln - 1] =~ /^-/);
$cnt--;
#print "RAW<$rawlines[$ln - 1]>\n";
- ($edge) = (defined $rawlines[$ln - 1] &&
- $rawlines[$ln - 1] =~ m@(/\*|\*/)@);
+ last if (!defined $rawlines[$ln - 1]);
+ ($edge) = ($rawlines[$ln - 1] =~ m@(/\*|\*/)@);
last if (defined $edge);
}
if (defined $edge && $edge eq '*/') {
--
1.6.0.2.711.gf1ba4
When we are detecting whether a comment is open when we start a hunk
we check for the first comment edge in the hunk and assume its inverse.
However if the hunk contains something like below, then we will assume
that a comment was open. Update this heuristic to see if the comment edge is
obviously within double quotes and ignore it if so:
foo(" */);
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index c73fd44..5acc48a 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -367,7 +367,7 @@ sub sanitise_line {
}
}
- #print "SQ:$sanitise_quote\n";
+ #print "c<$c> SQ<$sanitise_quote>\n";
if ($off != 0 && $sanitise_quote eq '*/' && $c ne "\t") {
substr($res, $off, 1, $;);
} elsif ($off != 0 && $sanitise_quote && $c ne "\t") {
@@ -1103,8 +1103,11 @@ sub process {
$cnt--;
#print "RAW<$rawlines[$ln - 1]>\n";
last if (!defined $rawlines[$ln - 1]);
- ($edge) = ($rawlines[$ln - 1] =~ m@(/\*|\*/)@);
- last if (defined $edge);
+ if ($rawlines[$ln - 1] =~ m@(/\*|\*/)@ &&
+ $rawlines[$ln - 1] !~ m@"[^"]*(?:/\*|\*/)[^"]*"@) {
+ ($edge) = $1;
+ last;
+ }
}
if (defined $edge && $edge eq '*/') {
$in_comment = 1;
--
1.6.0.2.711.gf1ba4
Add __weak as an official attribute. This tends to be used in a location
where the automated attribute detector misses it.
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5551eb1..90f78ef 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -116,7 +116,8 @@ our $Attribute = qr{
__(?:mem|cpu|dev|)(?:initdata|init)|
____cacheline_aligned|
____cacheline_aligned_in_smp|
- ____cacheline_internodealigned_in_smp
+ ____cacheline_internodealigned_in_smp|
+ __weak
}x;
our $Modifier;
our $Inline = qr{inline|__always_inline|noinline};
--
1.6.0.2.711.gf1ba4
Detect the colons (:) which make up secondary bitfield declarations
and apply binary colon checks. For example the following is common
idiom:
int foo:1,
bar:1;
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 90f78ef..c73fd44 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -844,11 +844,11 @@ sub annotate_values {
$type = 'V';
$av_pending = 'V';
- } elsif ($cur =~ /^($Ident\s*):/) {
- if ($type eq 'E') {
- $av_pend_colon = 'L';
- } elsif ($type eq 'T') {
+ } elsif ($cur =~ /^($Ident\s*):(?:\s*\d+\s*(,|=|;))?/) {
+ if (defined $2 && $type eq 'C' || $type eq 'T') {
$av_pend_colon = 'B';
+ } elsif ($type eq 'E') {
+ $av_pend_colon = 'L';
}
print "IDENT_COLON($1,$type>$av_pend_colon)\n" if ($dbg_values > 1);
$type = 'V';
@@ -866,6 +866,10 @@ sub annotate_values {
$type = 'E';
$av_pend_colon = 'O';
+ } elsif ($cur =~/^(,)/) {
+ print "COMMA($1)\n" if ($dbg_values > 1);
+ $type = 'C';
+
} elsif ($cur =~ /^(\?)/o) {
print "QUESTION($1)\n" if ($dbg_values > 1);
$type = 'N';
@@ -881,7 +885,7 @@ sub annotate_values {
}
$av_pend_colon = 'O';
- } elsif ($cur =~ /^(;|\[)/o) {
+ } elsif ($cur =~ /^(\[)/o) {
print "CLOSE($1)\n" if ($dbg_values > 1);
$type = 'N';
--
1.6.0.2.711.gf1ba4
Disallow spaces within multiple pointer stars (*) in both casts and
definitions. Both of these would now be reported:
(char * *)
char * *foo;
Also now consistently detects and reports the attributes within these
structures making the error report itself clearer.
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++---------------
1 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 5acc48a..7cc7473 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -191,7 +191,7 @@ sub build_types {
}x;
$Type = qr{
$NonptrType
- (?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)?
+ (?:[\s\*]+\s*const|[\s\*]+|(?:\s*\[\s*\])+)?
(?:\s+$Inline|\s+$Modifier)*
}x;
$Declare = qr{(?:$Storage\s+)?$Type};
@@ -1344,7 +1344,7 @@ sub process {
}
# any (foo ... *) is a pointer cast, and foo is a type
- while ($s =~ /\(($Ident)(?:\s+$Sparse)*\s*\*+\s*\)/sg) {
+ while ($s =~ /\(($Ident)(?:\s+$Sparse)*[\s\*]+\s*\)/sg) {
possible($1, "C:" . $s);
}
@@ -1618,21 +1618,39 @@ sub process {
}
# * goes on variable not on type
- if ($line =~ m{\($NonptrType(\*+)(?:\s+const)?\)}) {
- ERROR("\"(foo$1)\" should be \"(foo $1)\"\n" .
- $herecurr);
+ # (char*[ const])
+ if ($line =~ m{\($NonptrType(\s*\*[\s\*]*(?:$Modifier\s*)*)\)}) {
+ my ($from, $to) = ($1, $1);
- } elsif ($line =~ m{\($NonptrType\s+(\*+)(?!\s+const)\s+\)}) {
- ERROR("\"(foo $1 )\" should be \"(foo $1)\"\n" .
- $herecurr);
+ # Should start with a space.
+ $to =~ s/^(\S)/ $1/;
+ # Should not end with a space.
+ $to =~ s/\s+$//;
+ # '*'s should not have spaces between.
+ while ($to =~ s/(.)\s\*/$1\*/) {
+ }
- } elsif ($line =~ m{\b$NonptrType(\*+)(?:\s+(?:$Attribute|$Sparse))?\s+[A-Za-z\d_]+}) {
- ERROR("\"foo$1 bar\" should be \"foo $1bar\"\n" .
- $herecurr);
+ #print "from<$from> to<$to>\n";
+ if ($from ne $to) {
+ ERROR("\"(foo$from)\" should be \"(foo$to)\"\n" . $herecurr);
+ }
+ } elsif ($line =~ m{\b$NonptrType(\s*\*[\s\*]*(?:$Modifier\s*)?)($Ident)}) {
+ my ($from, $to, $ident) = ($1, $1, $2);
- } elsif ($line =~ m{\b$NonptrType\s+(\*+)(?!\s+(?:$Attribute|$Sparse))\s+[A-Za-z\d_]+}) {
- ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" .
- $herecurr);
+ # Should start with a space.
+ $to =~ s/^(\S)/ $1/;
+ # Should not end with a space.
+ $to =~ s/\s+$//;
+ # '*'s should not have spaces between.
+ while ($to =~ s/(.)\s\*/$1\*/) {
+ }
+ # Modifiers should have spaces.
+ $to =~ s/(\b$Modifier$)/$1 /;
+
+ #print "from<$from> to<$to>\n";
+ if ($from ne $to) {
+ ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" . $herecurr);
+ }
}
# # no BUG() or BUG_ON()
--
1.6.0.2.711.gf1ba4
in_atomic() is not for driver use so report any such use as an ERROR.
Also in_atomic() is often used to determine if we may sleep, but it is
not reliable in this use model therefore strongly discourage its use.
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e30bac1..dbbf96f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2466,6 +2466,15 @@ sub process {
last;
}
}
+
+# whine mightly about in_atomic
+ if ($line =~ /\bin_atomic\s*\(/) {
+ if ($realfile =~ m@^drivers/@) {
+ ERROR("do not use in_atomic in drivers\n" . $herecurr);
+ } else {
+ WARN("use of in_atomic() is incorrect outside core kernel code\n" . $herecurr);
+ }
+ }
}
# If we have no input at all, then there is nothing to report on
--
1.6.0.2.711.gf1ba4
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 7cc7473..f56ef04 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,7 +9,7 @@ use strict;
my $P = $0;
$P =~ s@.*/@@g;
-my $V = '0.24';
+my $V = '0.25';
use Getopt::Long qw(:config no_auto_abbrev);
--
1.6.0.2.711.gf1ba4