aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndy Whitcroft <apw@shadowen.org>2007-07-19 04:48:34 -0400
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2007-07-19 13:04:47 -0400
commitf0a594c1c74fedbd838402e7372030311be8cc6e (patch)
tree5ad2537167712b7bb281c30ca89ea1a076ee1bf6
parent4b8a8b812edd7067fffcc4a85b8aa3adae081535 (diff)
update checkpatch.pl to version 0.08
This version brings a number of new checks, and a number of bug fixes. Of note: - warnings for multiple assignments per line - warnings for multiple declarations per line - checks for single statement blocks with braces This patch includes an update for feature-removal-schedule.txt to better target checks. Andy Whitcroft (12): Version: 0.08 only apply printk checks where there is a string literal allow suppression of errors for when no patch is found warn about multiple assignments warn on declaration of multiple variables check for kfree() with needless null check check for single statement braced blocks check for aggregate initialisation on the next line handle the => operator check for spaces between function name and open parenthesis move to explicit Check: entries in feature-removal-schedule.txt handle pointer attributes Signed-off-by: Andy Whitcroft <apw@shadowen.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--Documentation/feature-removal-schedule.txt3
-rwxr-xr-xscripts/checkpatch.pl175
2 files changed, 145 insertions, 33 deletions
diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index cff63befeb9a..a9941544ed8e 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -51,6 +51,7 @@ Who: David Miller <davem@davemloft.net>
51What: Video4Linux API 1 ioctls and video_decoder.h from Video devices. 51What: Video4Linux API 1 ioctls and video_decoder.h from Video devices.
52When: December 2006 52When: December 2006
53Files: include/linux/video_decoder.h 53Files: include/linux/video_decoder.h
54Check: include/linux/video_decoder.h
54Why: V4L1 AP1 was replaced by V4L2 API. during migration from 2.4 to 2.6 55Why: V4L1 AP1 was replaced by V4L2 API. during migration from 2.4 to 2.6
55 series. The old API have lots of drawbacks and don't provide enough 56 series. The old API have lots of drawbacks and don't provide enough
56 means to work with all video and audio standards. The newer API is 57 means to work with all video and audio standards. The newer API is
@@ -84,7 +85,7 @@ Who: Dominik Brodowski <linux@brodo.de>
84What: remove EXPORT_SYMBOL(kernel_thread) 85What: remove EXPORT_SYMBOL(kernel_thread)
85When: August 2006 86When: August 2006
86Files: arch/*/kernel/*_ksyms.c 87Files: arch/*/kernel/*_ksyms.c
87Funcs: kernel_thread 88Check: kernel_thread
88Why: kernel_thread is a low-level implementation detail. Drivers should 89Why: kernel_thread is a low-level implementation detail. Drivers should
89 use the <linux/kthread.h> API instead which shields them from 90 use the <linux/kthread.h> API instead which shields them from
90 implementation details and provides a higherlevel interface that 91 implementation details and provides a higherlevel interface that
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 25e20a27fc59..73751ab6ec0c 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,7 +9,7 @@ use strict;
9my $P = $0; 9my $P = $0;
10$P =~ s@.*/@@g; 10$P =~ s@.*/@@g;
11 11
12my $V = '0.07'; 12my $V = '0.08';
13 13
14use Getopt::Long qw(:config no_auto_abbrev); 14use Getopt::Long qw(:config no_auto_abbrev);
15 15
@@ -47,16 +47,14 @@ my $removal = 'Documentation/feature-removal-schedule.txt';
47if ($tree && -f $removal) { 47if ($tree && -f $removal) {
48 open(REMOVE, "<$removal") || die "$P: $removal: open failed - $!\n"; 48 open(REMOVE, "<$removal") || die "$P: $removal: open failed - $!\n";
49 while (<REMOVE>) { 49 while (<REMOVE>) {
50 if (/^Files:\s+(.*\S)/) { 50 if (/^Check:\s+(.*\S)/) {
51 for my $file (split(/[, ]+/, $1)) { 51 for my $entry (split(/[, ]+/, $1)) {
52 if ($file =~ m@include/(.*)@) { 52 if ($entry =~ m@include/(.*)@) {
53 push(@dep_includes, $1); 53 push(@dep_includes, $1);
54 }
55 }
56 54
57 } elsif (/^Funcs:\s+(.*\S)/) { 55 } elsif ($entry !~ m@/@) {
58 for my $func (split(/[, ]+/, $1)) { 56 push(@dep_functions, $entry);
59 push(@dep_functions, $func); 57 }
60 } 58 }
61 } 59 }
62 } 60 }
@@ -153,7 +151,7 @@ sub sanitise_line {
153} 151}
154 152
155sub ctx_block_get { 153sub ctx_block_get {
156 my ($linenr, $remain, $outer, $open, $close) = @_; 154 my ($linenr, $remain, $outer, $open, $close, $off) = @_;
157 my $line; 155 my $line;
158 my $start = $linenr - 1; 156 my $start = $linenr - 1;
159 my $blk = ''; 157 my $blk = '';
@@ -161,38 +159,58 @@ sub ctx_block_get {
161 my @c; 159 my @c;
162 my @res = (); 160 my @res = ();
163 161
162 my $level = 0;
164 for ($line = $start; $remain > 0; $line++) { 163 for ($line = $start; $remain > 0; $line++) {
165 next if ($rawlines[$line] =~ /^-/); 164 next if ($rawlines[$line] =~ /^-/);
166 $remain--; 165 $remain--;
167 166
168 $blk .= $rawlines[$line]; 167 $blk .= $rawlines[$line];
168 foreach my $c (split(//, $rawlines[$line])) {
169 ##print "C<$c>L<$level><$open$close>O<$off>\n";
170 if ($off > 0) {
171 $off--;
172 next;
173 }
169 174
170 @o = ($blk =~ /$open/g); 175 if ($c eq $close && $level > 0) {
171 @c = ($blk =~ /$close/g); 176 $level--;
177 last if ($level == 0);
178 } elsif ($c eq $open) {
179 $level++;
180 }
181 }
172 182
173 if (!$outer || (scalar(@o) - scalar(@c)) == 1) { 183 if (!$outer || $level <= 1) {
174 push(@res, $rawlines[$line]); 184 push(@res, $rawlines[$line]);
175 } 185 }
176 186
177 last if (scalar(@o) == scalar(@c)); 187 last if ($level == 0);
178 } 188 }
179 189
180 return @res; 190 return ($level, @res);
181} 191}
182sub ctx_block_outer { 192sub ctx_block_outer {
183 my ($linenr, $remain) = @_; 193 my ($linenr, $remain) = @_;
184 194
185 return ctx_block_get($linenr, $remain, 1, '\{', '\}'); 195 my ($level, @r) = ctx_block_get($linenr, $remain, 1, '{', '}', 0);
196 return @r;
186} 197}
187sub ctx_block { 198sub ctx_block {
188 my ($linenr, $remain) = @_; 199 my ($linenr, $remain) = @_;
189 200
190 return ctx_block_get($linenr, $remain, 0, '\{', '\}'); 201 my ($level, @r) = ctx_block_get($linenr, $remain, 0, '{', '}', 0);
202 return @r;
191} 203}
192sub ctx_statement { 204sub ctx_statement {
205 my ($linenr, $remain, $off) = @_;
206
207 my ($level, @r) = ctx_block_get($linenr, $remain, 0, '(', ')', $off);
208 return @r;
209}
210sub ctx_block_level {
193 my ($linenr, $remain) = @_; 211 my ($linenr, $remain) = @_;
194 212
195 return ctx_block_get($linenr, $remain, 0, '\(', '\)'); 213 return ctx_block_get($linenr, $remain, 0, '{', '}', 0);
196} 214}
197 215
198sub ctx_locate_comment { 216sub ctx_locate_comment {
@@ -246,16 +264,23 @@ sub cat_vet {
246 return $vet; 264 return $vet;
247} 265}
248 266
267my @report = ();
268sub report {
269 push(@report, $_[0]);
270}
271sub report_dump {
272 @report;
273}
249sub ERROR { 274sub ERROR {
250 print "ERROR: $_[0]\n"; 275 report("ERROR: $_[0]\n");
251 our $clean = 0; 276 our $clean = 0;
252} 277}
253sub WARN { 278sub WARN {
254 print "WARNING: $_[0]\n"; 279 report("WARNING: $_[0]\n");
255 our $clean = 0; 280 our $clean = 0;
256} 281}
257sub CHK { 282sub CHK {
258 print "CHECK: $_[0]\n"; 283 report("CHECK: $_[0]\n");
259 our $clean = 0; 284 our $clean = 0;
260} 285}
261 286
@@ -318,7 +343,10 @@ sub process {
318 (?:\s*\*+\s*const|\s*\*+)? 343 (?:\s*\*+\s*const|\s*\*+)?
319 }x; 344 }x;
320 my $Declare = qr{(?:$Storage\s+)?$Type}; 345 my $Declare = qr{(?:$Storage\s+)?$Type};
321 my $Attribute = qr{__read_mostly|__init|__initdata}; 346 my $Attribute = qr{const|__read_mostly|__init|__initdata|__meminit};
347
348 my $Member = qr{->$Ident|\.$Ident|\[[^]]*\]};
349 my $Lval = qr{$Ident(?:$Member)*};
322 350
323 # Pre-scan the patch looking for any __setup documentation. 351 # Pre-scan the patch looking for any __setup documentation.
324 my @setup_docs = (); 352 my @setup_docs = ();
@@ -509,7 +537,7 @@ sub process {
509# if/while/etc brace do not go on next line, unless defining a do while loop, 537# if/while/etc brace do not go on next line, unless defining a do while loop,
510# or if that brace on the next line is for something else 538# or if that brace on the next line is for something else
511 if ($line =~ /\b(?:(if|while|for|switch)\s*\(|do\b|else\b)/ && $line !~ /^.#/) { 539 if ($line =~ /\b(?:(if|while|for|switch)\s*\(|do\b|else\b)/ && $line !~ /^.#/) {
512 my @ctx = ctx_statement($linenr, $realcnt); 540 my @ctx = ctx_statement($linenr, $realcnt, 0);
513 my $ctx_ln = $linenr + $#ctx + 1; 541 my $ctx_ln = $linenr + $#ctx + 1;
514 my $ctx_cnt = $realcnt - $#ctx - 1; 542 my $ctx_cnt = $realcnt - $#ctx - 1;
515 my $ctx = join("\n", @ctx); 543 my $ctx = join("\n", @ctx);
@@ -521,7 +549,7 @@ sub process {
521 ##warn "line<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>"; 549 ##warn "line<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>";
522 550
523 if ($ctx !~ /{\s*/ && $ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { 551 if ($ctx !~ /{\s*/ && $ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
524 ERROR("That { should be on the previous line\n" . 552 ERROR("That open brace { should be on the previous line\n" .
525 "$here\n$ctx\n$lines[$ctx_ln - 1]"); 553 "$here\n$ctx\n$lines[$ctx_ln - 1]");
526 } 554 }
527 } 555 }
@@ -535,6 +563,12 @@ sub process {
535 next; 563 next;
536 } 564 }
537 565
566# check for initialisation to aggregates open brace on the next line
567 if ($prevline =~ /$Declare\s*$Ident\s*=\s*$/ &&
568 $line =~ /^.\s*{/) {
569 ERROR("That open brace { should be on the previous line\n" . $hereprev);
570 }
571
538# 572#
539# Checks which are anchored on the added line. 573# Checks which are anchored on the added line.
540# 574#
@@ -570,8 +604,13 @@ sub process {
570 } 604 }
571 } 605 }
572 606
607# check for external initialisers.
608 if ($line =~ /^.$Type\s*$Ident\s*=\s*(0|NULL);/) {
609 ERROR("do not initialise externals to 0 or NULL\n" .
610 $herecurr);
611 }
573# check for static initialisers. 612# check for static initialisers.
574 if ($line=~/\s*static\s.*=\s+(0|NULL);/) { 613 if ($line =~ /\s*static\s.*=\s*(0|NULL);/) {
575 ERROR("do not initialise statics to 0 or NULL\n" . 614 ERROR("do not initialise statics to 0 or NULL\n" .
576 $herecurr); 615 $herecurr);
577 } 616 }
@@ -593,11 +632,11 @@ sub process {
593 ERROR("\"(foo $1 )\" should be \"(foo $1)\"\n" . 632 ERROR("\"(foo $1 )\" should be \"(foo $1)\"\n" .
594 $herecurr); 633 $herecurr);
595 634
596 } elsif ($line =~ m{$NonptrType(\*+)(?:\s+const)?\s+[A-Za-z\d_]+}) { 635 } elsif ($line =~ m{$NonptrType(\*+)(?:\s+$Attribute)?\s+[A-Za-z\d_]+}) {
597 ERROR("\"foo$1 bar\" should be \"foo $1bar\"\n" . 636 ERROR("\"foo$1 bar\" should be \"foo $1bar\"\n" .
598 $herecurr); 637 $herecurr);
599 638
600 } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+const)\s+[A-Za-z\d_]+}) { 639 } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+$Attribute)\s+[A-Za-z\d_]+}) {
601 ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" . 640 ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" .
602 $herecurr); 641 $herecurr);
603 } 642 }
@@ -614,7 +653,7 @@ sub process {
614# to try and find and validate the current printk. In summary the current 653# to try and find and validate the current printk. In summary the current
615# printk includes all preceeding printk's which have no newline on the end. 654# printk includes all preceeding printk's which have no newline on the end.
616# we assume the first bad printk is the one to report. 655# we assume the first bad printk is the one to report.
617 if ($line =~ /\bprintk\((?!KERN_)/) { 656 if ($line =~ /\bprintk\((?!KERN_)\s*"/) {
618 my $ok = 0; 657 my $ok = 0;
619 for (my $ln = $linenr - 1; $ln >= $first_line; $ln--) { 658 for (my $ln = $linenr - 1; $ln >= $first_line; $ln--) {
620 #print "CHECK<$lines[$ln - 1]\n"; 659 #print "CHECK<$lines[$ln - 1]\n";
@@ -639,6 +678,12 @@ sub process {
639 ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr); 678 ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr);
640 } 679 }
641 680
681# check for spaces between functions and their parentheses.
682 if ($line =~ /($Ident)\s+\(/ &&
683 $1 !~ /^(?:if|for|while|switch|return|volatile)$/ &&
684 $line !~ /$Type\s+\(/ && $line !~ /^.\#\s*define\b/) {
685 ERROR("no space between function name and open parenthesis '('\n" . $herecurr);
686 }
642# Check operator spacing. 687# Check operator spacing.
643 # Note we expand the line with the leading + as the real 688 # Note we expand the line with the leading + as the real
644 # line will be displayed with the leading + and the tabs 689 # line will be displayed with the leading + and the tabs
@@ -647,7 +692,7 @@ sub process {
647 $opline = expand_tabs($opline); 692 $opline = expand_tabs($opline);
648 $opline =~ s/^./ /; 693 $opline =~ s/^./ /;
649 if (!($line=~/\#\s*include/)) { 694 if (!($line=~/\#\s*include/)) {
650 my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline); 695 my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|=>|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline);
651 my $off = 0; 696 my $off = 0;
652 for (my $n = 0; $n < $#elements; $n += 2) { 697 for (my $n = 0; $n < $#elements; $n += 2) {
653 $off += length($elements[$n]); 698 $off += length($elements[$n]);
@@ -773,6 +818,18 @@ sub process {
773 } 818 }
774 } 819 }
775 820
821# check for multiple assignments
822 if ($line =~ /^.\s*$Lval\s*=\s*$Lval\s*=(?!=)/) {
823 WARN("multiple assignments should be avoided\n" . $herecurr);
824 }
825
826# check for multiple declarations, allowing for a function declaration
827# continuation.
828 if ($line =~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Ident.*/ &&
829 $line !~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Type\s*$Ident.*/) {
830 WARN("declaring multiple variables together should be avoided\n" . $herecurr);
831 }
832
776#need space before brace following if, while, etc 833#need space before brace following if, while, etc
777 if ($line =~ /\(.*\){/ || $line =~ /do{/) { 834 if ($line =~ /\(.*\){/ || $line =~ /do{/) {
778 ERROR("need a space before the open brace '{'\n" . $herecurr); 835 ERROR("need a space before the open brace '{'\n" . $herecurr);
@@ -847,13 +904,18 @@ sub process {
847 # or the one below. 904 # or the one below.
848 my $ln = $linenr; 905 my $ln = $linenr;
849 my $cnt = $realcnt; 906 my $cnt = $realcnt;
907 my $off = 0;
850 908
851 # If the macro starts on the define line start there. 909 # If the macro starts on the define line start
852 if ($prevline !~ m{^.#\s*define\s*$Ident(?:\([^\)]*\))?\s*\\\s*$}) { 910 # grabbing the statement after the identifier
911 $prevline =~ m{^(.#\s*define\s*$Ident(?:\([^\)]*\))?\s*)(.*)\\\s*$};
912 ##print "1<$1> 2<$2>\n";
913 if ($2 ne '') {
914 $off = length($1);
853 $ln--; 915 $ln--;
854 $cnt++; 916 $cnt++;
855 } 917 }
856 my @ctx = ctx_statement($ln, $cnt); 918 my @ctx = ctx_statement($ln, $cnt, $off);
857 my $ctx_ln = $ln + $#ctx + 1; 919 my $ctx_ln = $ln + $#ctx + 1;
858 my $ctx = join("\n", @ctx); 920 my $ctx = join("\n", @ctx);
859 921
@@ -873,6 +935,44 @@ sub process {
873 } 935 }
874 } 936 }
875 937
938# check for redundant bracing round if etc
939 if ($line =~ /\b(if|while|for|else)\b/) {
940 # Locate the end of the opening statement.
941 my @control = ctx_statement($linenr, $realcnt, 0);
942 my $nr = $linenr + (scalar(@control) - 1);
943 my $cnt = $realcnt - (scalar(@control) - 1);
944
945 my $off = $realcnt - $cnt;
946 #print "$off: line<$line>end<" . $lines[$nr - 1] . ">\n";
947
948 # If this is is a braced statement group check it
949 if ($lines[$nr - 1] =~ /{\s*$/) {
950 my ($lvl, @block) = ctx_block_level($nr, $cnt);
951
952 my $stmt = join(' ', @block);
953 $stmt =~ s/^[^{]*{//;
954 $stmt =~ s/}[^}]*$//;
955
956 #print "block<" . join(' ', @block) . "><" . scalar(@block) . ">\n";
957 #print "stmt<$stmt>\n\n";
958
959 # Count the ;'s if there is fewer than two
960 # then there can only be one statement,
961 # if there is a brace inside we cannot
962 # trivially detect if its one statement.
963 # Also nested if's often require braces to
964 # disambiguate the else binding so shhh there.
965 my @semi = ($stmt =~ /;/g);
966 ##print "semi<" . scalar(@semi) . ">\n";
967 if ($lvl == 0 && scalar(@semi) < 2 &&
968 $stmt !~ /{/ && $stmt !~ /\bif\b/) {
969 my $herectx = "$here\n" . join("\n", @control, @block[1 .. $#block]) . "\n";
970 shift(@block);
971 ERROR("braces {} are not necessary for single statement blocks\n" . $herectx);
972 }
973 }
974 }
975
876# don't include deprecated include files (uses RAW line) 976# don't include deprecated include files (uses RAW line)
877 for my $inc (@dep_includes) { 977 for my $inc (@dep_includes) {
878 if ($rawline =~ m@\#\s*include\s*\<$inc>@) { 978 if ($rawline =~ m@\#\s*include\s*\<$inc>@) {
@@ -898,6 +998,14 @@ sub process {
898 $herecurr); 998 $herecurr);
899 } 999 }
900 1000
1001# check for needless kfree() checks
1002 if ($prevline =~ /\bif\s*\(([^\)]*)\)/) {
1003 my $expr = $1;
1004 if ($line =~ /\bkfree\(\Q$expr\E\);/) {
1005 WARN("kfree(NULL) is safe this check is probabally not required\n" . $hereprev);
1006 }
1007 }
1008
901# warn about #ifdefs in C files 1009# warn about #ifdefs in C files
902# if ($line =~ /^.#\s*if(|n)def/ && ($realfile =~ /\.c$/)) { 1010# if ($line =~ /^.#\s*if(|n)def/ && ($realfile =~ /\.c$/)) {
903# print "#ifdef in C files should be avoided\n"; 1011# print "#ifdef in C files should be avoided\n";
@@ -952,6 +1060,9 @@ sub process {
952 ERROR("Missing Signed-off-by: line(s)\n"); 1060 ERROR("Missing Signed-off-by: line(s)\n");
953 } 1061 }
954 1062
1063 if ($clean == 0 && ($chk_patch || $is_patch)) {
1064 print report_dump();
1065 }
955 if ($clean == 1 && $quiet == 0) { 1066 if ($clean == 1 && $quiet == 0) {
956 print "Your patch has no obvious style problems and is ready for submission.\n" 1067 print "Your patch has no obvious style problems and is ready for submission.\n"
957 } 1068 }