aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndy Whitcroft <apw@shadowen.org>2007-06-23 20:16:34 -0400
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2007-06-24 11:59:11 -0400
commit653d4876b730fedca8473481863cf700245e3582 (patch)
tree360946594686e00800056eafbdce369c2ed565a5
parent92c4ca5c3a5e180e9762438db235f41d192cb955 (diff)
update checkpatch.pl to version 0.05
This version brings a some new tests, and a host of changes to fix false positives, of particular note: - detect 'var ++;' and 'var --;' as a bad combination - multistatement #defines are now checked based on statement count - multistatement #defines with initialisation correctly reported - checks the location of the inline keywords - EXPORT_SYMBOL for variables are now understood - typedefs are loosened to handle sparse etc This version of checkpatch.pl can be found at the following URL: http://www.shadowen.org/~apw/public/checkpatch/checkpatch.pl-0.05 Full Changelog: Andy Whitcroft (18): Version: 0.05 macro definition checks should be for a single statement avoid assignements only in if conditionals declarations of function pointers need no space multiline macros which are purely initialisation cannot be wrapped EXPORT_SYMBOL can also directly follow a variable definition check on the location of the inline keyword EXPORT_SYMBOL needs to allow for attributes ensure we do not find C99 // in strings handle malformed #include lines accept the {0,} form typedefs are sensible for defining function pointer parameters ensure { handling correctly handles nested switch() statements trailing whitespace checks are not anchored typedefs for sparse bitwise annotations make sense update the type matcher to include sparse annotations clean up indent and spacing 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>
-rwxr-xr-xscripts/checkpatch.pl183
1 files changed, 129 insertions, 54 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index aea90d30d229..56c364c1df81 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.04'; 12my $V = '0.05';
13 13
14use Getopt::Long qw(:config no_auto_abbrev); 14use Getopt::Long qw(:config no_auto_abbrev);
15 15
@@ -17,11 +17,13 @@ my $quiet = 0;
17my $tree = 1; 17my $tree = 1;
18my $chk_signoff = 1; 18my $chk_signoff = 1;
19my $chk_patch = 1; 19my $chk_patch = 1;
20my $tst_type = 0;
20GetOptions( 21GetOptions(
21 'q|quiet' => \$quiet, 22 'q|quiet' => \$quiet,
22 'tree!' => \$tree, 23 'tree!' => \$tree,
23 'signoff!' => \$chk_signoff, 24 'signoff!' => \$chk_signoff,
24 'patch!' => \$chk_patch, 25 'patch!' => \$chk_patch,
26 'test-type!' => \$tst_type,
25) or exit; 27) or exit;
26 28
27my $exit = 0; 29my $exit = 0;
@@ -151,7 +153,7 @@ sub sanitise_line {
151} 153}
152 154
153sub ctx_block_get { 155sub ctx_block_get {
154 my ($linenr, $remain, $outer) = @_; 156 my ($linenr, $remain, $outer, $open, $close) = @_;
155 my $line; 157 my $line;
156 my $start = $linenr - 1; 158 my $start = $linenr - 1;
157 my $blk = ''; 159 my $blk = '';
@@ -165,8 +167,8 @@ sub ctx_block_get {
165 167
166 $blk .= $rawlines[$line]; 168 $blk .= $rawlines[$line];
167 169
168 @o = ($blk =~ /\{/g); 170 @o = ($blk =~ /$open/g);
169 @c = ($blk =~ /\}/g); 171 @c = ($blk =~ /$close/g);
170 172
171 if (!$outer || (scalar(@o) - scalar(@c)) == 1) { 173 if (!$outer || (scalar(@o) - scalar(@c)) == 1) {
172 push(@res, $rawlines[$line]); 174 push(@res, $rawlines[$line]);
@@ -180,12 +182,17 @@ sub ctx_block_get {
180sub ctx_block_outer { 182sub ctx_block_outer {
181 my ($linenr, $remain) = @_; 183 my ($linenr, $remain) = @_;
182 184
183 return ctx_block_get($linenr, $remain, 1); 185 return ctx_block_get($linenr, $remain, 1, '\{', '\}');
184} 186}
185sub ctx_block { 187sub ctx_block {
186 my ($linenr, $remain) = @_; 188 my ($linenr, $remain) = @_;
187 189
188 return ctx_block_get($linenr, $remain, 0); 190 return ctx_block_get($linenr, $remain, 0, '\{', '\}');
191}
192sub ctx_statement {
193 my ($linenr, $remain) = @_;
194
195 return ctx_block_get($linenr, $remain, 0, '\(', '\)');
189} 196}
190 197
191sub ctx_locate_comment { 198sub ctx_locate_comment {
@@ -264,9 +271,30 @@ sub process {
264 my $in_comment = 0; 271 my $in_comment = 0;
265 my $first_line = 0; 272 my $first_line = 0;
266 273
274 my $ident = '[A-Za-z\d_]+';
275 my $storage = '(?:extern|static)';
276 my $sparse = '(?:__user|__kernel|__force|__iomem)';
277 my $type = '(?:unsigned\s+)?' .
278 '(?:void|char|short|int|long|unsigned|float|double|' .
279 'long\s+long|' .
280 "struct\\s+${ident}|" .
281 "union\\s+${ident}|" .
282 "${ident}_t)" .
283 "(?:\\s+$sparse)*" .
284 '(?:\s*\*+)?';
285 my $attribute = '(?:__read_mostly|__init|__initdata)';
286
287 my $Ident = $ident;
288 my $Type = $type;
289 my $Storage = $storage;
290 my $Declare = "(?:$storage\\s+)?$type";
291 my $Attribute = $attribute;
292
267 foreach my $line (@lines) { 293 foreach my $line (@lines) {
268 $linenr++; 294 $linenr++;
269 295
296 my $rawline = $line;
297
270#extract the filename as it passes 298#extract the filename as it passes
271 if ($line=~/^\+\+\+\s+(\S+)/) { 299 if ($line=~/^\+\+\+\s+(\S+)/) {
272 $realfile=$1; 300 $realfile=$1;
@@ -361,7 +389,7 @@ sub process {
361 next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); 389 next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
362 390
363#trailing whitespace 391#trailing whitespace
364 if ($line=~/\+.*\S\s+$/) { 392 if ($line=~/^\+.*\S\s+$/) {
365 my $herevet = "$here\n" . cat_vet($line) . "\n\n"; 393 my $herevet = "$here\n" . cat_vet($line) . "\n\n";
366 print "trailing whitespace\n"; 394 print "trailing whitespace\n";
367 print "$herevet"; 395 print "$herevet";
@@ -392,17 +420,20 @@ sub process {
392 # 420 #
393 next if ($in_comment); 421 next if ($in_comment);
394 422
395 # Remove comments from the line before processing. 423# Remove comments from the line before processing.
396 $line =~ s@/\*.*\*/@@g; 424 $line =~ s@/\*.*\*/@@g;
397 $line =~ s@/\*.*@@; 425 $line =~ s@/\*.*@@;
398 $line =~ s@.*\*/@@; 426 $line =~ s@.*\*/@@;
399 427
400 # 428# Standardise the strings and chars within the input to simplify matching.
401 # Checks which may be anchored in the context. 429 $line = sanitise_line($line);
402 # 430
431#
432# Checks which may be anchored in the context.
433#
403 434
404 # Check for switch () and associated case and default 435# Check for switch () and associated case and default
405 # statements should be at the same indent. 436# statements should be at the same indent.
406 if ($line=~/\bswitch\s*\(.*\)/) { 437 if ($line=~/\bswitch\s*\(.*\)/) {
407 my $err = ''; 438 my $err = '';
408 my $sep = ''; 439 my $sep = '';
@@ -428,9 +459,30 @@ sub process {
428#ignore lines not being added 459#ignore lines not being added
429 if ($line=~/^[^\+]/) {next;} 460 if ($line=~/^[^\+]/) {next;}
430 461
431 # 462# TEST: allow direct testing of the type matcher.
432 # Checks which are anchored on the added line. 463 if ($tst_type && $line =~ /^.$Declare$/) {
433 # 464 print "TEST: is type $Declare\n";
465 print "$herecurr";
466 $clean = 0;
467 next;
468 }
469
470#
471# Checks which are anchored on the added line.
472#
473
474# check for malformed paths in #include statements (uses RAW line)
475 if ($rawline =~ m{^.#\s*include\s+[<"](.*)[">]}) {
476 my $path = $1;
477 if ($path =~ m{//}) {
478 print "malformed #include filename\n";
479 print "$herecurr";
480 $clean = 0;
481 }
482 # Sanitise this special form of string.
483 $path = 'X' x length($path);
484 $line =~ s{\<.*\>}{<$path>};
485 }
434 486
435# no C99 // comments 487# no C99 // comments
436 if ($line =~ m{//}) { 488 if ($line =~ m{//}) {
@@ -441,47 +493,44 @@ sub process {
441 # Remove C99 comments. 493 # Remove C99 comments.
442 $line =~ s@//.*@@; 494 $line =~ s@//.*@@;
443 495
444 # Standardise the strings and chars within the input
445 # to simplify matching.
446 $line = sanitise_line($line);
447
448#EXPORT_SYMBOL should immediately follow its function closing }. 496#EXPORT_SYMBOL should immediately follow its function closing }.
449 if (($line =~ /EXPORT_SYMBOL.*\(.*\)/) || 497 if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) ||
450 ($line =~ /EXPORT_UNUSED_SYMBOL.*\(.*\)/)) { 498 ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) {
499 my $name = $1;
451 if (($prevline !~ /^}/) && 500 if (($prevline !~ /^}/) &&
452 ($prevline !~ /^\+}/) && 501 ($prevline !~ /^\+}/) &&
453 ($prevline !~ /^ }/)) { 502 ($prevline !~ /^ }/) &&
454 print "EXPORT_SYMBOL(func); should immediately follow its function\n"; 503 ($prevline !~ /\s$name(?:\s+$Attribute)?\s*(?:;|=)/)) {
504 print "EXPORT_SYMBOL(foo); should immediately follow its function/variable\n";
455 print "$herecurr"; 505 print "$herecurr";
456 $clean = 0; 506 $clean = 0;
457 } 507 }
458 } 508 }
459 509
460 # check for static initialisers. 510# check for static initialisers.
461 if ($line=~/\s*static\s.*=\s+(0|NULL);/) { 511 if ($line=~/\s*static\s.*=\s+(0|NULL);/) {
462 print "do not initialise statics to 0 or NULL\n"; 512 print "do not initialise statics to 0 or NULL\n";
463 print "$herecurr"; 513 print "$herecurr";
464 $clean = 0; 514 $clean = 0;
465 } 515 }
466 516
467 # check for new typedefs. 517# check for new typedefs, only function parameters and sparse annotations
468 if ($line=~/\s*typedef\s/) { 518# make sense.
519 if ($line =~ /\btypedef\s/ &&
520 $line !~ /\btypedef\s+$Type\s+\(\s*$Ident\s*\)\s*\(/ &&
521 $line !~ /\b__bitwise(?:__|)\b/) {
469 print "do not add new typedefs\n"; 522 print "do not add new typedefs\n";
470 print "$herecurr"; 523 print "$herecurr";
471 $clean = 0; 524 $clean = 0;
472 } 525 }
473 526
474# * goes on variable not on type 527# * goes on variable not on type
475 my $type = '(?:char|short|int|long|unsigned|float|double|' .
476 'struct\s+[A-Za-z\d_]+|' .
477 'union\s+[A-Za-z\d_]+)';
478
479 if ($line =~ m{[A-Za-z\d_]+(\*+) [A-Za-z\d_]+}) { 528 if ($line =~ m{[A-Za-z\d_]+(\*+) [A-Za-z\d_]+}) {
480 print "\"foo$1 bar\" should be \"foo $1bar\"\n"; 529 print "\"foo$1 bar\" should be \"foo $1bar\"\n";
481 print "$herecurr"; 530 print "$herecurr";
482 $clean = 0; 531 $clean = 0;
483 } 532 }
484 if ($line =~ m{$type (\*) [A-Za-z\d_]+} || 533 if ($line =~ m{$Type (\*) [A-Za-z\d_]+} ||
485 $line =~ m{[A-Za-z\d_]+ (\*\*+) [A-Za-z\d_]+}) { 534 $line =~ m{[A-Za-z\d_]+ (\*\*+) [A-Za-z\d_]+}) {
486 print "\"foo $1 bar\" should be \"foo $1bar\"\n"; 535 print "\"foo $1 bar\" should be \"foo $1bar\"\n";
487 print "$herecurr"; 536 print "$herecurr";
@@ -530,13 +579,16 @@ sub process {
530 } 579 }
531 } 580 }
532 581
533#function brace can't be on same line, except for #defines of do while, or if closed on same line 582# function brace can't be on same line, except for #defines of do while,
583# or if closed on same line
534 if (($line=~/[A-Za-z\d_]+\**\s+\**[A-Za-z\d_]+\(.*\).* {/) and 584 if (($line=~/[A-Za-z\d_]+\**\s+\**[A-Za-z\d_]+\(.*\).* {/) and
535 !($line=~/\#define.*do\s{/) and !($line=~/}/)) { 585 !($line=~/\#define.*do\s{/) and !($line=~/}/)) {
536 print "braces following function declarations go on the next line\n"; 586 print "braces following function declarations go on the next line\n";
537 print "$herecurr"; 587 print "$herecurr";
538 $clean = 0; 588 $clean = 0;
539 } 589 }
590
591# Check operator spacing.
540 # Note we expand the line with the leading + as the real 592 # Note we expand the line with the leading + as the real
541 # line will be displayed with the leading + and the tabs 593 # line will be displayed with the leading + and the tabs
542 # will therefore also expand that way. 594 # will therefore also expand that way.
@@ -544,7 +596,6 @@ sub process {
544 $opline = expand_tabs($opline); 596 $opline = expand_tabs($opline);
545 $opline =~ s/^./ /; 597 $opline =~ s/^./ /;
546 if (!($line=~/\#\s*include/)) { 598 if (!($line=~/\#\s*include/)) {
547 # Check operator spacing.
548 my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline); 599 my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline);
549 my $off = 0; 600 my $off = 0;
550 for (my $n = 0; $n < $#elements; $n += 2) { 601 for (my $n = 0; $n < $#elements; $n += 2) {
@@ -572,8 +623,8 @@ sub process {
572 # Pick up the preceeding and succeeding characters. 623 # Pick up the preceeding and succeeding characters.
573 my $ca = substr($opline, $off - 1, 1); 624 my $ca = substr($opline, $off - 1, 1);
574 my $cc = ''; 625 my $cc = '';
575 if (length($opline) > ($off + length($elements[$n]))) { 626 if (length($opline) >= ($off + length($elements[$n + 1]))) {
576 $cc = substr($opline, $off + 1 + length($elements[$n]), 1); 627 $cc = substr($opline, $off + length($elements[$n + 1]), 1);
577 } 628 }
578 629
579 my $ctx = "${a}x${c}"; 630 my $ctx = "${a}x${c}";
@@ -598,7 +649,7 @@ sub process {
598 649
599 # , must have a space on the right. 650 # , must have a space on the right.
600 } elsif ($op eq ',') { 651 } elsif ($op eq ',') {
601 if ($ctx !~ /.xW|.xE/) { 652 if ($ctx !~ /.xW|.xE/ && $cc ne '}') {
602 print "need space after that '$op' $at\n"; 653 print "need space after that '$op' $at\n";
603 print "$hereptr"; 654 print "$hereptr";
604 $clean = 0; 655 $clean = 0;
@@ -619,11 +670,16 @@ sub process {
619 670
620 # unary ++ and unary -- are allowed no space on one side. 671 # unary ++ and unary -- are allowed no space on one side.
621 } elsif ($op eq '++' or $op eq '--') { 672 } elsif ($op eq '++' or $op eq '--') {
622 if ($ctx !~ /[WOB]x[^W]|[^W]x[WOB]/) { 673 if ($ctx !~ /[WOB]x[^W]/ && $ctx !~ /[^W]x[WOB]/) {
623 print "need space one side of that '$op' $at\n"; 674 print "need space one side of that '$op' $at\n";
624 print "$hereptr"; 675 print "$hereptr";
625 $clean = 0; 676 $clean = 0;
626 } 677 }
678 if ($ctx =~ /Wx./ && $cc eq ';') {
679 print "no space before that '$op' $at\n";
680 print "$hereptr";
681 $clean = 0;
682 }
627 683
628 # & is both unary and binary 684 # & is both unary and binary
629 # unary: 685 # unary:
@@ -656,7 +712,7 @@ sub process {
656 print "$hereptr"; 712 print "$hereptr";
657 $clean = 0; 713 $clean = 0;
658 } 714 }
659 } elsif ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB/) { 715 } elsif ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB|BxB/) {
660 print "need space before that '$op' $at\n"; 716 print "need space before that '$op' $at\n";
661 print "$hereptr"; 717 print "$hereptr";
662 $clean = 0; 718 $clean = 0;
@@ -705,9 +761,9 @@ sub process {
705 } 761 }
706 762
707# Check for illegal assignment in if conditional. 763# Check for illegal assignment in if conditional.
708 if ($line=~/\b(if|while)\s*\(.*[^<>!=]=[^=].*\)/) { 764 if ($line=~/\bif\s*\(.*[^<>!=]=[^=].*\)/) {
709 #next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/); 765 #next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/);
710 print "do not use assignment in condition\n"; 766 print "do not use assignment in if condition\n";
711 print "$herecurr"; 767 print "$herecurr";
712 $clean = 0; 768 $clean = 0;
713 } 769 }
@@ -735,8 +791,8 @@ sub process {
735 $clean = 0; 791 $clean = 0;
736 } 792 }
737 793
738#warn if <asm/foo.h> is #included and <linux/foo.h> is available. 794#warn if <asm/foo.h> is #included and <linux/foo.h> is available (uses RAW line)
739 if ($tree && $line =~ qr|\s*\#\s*include\s*\<asm\/(.*)\.h\>|) { 795 if ($tree && $rawline =~ m{^.\#\s*include\s*\<asm\/(.*)\.h\>}) {
740 my $checkfile = "include/linux/$1.h"; 796 my $checkfile = "include/linux/$1.h";
741 if (-f $checkfile) { 797 if (-f $checkfile) {
742 print "Use #include <linux/$1.h> instead of <asm/$1.h>\n"; 798 print "Use #include <linux/$1.h> instead of <asm/$1.h>\n";
@@ -745,7 +801,8 @@ sub process {
745 } 801 }
746 } 802 }
747 803
748#if/while/etc brace do not go on next line, unless #defining a do while loop, or if that brace on the next line is for something else 804# if/while/etc brace do not go on next line, unless defining a do while loop,
805# or if that brace on the next line is for something else
749 if ($prevline=~/\b(if|while|for|switch)\s*\(/) { 806 if ($prevline=~/\b(if|while|for|switch)\s*\(/) {
750 my @opened = $prevline=~/\(/g; 807 my @opened = $prevline=~/\(/g;
751 my @closed = $prevline=~/\)/g; 808 my @closed = $prevline=~/\)/g;
@@ -767,25 +824,36 @@ sub process {
767 } 824 }
768 825
769 if (($prevline=~/\b(if|while|for|switch)\s*\(.*\)\s*$/) and ($next_line=~/{/) and 826 if (($prevline=~/\b(if|while|for|switch)\s*\(.*\)\s*$/) and ($next_line=~/{/) and
770 !($next_line=~/\b(if|while|for)/) and !($next_line=~/\#define.*do.*while/)) { 827 !($next_line=~/\b(if|while|for|switch)/) and !($next_line=~/\#define.*do.*while/)) {
771 print "That { should be on the previous line\n"; 828 print "That { should be on the previous line\n";
772 print "$here\n$display_segment\n$next_line\n\n"; 829 print "$here\n$display_segment\n$next_line\n\n";
773 $clean = 0; 830 $clean = 0;
774 } 831 }
775 } 832 }
776 833
777#multiline macros should be enclosed in a do while loop 834# multi-statement macros should be enclosed in a do while loop, grab the
778 if (($prevline=~/\#define.*\\/) and !($prevline=~/do\s+{/) and 835# first statement and ensure its the whole macro if its not enclosed
779 !($prevline=~/\(\{/) and ($line=~/;\s*\\/) and 836# in a known goot container
780 !($line=~/do.*{/) and !($line=~/\(\{/)) { 837 if (($prevline=~/\#define.*\\/) and
781 print "Macros with multiple statements should be enclosed in a do - while loop\n"; 838 !($prevline=~/do\s+{/) and !($prevline=~/\(\{/) and
782 print "$hereprev"; 839 !($line=~/do.*{/) and !($line=~/\(\{/) and
783 $clean = 0; 840 !($line=~/^.\s*$Declare\s/)) {
841 # Grab the first statement, if that is the entire macro
842 # its ok. This may start either on the #define line
843 # or the one below.
844 my $ctx1 = join('', ctx_statement($linenr - 1, $realcnt + 1));
845 my $ctx2 = join('', ctx_statement($linenr, $realcnt));
846
847 if ($ctx1 =~ /\\$/ && $ctx2 =~ /\\$/) {
848 print "Macros with multiple statements should be enclosed in a do - while loop\n";
849 print "$hereprev";
850 $clean = 0;
851 }
784 } 852 }
785 853
786# don't include deprecated include files 854# don't include deprecated include files (uses RAW line)
787 for my $inc (@dep_includes) { 855 for my $inc (@dep_includes) {
788 if ($line =~ m@\#\s*include\s*\<$inc>@) { 856 if ($rawline =~ m@\#\s*include\s*\<$inc>@) {
789 print "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n"; 857 print "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n";
790 print "$herecurr"; 858 print "$herecurr";
791 $clean = 0; 859 $clean = 0;
@@ -845,6 +913,13 @@ sub process {
845 print "$herecurr"; 913 print "$herecurr";
846 $clean = 0; 914 $clean = 0;
847 } 915 }
916
917 if ($line =~ /$Type\s+(?:inline|__always_inline)\b/ ||
918 $line =~ /\b(?:inline|always_inline)\s+$Storage/) {
919 print "inline keyword should sit between storage class and type\n";
920 print "$herecurr";
921 $clean = 0;
922 }
848 } 923 }
849 924
850 if ($chk_patch && !$is_patch) { 925 if ($chk_patch && !$is_patch) {