Here is a batch of checkpatch updates. The second of which should fix
akpm's issue triggering perl warnings on some patches. The rest are the
normal slew of little fixes. These have passed testing here.
Complete changelog below.
-apw
Andy Whitcroft (7):
checkpatch: possible types -- prevent illegal modifiers being added
checkpatch: correctly stop scanning at the bottom of a hunk
checkpatch: update copyright dates
checkpatch: fix false errors due to macro concatenation
checkpatch: fix __attribute__ matching
checkpatch: fix false EXPORT_SYMBOL warning
checkpatch: version 0.30
scripts/checkpatch.pl | 84 +++++++++++++++++++++++++++++++++++-------------
1 files changed, 61 insertions(+), 23 deletions(-)
Prevent known non types being detected as modifiers. Ensure we do not
look at any type which starts with a keyword.
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 87bbb8b..b43e309 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -997,23 +997,25 @@ sub annotate_values {
sub possible {
my ($possible, $line) = @_;
-
- print "CHECK<$possible> ($line)\n" if ($dbg_possible > 2);
- if ($possible !~ /(?:
+ my $notPermitted = qr{(?:
^(?:
$Modifier|
$Storage|
$Type|
- DEFINE_\S+|
+ DEFINE_\S+
+ )$|
+ ^(?:
goto|
return|
case|
else|
asm|__asm__|
do
- )$|
+ )(?:\s|$)|
^(?:typedef|struct|enum)\b
- )/x) {
+ )}x;
+ warn "CHECK<$possible> ($line)\n" if ($dbg_possible > 2);
+ if ($possible !~ $notPermitted) {
# Check for modifiers.
$possible =~ s/\s*$Storage\s*//g;
$possible =~ s/\s*$Sparse\s*//g;
@@ -1022,8 +1024,10 @@ sub possible {
} elsif ($possible =~ /\s/) {
$possible =~ s/\s*$Type\s*//g;
for my $modifier (split(' ', $possible)) {
- warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible);
- push(@modifierList, $modifier);
+ if ($modifier !~ $notPermitted) {
+ warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible);
+ push(@modifierList, $modifier);
+ }
}
} else {
--
1.6.3.3
We are allowing context scanning checks to apply against the first line
of context outside at the end of the hunk. This can lead to false
matches to patch names leading to various perl warnings. Correctly stop
at the bottom of the hunk.
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 b43e309..1eca1e1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1234,7 +1234,6 @@ sub process {
$linenr++;
my $rawline = $rawlines[$linenr - 1];
- my $hunk_line = ($realcnt != 0);
#extract the line range in the file after the patch is applied
if ($line=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
@@ -1274,6 +1273,8 @@ sub process {
$realcnt--;
}
+ my $hunk_line = ($realcnt != 0);
+
#make up the handle for any error we report on this line
$prefix = "$filename:$realline: " if ($emacs && $file);
$prefix = "$filename:$linenr: " if ($emacs && !$file);
--
1.6.3.3
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 1eca1e1..05b10d6 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2,7 +2,7 @@
# (c) 2001, Dave Jones. <[email protected]> (the file handling bit)
# (c) 2005, Joel Schopp <[email protected]> (the ugly bit)
# (c) 2007,2008, Andy Whitcroft <[email protected]> (new conditions, test suite)
-# (c) 2008, Andy Whitcroft <[email protected]>
+# (c) 2008,2009, Andy Whitcroft <[email protected]>
# Licensed under the terms of the GNU GPL License version 2
use strict;
--
1.6.3.3
The macro concatenation (##) sequence can cause false errors when checking
macro's. Checkpatch doesn't currently know about the operator.
For example this line,
+ entry = (struct ftrace_raw_##call *)raw_data; \
is correct but it produces the following error,
ERROR: need consistent spacing around '*' (ctx:WxB)
+ entry = (struct ftrace_raw_##call *)raw_data;\
^
The line above doesn't have any spacing problems, and if you remove the
macro concatenation sequence checkpatch doesn't give any errors.
Extend identifier handling to include ## concatenation within the
definition of an identifier.
Reported-by: Daniel Walker <[email protected]>
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 05b10d6..cb2dac3 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -130,7 +130,10 @@ if ($tree) {
my $emitted_corrupt = 0;
-our $Ident = qr{[A-Za-z_][A-Za-z\d_]*};
+our $Ident = qr{
+ [A-Za-z_][A-Za-z\d_]*
+ (?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
+ }x;
our $Storage = qr{extern|static|asmlinkage};
our $Sparse = qr{
__user|
--
1.6.3.3
In the following code,
union thread_union init_thread_union
__attribute__((__section__(".data.init_task"))) =
{ INIT_THREAD_INFO(init_task) };
There is a non-conforming declaration. It should really be like the
following,
union thread_union init_thread_union
__attribute__((__section__(".data.init_task"))) = {
INIT_THREAD_INFO(init_task)
};
However, checkpatch doesn't catch this right now because it doesn't
correctly evaluate the "__attribute__".
It is not at all clear that we care what preceeds an assignment style
attribute when we find the open brace. Relax the test so we do not need
to check the __attribute__.
Reported-by: Daniel Walker <[email protected]>
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 cb2dac3..ba105a8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1669,8 +1669,8 @@ sub process {
}
# check for initialisation to aggregates open brace on the next line
- if ($prevline =~ /$Declare\s*$Ident\s*=\s*$/ &&
- $line =~ /^.\s*{/) {
+ if ($line =~ /^.\s*{/ &&
+ $prevline =~ /(?:^|[^=])=\s*$/) {
ERROR("that open brace { should be on the previous line\n" . $hereprev);
}
--
1.6.3.3
Ingo reported that the following lines triggered a false warning,
static struct lock_class_key rcu_lock_key;
struct lockdep_map rcu_lock_map =
STATIC_LOCKDEP_MAP_INIT("rcu_read_lock", &rcu_lock_key);
EXPORT_SYMBOL_GPL(rcu_lock_map);
from kernel/rcutree.c , and the false warning looked like this,
WARNING: EXPORT_SYMBOL(foo); should immediately follow its
function/variable
+EXPORT_SYMBOL_GPL(rcu_lock_map);
We actually should be checking the statement before the EXPORT_* for a
mention of the exported object, and complain where it is not there.
Cc: Ingo Molnar <[email protected]>
Cc: Paul E. McKenney <[email protected]>
Reported-by: Daniel Walker <[email protected]>
Signed-off-by: Andy Whitcroft <[email protected]>
---
scripts/checkpatch.pl | 48 +++++++++++++++++++++++++++++++++++++++---------
1 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ba105a8..07678c1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1145,6 +1145,7 @@ sub process {
# suppression flags
my %suppress_ifbraces;
my %suppress_whiletrailers;
+ my %suppress_export;
# Pre-scan the patch sanitizing the lines.
# Pre-scan the patch looking for any __setup documentation.
@@ -1253,6 +1254,7 @@ sub process {
%suppress_ifbraces = ();
%suppress_whiletrailers = ();
+ %suppress_export = ();
next;
# track the line number as we move through the hunk, note that
@@ -1428,13 +1430,22 @@ sub process {
}
# Check for potential 'bare' types
- my ($stat, $cond, $line_nr_next, $remain_next, $off_next);
+ my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
+ $realline_next);
if ($realcnt && $line =~ /.\s*\S/) {
($stat, $cond, $line_nr_next, $remain_next, $off_next) =
ctx_statement_block($linenr, $realcnt, 0);
$stat =~ s/\n./\n /g;
$cond =~ s/\n./\n /g;
+ # Find the real next line.
+ $realline_next = $line_nr_next;
+ if (defined $realline_next &&
+ (!defined $lines[$realline_next - 1] ||
+ substr($lines[$realline_next - 1], $off_next) =~ /^\s*$/)) {
+ $realline_next++;
+ }
+
my $s = $stat;
$s =~ s/{.*$//s;
@@ -1695,21 +1706,40 @@ sub process {
$line =~ s@//.*@@;
$opline =~ s@//.*@@;
-#EXPORT_SYMBOL should immediately follow its function closing }.
- if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) ||
- ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+# EXPORT_SYMBOL should immediately follow the thing it is exporting, consider
+# the whole statement.
+#print "APW <$lines[$realline_next - 1]>\n";
+ if (defined $realline_next &&
+ exists $lines[$realline_next - 1] &&
+ !defined $suppress_export{$realline_next} &&
+ ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
+ $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
my $name = $1;
- if ($prevline !~ /(?:
- ^.}|
+ if ($stat !~ /(?:
+ \n.}\s*$|
^.DEFINE_$Ident\(\Q$name\E\)|
^.DECLARE_$Ident\(\Q$name\E\)|
^.LIST_HEAD\(\Q$name\E\)|
- ^.$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
- \b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=|\[)
+ ^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(|
+ \b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\()
)/x) {
- WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
+#print "FOO A<$lines[$realline_next - 1]> stat<$stat> name<$name>\n";
+ $suppress_export{$realline_next} = 2;
+ } else {
+ $suppress_export{$realline_next} = 1;
}
}
+ if (!defined $suppress_export{$linenr} &&
+ $prevline =~ /^.\s*$/ &&
+ ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
+ $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
+#print "FOO B <$lines[$linenr - 1]>\n";
+ $suppress_export{$linenr} = 2;
+ }
+ if (defined $suppress_export{$linenr} &&
+ $suppress_export{$linenr} == 2) {
+ WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
+ }
# check for external initialisers.
if ($line =~ /^.$Type\s*$Ident\s*(?:\s+$Modifier)*\s*=\s*(0|NULL|false)\s*;/) {
--
1.6.3.3
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 07678c1..9f5c1b7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -10,7 +10,7 @@ use strict;
my $P = $0;
$P =~ s@.*/@@g;
-my $V = '0.29';
+my $V = '0.30';
use Getopt::Long qw(:config no_auto_abbrev);
--
1.6.3.3
On Fri, 2009-10-16 at 19:39 +0100, Andy Whitcroft wrote:
> Here is a batch of checkpatch updates. The second of which should fix
> akpm's issue triggering perl warnings on some patches. The rest are the
> normal slew of little fixes. These have passed testing here.
Andy, did you get my patches to checkpatch
or did you decide not to apply them?
http://patchwork.kernel.org/patch/51566/
http://patchwork.kernel.org/patch/51486/
On Fri, 2009-10-16 at 19:39 +0100, Andy Whitcroft wrote:
> - WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
> +#print "FOO A<$lines[$realline_next - 1]> stat<$stat> name<$name>\n";
> + $suppress_export{$realline_next} = 2;
> + } else {
> + $suppress_export{$realline_next} = 1;
> }
> }
> + if (!defined $suppress_export{$linenr} &&
> + $prevline =~ /^.\s*$/ &&
> + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
> + $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
ERROR: code indent should use tabs where possible
#148: FILE: scripts/checkpatch.pl:1734:
+ ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||$
This one line is spaced over instead of tabbed.. I guess checkpatch can
even check itself, which is nice .. I'm not sure this matters since this
it's perl tho..
Do you apply any kind of coding style to checkpatch itself ?
Daniel
On Fri, Oct 16, 2009 at 11:58:21AM -0700, Daniel Walker wrote:
> On Fri, 2009-10-16 at 19:39 +0100, Andy Whitcroft wrote:
>
> > - WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr);
> > +#print "FOO A<$lines[$realline_next - 1]> stat<$stat> name<$name>\n";
> > + $suppress_export{$realline_next} = 2;
> > + } else {
> > + $suppress_export{$realline_next} = 1;
> > }
> > }
> > + if (!defined $suppress_export{$linenr} &&
> > + $prevline =~ /^.\s*$/ &&
> > + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||
> > + $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
>
> ERROR: code indent should use tabs where possible
> #148: FILE: scripts/checkpatch.pl:1734:
> + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ ||$
>
> This one line is spaced over instead of tabbed.. I guess checkpatch can
> even check itself, which is nice .. I'm not sure this matters since this
> it's perl tho..
>
> Do you apply any kind of coding style to checkpatch itself ?
Yeah normally I checkpatch it, and ignore the width issues as the long
regexps are not always sensibly wrappable. I rushed this lot out as I
am travelling tommorrow. Sigh. Never rush out a set of patches.
-apw
On Fri, Oct 16, 2009 at 11:56:39AM -0700, Joe Perches wrote:
> On Fri, 2009-10-16 at 19:39 +0100, Andy Whitcroft wrote:
> > Here is a batch of checkpatch updates. The second of which should fix
> > akpm's issue triggering perl warnings on some patches. The rest are the
> > normal slew of little fixes. These have passed testing here.
>
> Andy, did you get my patches to checkpatch
> or did you decide not to apply them?
>
> http://patchwork.kernel.org/patch/51566/
> http://patchwork.kernel.org/patch/51486/
Missed them, I'll hoover them up tommorrow.
Thanks for the prod.
-apw