From 691d77b6b85c20e4166bafd12bd0131b28f95a16 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:16 -0800 Subject: checkpatch: add checks for in_atomic() 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f88bb3e21cd..826cdbac011 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 -- cgit v1.2.3 From 721c1cb60e0546d2e71b9aa50426c94e69c6521a Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:16 -0800 Subject: checkpatch: comment detection may miss an implied comment on the last hunk 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 826cdbac011..7c17e95bf36 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 '*/') { -- cgit v1.2.3 From 83242e0c239aaa33e757584605f788ac1eca2f0f Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:17 -0800 Subject: checkpatch: widen implied comment detection to allow multiple stars 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7c17e95bf36..a305aa52b8b 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; } -- cgit v1.2.3 From 383099fd636deacf698b91b4c96d0d6d01e6dc79 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:18 -0800 Subject: checkpatch: structure member assignments are not complex Ensure we do not trigger the complex macros checks on structure member assignment, for example: #define foo .bar = 10 Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a305aa52b8b..9208ec64af9 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/) -- cgit v1.2.3 From 5fe3af119bed58d240e2097fe76f322ab51902d7 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:18 -0800 Subject: checkpatch: __weak is an official attribute 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9208ec64af9..c79abb41793 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}; -- cgit v1.2.3 From 8e761b04a34288a3b0b29c0f49cdf157d7db8863 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:19 -0800 Subject: checkpatch: detect multiple bitfield declarations 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c79abb41793..9883de38b44 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'; -- cgit v1.2.3 From fae17daed7312bae708df0cce7e93971308698b5 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:20 -0800 Subject: checkpatch: comment ends inside strings is most likely not an open comment 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9883de38b44..45a97c9f4c9 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; -- cgit v1.2.3 From 65863862ba112bf4d06d5ebc142b9d746d1ee955 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:21 -0800 Subject: checkpatch: dissallow spaces between stars in pointer types 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 Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 45a97c9f4c9..85078367427 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() -- cgit v1.2.3 From 50a7dcfb5062a5d9a0dcb28943a1e0cd5f0f60c2 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:21 -0800 Subject: checkpatch: version: 0.25 Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 85078367427..770fa4101f4 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); -- cgit v1.2.3 From 2a5a2c25224e26c5ee491af0dc5d39e4a16f619c Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:23 -0800 Subject: checkpatch: update copyrights Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 770fa4101f4..f0156984796 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1,7 +1,8 @@ #!/usr/bin/perl -w # (c) 2001, Dave Jones. (the file handling bit) # (c) 2005, Joel Schopp (the ugly bit) -# (c) 2007, Andy Whitcroft (new conditions, test suite, etc) +# (c) 2007,2008, Andy Whitcroft (new conditions, test suite) +# (c) 2008, Andy Whitcroft # Licensed under the terms of the GNU GPL License version 2 use strict; -- cgit v1.2.3 From 1e85572697b348b1a126520349a29654f2ae6a12 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Tue, 6 Jan 2009 14:41:24 -0800 Subject: checkpatch: Add warning for p0-patches Some people work internally with -p0-patches which has the danger that one forgets to convert them to -p1 before mainlining. Bitten myself and seen p0-patches in mailing lists occasionally, this patch adds a warning to checkpatch.pl in case a patch is -p0. If you really want, you can fool this check to generate false positives, this is why it just spits a warning. Making the check 100% proof is trickier than it looks, so let's start with a version which catches the cases of real use. [apw@canonical.com: update message language, handle null prefix, add tests] Signed-off-by: Wolfram Sang Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f0156984796..b953c76be36 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1057,6 +1057,7 @@ sub process { my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; + my $p1_prefix = ''; my $prev_values = 'E'; @@ -1205,7 +1206,12 @@ sub process { # extract the filename as it passes if ($line=~/^\+\+\+\s+(\S+)/) { $realfile = $1; - $realfile =~ s@^[^/]*/@@; + $realfile =~ s@^([^/]*)/@@; + + $p1_prefix = $1; + if ($tree && $p1_prefix ne '' && -e "$root/$p1_prefix") { + WARN("patch prefix '$p1_prefix' exists, appears to be a -p0 patch\n"); + } if ($realfile =~ m@^include/asm/@) { ERROR("do not modify files in include/asm, change architecture specific files in include/asm-\n" . "$here$rawline\n"); -- cgit v1.2.3 From 86f9d059c6bc548f6337f01117897a4c823cb4ee Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:24 -0800 Subject: checkpatch: allow parentheses on return for comparisons It seems to be a common idiom to include braces on conditionals in all contexts including return. Allow this exception to the return is not a function checks. Reported by Kay Sievers. Cc: Kay Sievers Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b953c76be36..5c7fd1a4f93 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -127,6 +127,7 @@ our $Lval = qr{$Ident(?:$Member)*}; our $Constant = qr{(?:[0-9]+|0x[0-9a-fA-F]+)[UL]*}; our $Assignment = qr{(?:\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=)}; +our $Compare = qr{<=|>=|==|!=|<|>}; our $Operators = qr{ <=|>=|==|!=| =>|->|<<|>>|<|>|!|~| @@ -1983,9 +1984,9 @@ sub process { my $spacing = $1; my $value = $2; - # Flatten any parentheses and braces + # Flatten any parentheses $value =~ s/\)\(/\) \(/g; - while ($value =~ s/\([^\(\)]*\)/1/) { + while ($value !~ /(?:$Ident|-?$Constant)\s*$Compare\s*(?:$Ident|-?$Constant)/ && $value =~ s/\([^\(\)]*\)/1/) { } if ($value =~ /^(?:$Ident|-?$Constant)$/) { -- cgit v1.2.3 From 080ba929651a32f1840751d2b862e64c8ee1f0c6 Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Tue, 6 Jan 2009 14:41:25 -0800 Subject: checkpatch: try to catch missing VMLINUX_SYMBOL() in vmlinux.lds.h Seems like every other release we have someone who updates vmlinux.lds.h and adds C-visible symbols without VMLINUX_SYMBOL() around them. So start checking the file and reject assignments which have plain symbols on either side. [apw@canonical.com: soften the check, add tests] Signed-off-by: Mike Frysinger Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 5c7fd1a4f93..705a0439b39 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2224,6 +2224,15 @@ sub process { } } +# make sure symbols are always wrapped with VMLINUX_SYMBOL() ... +# all assignments may have only one of the following with an assignment: +# . +# ALIGN(...) +# VMLINUX_SYMBOL(...) + if ($realfile eq 'vmlinux.lds.h' && $line =~ /(?:(?:^|\s)$Ident\s*=|=\s*$Ident(?:\s|$))/) { + WARN("vmlinux.lds.h needs VMLINUX_SYMBOL() around C-visible symbols\n" . $herecurr); + } + # check for redundant bracing round if etc if ($line =~ /(^.*)\bif\b/ && $1 !~ /else\s*$/) { my ($level, $endln, @chunks) = -- cgit v1.2.3 From 8054576dca7e76dd1f58c525af3309cfc9c74454 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:26 -0800 Subject: checkpatch: loosen spacing on typedef function checks Loosen spacing checks to correctly detect this valid use of a typedef: typedef struct rcu_data *(*get_data_func)(int); Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 705a0439b39..d80b55a6f89 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1618,7 +1618,7 @@ sub process { # check for new typedefs, only function parameters and sparse annotations # make sense. if ($line =~ /\btypedef\s/ && - $line !~ /\btypedef\s+$Type\s+\(\s*\*?$Ident\s*\)\s*\(/ && + $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ && $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ && $line !~ /\b$typeTypedefs\b/ && $line !~ /\b__bitwise(?:__|)\b/) { -- cgit v1.2.3 From 8b1b33786b06a222cf3430b1bf942a3681532104 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:27 -0800 Subject: checkpatch: fix continuation detection when handling spacing on operators We are miscategorising a continuation fragment following an operator which may lead to us thinking that there is a space after it when there is not. Fix this up. Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d80b55a6f89..67b0c9faa32 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1793,7 +1793,7 @@ sub process { $c = 'C' if ($elements[$n + 2] =~ /^$;/); $c = 'B' if ($elements[$n + 2] =~ /^(\)|\]|;)/); $c = 'O' if ($elements[$n + 2] eq ''); - $c = 'E' if ($elements[$n + 2] =~ /\s*\\$/); + $c = 'E' if ($elements[$n + 2] =~ /^\s*\\$/); } else { $c = 'E'; } -- cgit v1.2.3 From 4635f4fbaf51555509c747eed02a7e7a580ae1e1 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:27 -0800 Subject: checkpatch: track #ifdef/#else/#endif when tracking blocks When picking up a complete statement or block for analysis we cannot simply track open/close/etc parenthesis we must take into account preprocessor section boundaries. Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 67b0c9faa32..906624c0e9e 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -405,6 +405,7 @@ sub ctx_statement_block { my $type = ''; my $level = 0; + my @stack = ([$type, $level]); my $p; my $c; my $len = 0; @@ -436,6 +437,16 @@ sub ctx_statement_block { $remainder = substr($blk, $off); #warn "CSB: c<$c> type<$type> level<$level> remainder<$remainder> coff_set<$coff_set>\n"; + + # Handle nested #if/#else. + if ($remainder =~ /^#\s*(?:ifndef|ifdef|if)\s/) { + push(@stack, [ $type, $level ]); + } elsif ($remainder =~ /^#\s*(?:else|elif)\b/) { + ($type, $level) = @{$stack[$#stack - 1]}; + } elsif ($remainder =~ /^#\s*endif\b/) { + ($type, $level) = @{pop(@stack)}; + } + # Statement ends at the ';' or a close '}' at the # outermost level. if ($level == 0 && $c eq ';') { @@ -582,11 +593,22 @@ sub ctx_block_get { my @res = (); my $level = 0; + my @stack = ($level); for ($line = $start; $remain > 0; $line++) { next if ($rawlines[$line] =~ /^-/); $remain--; $blk .= $rawlines[$line]; + + # Handle nested #if/#else. + if ($rawlines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) { + push(@stack, $level); + } elsif ($rawlines[$line] =~ /^.\s*#\s*(?:else|elif)\b/) { + $level = $stack[$#stack - 1]; + } elsif ($rawlines[$line] =~ /^.\s*#\s*endif\b/) { + $level = pop(@stack); + } + foreach my $c (split(//, $rawlines[$line])) { ##print "C<$c>L<$level><$open$close>O<$off>\n"; if ($off > 0) { -- cgit v1.2.3 From 2d1bafd799ee0442979e6ce4145d58b69d3cade2 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:28 -0800 Subject: checkpatch: do not report nr_static as a static declaration Ensure we do not report identifiers containing the word static as static declarations. For example this should not be reported as an unecessary assignement of 0: long nr_static = 0; Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 906624c0e9e..a521d493b0c 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1632,7 +1632,7 @@ sub process { $herecurr); } # check for static initialisers. - if ($line =~ /\s*static\s.*=\s*(0|NULL|false)\s*;/) { + if ($line =~ /\bstatic\s.*=\s*(0|NULL|false)\s*;/) { ERROR("do not initialise statics to 0 or NULL\n" . $herecurr); } -- cgit v1.2.3 From b53c8e104ed071c47ada2adce194625ab03d9f3d Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:29 -0800 Subject: checkpatch: ensure we actually detect if assignments split across lines When checking for assignments within if conditionals we check the whole of the condition, but the match is performed using a line constrained regular expression. This means we can miss split conditionals or those on the second line. Allow the check to span lines. Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index a521d493b0c..c39ce0b663b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2048,7 +2048,7 @@ sub process { $line =~ /\b(?:if|while|for)\s*\(/ && $line !~ /^.\s*#/) { my ($s, $c) = ($stat, $cond); - if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/) { + if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) { ERROR("do not use assignment in if condition\n" . $herecurr); } -- cgit v1.2.3 From 2b6db5cb65cb1276a7aa363a6e7335b0a8a68393 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:29 -0800 Subject: checkpatch: struct file_operations should normally be const In the general use case struct file_operations should be a const object. Check for and warn where it is not. As suggested by Steven and Ingo. Acked-by: Steven Rostedt Cc: Ingo Molnar Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c39ce0b663b..94371f69122 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2509,6 +2509,11 @@ sub process { if ($line =~ /^.\s*__initcall\s*\(/) { WARN("please use device_initcall() instead of __initcall()\n" . $herecurr); } +# check for struct file_operations, ensure they are const. + if ($line =~ /\bstruct\s+file_operations\b/ && + $line !~ /\bconst\b/) { + WARN("struct file_operations should normally be const\n" . $herecurr); + } # use of NR_CPUS is usually wrong # ignore definitions of NR_CPUS and usage to define arrays as likely right -- cgit v1.2.3 From 21caa13c02d67f755fad50370996c489c34a9b15 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:30 -0800 Subject: checkpatch: fix the perlcritic errors Clean up checkpatch using perlcritic. Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 94371f69122..bd6ac90d194 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -69,7 +69,9 @@ my $dbg_possible = 0; my $dbg_type = 0; my $dbg_attr = 0; for my $key (keys %debug) { - eval "\${dbg_$key} = '$debug{$key}';" + ## no critic + eval "\${dbg_$key} = '$debug{$key}';"; + die "$@" if ($@); } if ($terse) { @@ -206,9 +208,9 @@ my @dep_includes = (); my @dep_functions = (); my $removal = "Documentation/feature-removal-schedule.txt"; if ($tree && -f "$root/$removal") { - open(REMOVE, "<$root/$removal") || + open(my $REMOVE, '<', "$root/$removal") || die "$P: $removal: open failed - $!\n"; - while () { + while (<$REMOVE>) { if (/^Check:\s+(.*\S)/) { for my $entry (split(/[, ]+/, $1)) { if ($entry =~ m@include/(.*)@) { @@ -220,17 +222,21 @@ if ($tree && -f "$root/$removal") { } } } + close($REMOVE); } my @rawlines = (); my @lines = (); my $vname; for my $filename (@ARGV) { + my $FILE; if ($file) { - open(FILE, "diff -u /dev/null $filename|") || + open($FILE, '-|', "diff -u /dev/null $filename") || die "$P: $filename: diff failed - $!\n"; + } elsif ($filename eq '-') { + open($FILE, '<&STDIN'); } else { - open(FILE, "<$filename") || + open($FILE, '<', "$filename") || die "$P: $filename: open failed - $!\n"; } if ($filename eq '-') { @@ -238,11 +244,11 @@ for my $filename (@ARGV) { } else { $vname = $filename; } - while () { + while (<$FILE>) { chomp; push(@rawlines, $_); } - close(FILE); + close($FILE); if (!process($filename)) { $exit = 1; } -- cgit v1.2.3 From 57b9c6d9c5074a06fda770a7f009d655593e0e29 Mon Sep 17 00:00:00 2001 From: Andy Whitcroft Date: Tue, 6 Jan 2009 14:41:30 -0800 Subject: checkpatch: version: 0.26 Signed-off-by: Andy Whitcroft Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index bd6ac90d194..7bed4ed2c51 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -10,7 +10,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.25'; +my $V = '0.26'; use Getopt::Long qw(:config no_auto_abbrev); -- cgit v1.2.3