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
Showing the changes to scripts/checkpatch.pl (1 of 1 file). Show all
Unified · Split
| @@ -9,7 +9,7 @@ use strict; · 8 unmodified lines | ||
| 9 | 9 | my $P = $0; |
| 10 | 10 | $P =~ s@.*/@@g; |
| 11 | 11 | |
| 12 | −my $V = '0.04'; | |
| 12 | +my $V = '0.05'; | |
| 13 | 13 | |
| 14 | 14 | use Getopt::Long qw(:config no_auto_abbrev); |
| 15 | 15 | |
| @@ -17,11 +17,13 @@ my $quiet = 0; · 1 unmodified line | ||
| 17 | 17 | my $tree = 1; |
| 18 | 18 | my $chk_signoff = 1; |
| 19 | 19 | my $chk_patch = 1; |
| 20 | +my $tst_type = 0; | |
| 20 | 21 | GetOptions( |
| 21 | 22 | 'q|quiet' => \$quiet, |
| 22 | 23 | 'tree!' => \$tree, |
| 23 | 24 | 'signoff!' => \$chk_signoff, |
| 24 | 25 | 'patch!' => \$chk_patch, |
| 26 | + 'test-type!' => \$tst_type, | |
| 25 | 27 | ) or exit; |
| 26 | 28 | |
| 27 | 29 | my $exit = 0; |
| @@ -151,7 +153,7 @@ sub sanitise_line { · 123 unmodified lines | ||
| 151 | 153 | } |
| 152 | 154 | |
| 153 | 155 | sub ctx_block_get { |
| 154 | − my ($linenr, $remain, $outer) = @_; | |
| 156 | + my ($linenr, $remain, $outer, $open, $close) = @_; | |
| 155 | 157 | my $line; |
| 156 | 158 | my $start = $linenr - 1; |
| 157 | 159 | my $blk = ''; |
| @@ -165,8 +167,8 @@ sub ctx_block_get { · 7 unmodified lines | ||
| 165 | 167 | |
| 166 | 168 | $blk .= $rawlines[$line]; |
| 167 | 169 | |
| 168 | − @o = ($blk =~ /\{/g); | |
| 169 | − @c = ($blk =~ /\}/g); | |
| 170 | + @o = ($blk =~ /$open/g); | |
| 171 | + @c = ($blk =~ /$close/g); | |
| 170 | 172 | |
| 171 | 173 | if (!$outer || (scalar(@o) - scalar(@c)) == 1) { |
| 172 | 174 | push(@res, $rawlines[$line]); |
| @@ -180,12 +182,17 @@ sub ctx_block_get { · 7 unmodified lines | ||
| 180 | 182 | sub ctx_block_outer { |
| 181 | 183 | my ($linenr, $remain) = @_; |
| 182 | 184 | |
| 183 | − return ctx_block_get($linenr, $remain, 1); | |
| 185 | + return ctx_block_get($linenr, $remain, 1, '\{', '\}'); | |
| 184 | 186 | } |
| 185 | 187 | sub ctx_block { |
| 186 | 188 | my ($linenr, $remain) = @_; |
| 187 | 189 | |
| 188 | − return ctx_block_get($linenr, $remain, 0); | |
| 190 | + return ctx_block_get($linenr, $remain, 0, '\{', '\}'); | |
| 191 | +} | |
| 192 | +sub ctx_statement { | |
| 193 | + my ($linenr, $remain) = @_; | |
| 194 | + | |
| 195 | + return ctx_block_get($linenr, $remain, 0, '\(', '\)'); | |
| 189 | 196 | } |
| 190 | 197 | |
| 191 | 198 | sub ctx_locate_comment { |
| @@ -264,9 +271,30 @@ sub process { · 72 unmodified lines | ||
| 264 | 271 | my $in_comment = 0; |
| 265 | 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 | 293 | foreach my $line (@lines) { |
| 268 | 294 | $linenr++; |
| 269 | 295 | |
| 296 | + my $rawline = $line; | |
| 297 | + | |
| 270 | 298 | #extract the filename as it passes |
| 271 | 299 | if ($line=~/^\+\+\+\s+(\S+)/) { |
| 272 | 300 | $realfile=$1; |
| @@ -361,7 +389,7 @@ sub process { · 88 unmodified lines | ||
| 361 | 389 | next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); |
| 362 | 390 | |
| 363 | 391 | #trailing whitespace |
| 364 | − if ($line=~/\+.*\S\s+$/) { | |
| 392 | + if ($line=~/^\+.*\S\s+$/) { | |
| 365 | 393 | my $herevet = "$here\n" . cat_vet($line) . "\n\n"; |
| 366 | 394 | print "trailing whitespace\n"; |
| 367 | 395 | print "$herevet"; |
| @@ -392,17 +420,20 @@ sub process { · 24 unmodified lines | ||
| 392 | 420 | # |
| 393 | 421 | next if ($in_comment); |
| 394 | 422 | |
| 395 | − # Remove comments from the line before processing. | |
| 423 | +# Remove comments from the line before processing. | |
| 396 | 424 | $line =~ s@/\*.*\*/@@g; |
| 397 | 425 | $line =~ s@/\*.*@@; |
| 398 | 426 | $line =~ s@.*\*/@@; |
| 399 | 427 | |
| 400 | − # | |
| 401 | − # Checks which may be anchored in the context. | |
| 402 | − # | |
| 428 | +# Standardise the strings and chars within the input to simplify matching. | |
| 429 | + $line = sanitise_line($line); | |
| 430 | + | |
| 431 | +# | |
| 432 | +# Checks which may be anchored in the context. | |
| 433 | +# | |
| 403 | 434 | |
| 404 | − # Check for switch () and associated case and default | |
| 405 | − # statements should be at the same indent. | |
| 435 | +# Check for switch () and associated case and default | |
| 436 | +# statements should be at the same indent. | |
| 406 | 437 | if ($line=~/\bswitch\s*\(.*\)/) { |
| 407 | 438 | my $err = ''; |
| 408 | 439 | my $sep = ''; |
| @@ -428,9 +459,30 @@ sub process { · 19 unmodified lines | ||
| 428 | 459 | #ignore lines not being added |
| 429 | 460 | if ($line=~/^[^\+]/) {next;} |
| 430 | 461 | |
| 431 | − # | |
| 432 | − # Checks which are anchored on the added line. | |
| 433 | − # | |
| 462 | +# TEST: allow direct testing of the type matcher. | |
| 463 | + if ($tst_type && $line =~ /^.$Declare$/) { | |
| 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 | 487 | # no C99 // comments |
| 436 | 488 | if ($line =~ m{//}) { |
| @@ -441,47 +493,44 @@ sub process { · 4 unmodified lines | ||
| 441 | 493 | # Remove C99 comments. |
| 442 | 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 | 496 | #EXPORT_SYMBOL should immediately follow its function closing }. |
| 449 | − if (($line =~ /EXPORT_SYMBOL.*\(.*\)/) || | |
| 450 | − ($line =~ /EXPORT_UNUSED_SYMBOL.*\(.*\)/)) { | |
| 497 | + if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) || | |
| 498 | + ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { | |
| 499 | + my $name = $1; | |
| 451 | 500 | if (($prevline !~ /^}/) && |
| 452 | 501 | ($prevline !~ /^\+}/) && |
| 453 | − ($prevline !~ /^ }/)) { | |
| 454 | − print "EXPORT_SYMBOL(func); should immediately follow its function\n"; | |
| 502 | + ($prevline !~ /^ }/) && | |
| 503 | + ($prevline !~ /\s$name(?:\s+$Attribute)?\s*(?:;|=)/)) { | |
| 504 | + print "EXPORT_SYMBOL(foo); should immediately follow its function/variable\n"; | |
| 455 | 505 | print "$herecurr"; |
| 456 | 506 | $clean = 0; |
| 457 | 507 | } |
| 458 | 508 | } |
| 459 | 509 | |
| 460 | − # check for static initialisers. | |
| 510 | +# check for static initialisers. | |
| 461 | 511 | if ($line=~/\s*static\s.*=\s+(0|NULL);/) { |
| 462 | 512 | print "do not initialise statics to 0 or NULL\n"; |
| 463 | 513 | print "$herecurr"; |
| 464 | 514 | $clean = 0; |
| 465 | 515 | } |
| 466 | 516 | |
| 467 | − # check for new typedefs. | |
| 468 | − if ($line=~/\s*typedef\s/) { | |
| 517 | +# check for new typedefs, only function parameters and sparse annotations | |
| 518 | +# make sense. | |
| 519 | + if ($line =~ /\btypedef\s/ && | |
| 520 | + $line !~ /\btypedef\s+$Type\s+\(\s*$Ident\s*\)\s*\(/ && | |
| 521 | + $line !~ /\b__bitwise(?:__|)\b/) { | |
| 469 | 522 | print "do not add new typedefs\n"; |
| 470 | 523 | print "$herecurr"; |
| 471 | 524 | $clean = 0; |
| 472 | 525 | } |
| 473 | 526 | |
| 474 | 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 | 528 | if ($line =~ m{[A-Za-z\d_]+(\*+) [A-Za-z\d_]+}) { |
| 480 | 529 | print "\"foo$1 bar\" should be \"foo $1bar\"\n"; |
| 481 | 530 | print "$herecurr"; |
| 482 | 531 | $clean = 0; |
| 483 | 532 | } |
| 484 | − if ($line =~ m{$type (\*) [A-Za-z\d_]+} || | |
| 533 | + if ($line =~ m{$Type (\*) [A-Za-z\d_]+} || | |
| 485 | 534 | $line =~ m{[A-Za-z\d_]+ (\*\*+) [A-Za-z\d_]+}) { |
| 486 | 535 | print "\"foo $1 bar\" should be \"foo $1bar\"\n"; |
| 487 | 536 | print "$herecurr"; |
| @@ -530,13 +579,16 @@ sub process { · 42 unmodified lines | ||
| 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 | 584 | if (($line=~/[A-Za-z\d_]+\**\s+\**[A-Za-z\d_]+\(.*\).* {/) and |
| 535 | 585 | !($line=~/\#define.*do\s{/) and !($line=~/}/)) { |
| 536 | 586 | print "braces following function declarations go on the next line\n"; |
| 537 | 587 | print "$herecurr"; |
| 538 | 588 | $clean = 0; |
| 539 | 589 | } |
| 590 | + | |
| 591 | +# Check operator spacing. | |
| 540 | 592 | # Note we expand the line with the leading + as the real |
| 541 | 593 | # line will be displayed with the leading + and the tabs |
| 542 | 594 | # will therefore also expand that way. |
| @@ -544,7 +596,6 @@ sub process { · 1 unmodified line | ||
| 544 | 596 | $opline = expand_tabs($opline); |
| 545 | 597 | $opline =~ s/^./ /; |
| 546 | 598 | if (!($line=~/\#\s*include/)) { |
| 547 | − # Check operator spacing. | |
| 548 | 599 | my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline); |
| 549 | 600 | my $off = 0; |
| 550 | 601 | for (my $n = 0; $n < $#elements; $n += 2) { |
| @@ -572,8 +623,8 @@ sub process { · 21 unmodified lines | ||
| 572 | 623 | # Pick up the preceeding and succeeding characters. |
| 573 | 624 | my $ca = substr($opline, $off - 1, 1); |
| 574 | 625 | my $cc = ''; |
| 575 | − if (length($opline) > ($off + length($elements[$n]))) { | |
| 576 | − $cc = substr($opline, $off + 1 + length($elements[$n]), 1); | |
| 626 | + if (length($opline) >= ($off + length($elements[$n + 1]))) { | |
| 627 | + $cc = substr($opline, $off + length($elements[$n + 1]), 1); | |
| 577 | 628 | } |
| 578 | 629 | |
| 579 | 630 | my $ctx = "${a}x${c}"; |
| @@ -598,7 +649,7 @@ sub process { · 18 unmodified lines | ||
| 598 | 649 | |
| 599 | 650 | # , must have a space on the right. |
| 600 | 651 | } elsif ($op eq ',') { |
| 601 | − if ($ctx !~ /.xW|.xE/) { | |
| 652 | + if ($ctx !~ /.xW|.xE/ && $cc ne '}') { | |
| 602 | 653 | print "need space after that '$op' $at\n"; |
| 603 | 654 | print "$hereptr"; |
| 604 | 655 | $clean = 0; |
| @@ -619,11 +670,16 @@ sub process { · 14 unmodified lines | ||
| 619 | 670 | |
| 620 | 671 | # unary ++ and unary -- are allowed no space on one side. |
| 621 | 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 | 674 | print "need space one side of that '$op' $at\n"; |
| 624 | 675 | print "$hereptr"; |
| 625 | 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 | 684 | # & is both unary and binary |
| 629 | 685 | # unary: |
| @@ -656,7 +712,7 @@ sub process { · 26 unmodified lines | ||
| 656 | 712 | print "$hereptr"; |
| 657 | 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 | 716 | print "need space before that '$op' $at\n"; |
| 661 | 717 | print "$hereptr"; |
| 662 | 718 | $clean = 0; |
| @@ -705,9 +761,9 @@ sub process { · 42 unmodified lines | ||
| 705 | 761 | } |
| 706 | 762 | |
| 707 | 763 | # Check for illegal assignment in if conditional. |
| 708 | − if ($line=~/\b(if|while)\s*\(.*[^<>!=]=[^=].*\)/) { | |
| 764 | + if ($line=~/\bif\s*\(.*[^<>!=]=[^=].*\)/) { | |
| 709 | 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 | 767 | print "$herecurr"; |
| 712 | 768 | $clean = 0; |
| 713 | 769 | } |
| @@ -735,8 +791,8 @@ sub process { · 21 unmodified lines | ||
| 735 | 791 | $clean = 0; |
| 736 | 792 | } |
| 737 | 793 | |
| 738 | −#warn if <asm/foo.h> is #included and <linux/foo.h> is available. | |
| 739 | − if ($tree && $line =~ qr|\s*\#\s*include\s*\<asm\/(.*)\.h\>|) { | |
| 794 | +#warn if <asm/foo.h> is #included and <linux/foo.h> is available (uses RAW line) | |
| 795 | + if ($tree && $rawline =~ m{^.\#\s*include\s*\<asm\/(.*)\.h\>}) { | |
| 740 | 796 | my $checkfile = "include/linux/$1.h"; |
| 741 | 797 | if (-f $checkfile) { |
| 742 | 798 | print "Use #include <linux/$1.h> instead of <asm/$1.h>\n"; |
| @@ -745,7 +801,8 @@ sub process { · 2 unmodified lines | ||
| 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 | 806 | if ($prevline=~/\b(if|while|for|switch)\s*\(/) { |
| 750 | 807 | my @opened = $prevline=~/\(/g; |
| 751 | 808 | my @closed = $prevline=~/\)/g; |
| @@ -767,25 +824,36 @@ sub process { · 15 unmodified lines | ||
| 767 | 824 | } |
| 768 | 825 | |
| 769 | 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 | 828 | print "That { should be on the previous line\n"; |
| 772 | 829 | print "$here\n$display_segment\n$next_line\n\n"; |
| 773 | 830 | $clean = 0; |
| 774 | 831 | } |
| 775 | 832 | } |
| 776 | 833 | |
| 777 | −#multiline macros should be enclosed in a do while loop | |
| 778 | − if (($prevline=~/\#define.*\\/) and !($prevline=~/do\s+{/) and | |
| 779 | − !($prevline=~/\(\{/) and ($line=~/;\s*\\/) and | |
| 780 | − !($line=~/do.*{/) and !($line=~/\(\{/)) { | |
| 781 | − print "Macros with multiple statements should be enclosed in a do - while loop\n"; | |
| 782 | − print "$hereprev"; | |
| 783 | − $clean = 0; | |
| 834 | +# multi-statement macros should be enclosed in a do while loop, grab the | |
| 835 | +# first statement and ensure its the whole macro if its not enclosed | |
| 836 | +# in a known goot container | |
| 837 | + if (($prevline=~/\#define.*\\/) and | |
| 838 | + !($prevline=~/do\s+{/) and !($prevline=~/\(\{/) and | |
| 839 | + !($line=~/do.*{/) and !($line=~/\(\{/) and | |
| 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 | 855 | for my $inc (@dep_includes) { |
| 788 | − if ($line =~ m@\#\s*include\s*\<$inc>@) { | |
| 856 | + if ($rawline =~ m@\#\s*include\s*\<$inc>@) { | |
| 789 | 857 | print "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n"; |
| 790 | 858 | print "$herecurr"; |
| 791 | 859 | $clean = 0; |
| @@ -845,6 +913,13 @@ sub process { · 53 unmodified lines | ||
| 845 | 913 | print "$herecurr"; |
| 846 | 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 | 925 | if ($chk_patch && !$is_patch) { |