X-Git-Url: http://ftp.safe.ca/?a=blobdiff_plain;f=scripts%2Fcheckpatch.pl;h=57df90699219e57bb22937d63c1ab87763522b3c;hb=ea71a0a019f913bdf506103bd90140d93a2b83f0;hp=f88bb3e21cda9c9a29470004f9e964907a1a17bb;hpb=f4432c5caec5fa95ea7eefd00f8e6cee17e2e023;p=safe%2Fjmp%2Flinux-2.6 diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index f88bb3e..57df906 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; @@ -9,7 +10,7 @@ use strict; my $P = $0; $P =~ s@.*/@@g; -my $V = '0.24'; +my $V = '0.28'; use Getopt::Long qw(:config no_auto_abbrev); @@ -27,6 +28,41 @@ my $mailback = 0; my $summary_file = 0; my $root; my %debug; +my $help = 0; + +sub help { + my ($exitcode) = @_; + + print << "EOM"; +Usage: $P [OPTION]... [FILE]... +Version: $V + +Options: + -q, --quiet quiet + --no-tree run without a kernel tree + --no-signoff do not check for 'Signed-off-by' line + --patch treat FILE as patchfile (default) + --emacs emacs compile window format + --terse one line per report + -f, --file treat FILE as regular source file + --subjective, --strict enable more subjective tests + --root=PATH PATH to the kernel tree root + --no-summary suppress the per-file summary + --mailback only produce a report in case of warnings/errors + --summary-file include the filename in summary + --debug KEY=[0|1] turn on/off debugging of KEY, where KEY is one of + 'values', 'possible', 'type', and 'attr' (default + is all off) + --test-only=WORD report only warnings/errors containing WORD + literally + -h, --help, --version display this help and exit + +When FILE is - read standard input. +EOM + + exit($exitcode); +} + GetOptions( 'q|quiet+' => \$quiet, 'tree!' => \$tree, @@ -34,7 +70,7 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, - 'file!' => \$file, + 'f|file!' => \$file, 'subjective!' => \$check, 'strict!' => \$check, 'root=s' => \$root, @@ -44,22 +80,16 @@ GetOptions( 'debug=s' => \%debug, 'test-only=s' => \$tst_only, -) or exit; + 'h|help' => \$help, + 'version' => \$help +) or help(1); + +help(0) if ($help); my $exit = 0; if ($#ARGV < 0) { - print "usage: $P [options] patchfile\n"; - print "version: $V\n"; - print "options: -q => quiet\n"; - print " --no-tree => run without a kernel tree\n"; - print " --terse => one line per report\n"; - print " --emacs => emacs compile window format\n"; - print " --file => check a source file\n"; - print " --strict => enable more subjective tests\n"; - print " --root => path to the kernel tree root\n"; - print " --no-summary => suppress the per-file summary\n"; - print " --summary-file => include the filename in summary\n"; + print "$P: no input files\n"; exit(1); } @@ -68,7 +98,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) { @@ -107,7 +139,8 @@ our $Sparse = qr{ __iomem| __must_check| __init_refok| - __kprobes + __kprobes| + __ref }x; our $Attribute = qr{ const| @@ -116,7 +149,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}; @@ -125,6 +159,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{ <=|>=|==|!=| =>|->|<<|>>|<|>|!|~| @@ -190,7 +225,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}; @@ -203,9 +238,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/(.*)@) { @@ -217,17 +252,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 '-') { @@ -235,11 +274,11 @@ for my $filename (@ARGV) { } else { $vname = $filename; } - while () { + while (<$FILE>) { chomp; push(@rawlines, $_); } - close(FILE); + close($FILE); if (!process($filename)) { $exit = 1; } @@ -346,6 +385,13 @@ sub sanitise_line { $off++; next; } + if ($sanitise_quote eq '' && substr($line, $off, 2) eq '//') { + $sanitise_quote = '//'; + + substr($res, $off, 2, $sanitise_quote); + $off++; + next; + } # A \ in a string means ignore the next character. if (($sanitise_quote eq "'" || $sanitise_quote eq '"') && @@ -366,9 +412,11 @@ 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 eq '//' && $c ne "\t") { + substr($res, $off, 1, $;); } elsif ($off != 0 && $sanitise_quote && $c ne "\t") { substr($res, $off, 1, 'X'); } else { @@ -376,6 +424,10 @@ sub sanitise_line { } } + if ($sanitise_quote eq '//') { + $sanitise_quote = ''; + } + # The pathname on a #include may be surrounded by '<' and '>'. if ($res =~ /^.\s*\#\s*include\s+\<(.*)\>/) { my $clean = 'X' x length($1); @@ -402,12 +454,15 @@ sub ctx_statement_block { my $type = ''; my $level = 0; + my @stack = (); my $p; my $c; my $len = 0; my $remainder; while (1) { + @stack = (['', 0]) if ($#stack == -1); + #warn "CSB: blk<$blk> remain<$remain>\n"; # If we are about to drop off the end, pull in more # context. @@ -433,6 +488,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 ';') { @@ -579,11 +644,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) { @@ -843,11 +919,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'; @@ -865,6 +941,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'; @@ -880,7 +960,7 @@ sub annotate_values { } $av_pend_colon = 'O'; - } elsif ($cur =~ /^(;|\[)/o) { + } elsif ($cur =~ /^(\[)/o) { print "CLOSE($1)\n" if ($dbg_values > 1); $type = 'N'; @@ -1051,6 +1131,7 @@ sub process { my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; + my $p1_prefix = ''; my $prev_values = 'E'; @@ -1097,9 +1178,12 @@ sub process { $rawlines[$ln - 1] =~ /^-/); $cnt--; #print "RAW<$rawlines[$ln - 1]>\n"; - ($edge) = (defined $rawlines[$ln - 1] && - $rawlines[$ln - 1] =~ m@(/\*|\*/)@); - last if (defined $edge); + last if (!defined $rawlines[$ln - 1]); + if ($rawlines[$ln - 1] =~ m@(/\*|\*/)@ && + $rawlines[$ln - 1] !~ m@"[^"]*(?:/\*|\*/)[^"]*"@) { + ($edge) = $1; + last; + } } if (defined $edge && $edge eq '*/') { $in_comment = 1; @@ -1109,7 +1193,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; } @@ -1196,7 +1280,13 @@ sub process { # extract the filename as it passes if ($line=~/^\+\+\+\s+(\S+)/) { $realfile = $1; - $realfile =~ s@^[^/]*/@@; + $realfile =~ s@^([^/]*)/@@; + + $p1_prefix = $1; + if (!$file && $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"); @@ -1324,6 +1414,8 @@ sub process { # Ignore functions being called } elsif ($s =~ /^.\s*$Ident\s*\(/s) { + } elsif ($s =~ /^.\s*else\b/s) { + # declarations always start with types } elsif ($prev_values eq 'E' && $s =~ /^.\s*(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?((?:\s*$Ident)+?)\b(?:\s+$Sparse)?\s*\**\s*(?:$Ident|\(\*[^\)]*\))(?:\s*$Modifier)?\s*(?:;|=|,|\()/s) { my $type = $1; @@ -1336,7 +1428,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); } @@ -1484,8 +1576,9 @@ sub process { $s =~ /^\s*#\s*?/ || $s =~ /^\s*$Ident\s*:/) { $continuation = ($s =~ /^.*?\\\n/) ? 1 : 0; - $s =~ s/^.*?\n//; - $cond_lines++; + if ($s =~ s/^.*?\n//) { + $cond_lines++; + } } } @@ -1537,9 +1630,9 @@ sub process { } # TEST: allow direct testing of the attribute matcher. if ($dbg_attr) { - if ($line =~ /^.\s*$Attribute\s*$/) { + if ($line =~ /^.\s*$Modifier\s*$/) { ERROR("TEST: is attr\n" . $herecurr); - } elsif ($dbg_attr > 1 && $line =~ /^.+($Attribute)/) { + } elsif ($dbg_attr > 1 && $line =~ /^.+($Modifier)/) { ERROR("TEST: is not attr ($1 is)\n". $herecurr); } next; @@ -1594,7 +1687,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); } @@ -1602,7 +1695,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/) { @@ -1610,21 +1703,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*(?:$Modifier\b\s*|\*\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+\*/\*\*/) { + } - } 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*(?:$Modifier\b\s*|\*\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+\*/\*\*/) { + } + # Modifiers should have spaces. + $to =~ s/(\b$Modifier$)/$1 /; + + #print "from<$from> to<$to> ident<$ident>\n"; + if ($from ne $to && $ident !~ /^$Modifier$/) { + ERROR("\"foo${from}bar\" should be \"foo${to}bar\"\n" . $herecurr); + } } # # no BUG() or BUG_ON() @@ -1759,7 +1870,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'; } @@ -1821,11 +1932,11 @@ sub process { if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) { ERROR("space required before that '$op' $at\n" . $hereptr); } - if ($op eq '*' && $cc =~/\s*const\b/) { + if ($op eq '*' && $cc =~/\s*$Modifier\b/) { # A unary '*' may be const } elsif ($ctx =~ /.xW/) { - ERROR("space prohibited after that '$op' $at\n" . $hereptr); + ERROR("Aspace prohibited after that '$op' $at\n" . $hereptr); } # unary ++ and unary -- are allowed no space on one side. @@ -1950,9 +2061,13 @@ 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 =~ s/\[[^\{\}]*\]/1/ || + $value !~ /(?:$Ident|-?$Constant)\s* + $Compare\s* + (?:$Ident|-?$Constant)/x && + $value =~ s/\([^\(\)]*\)/1/) { } if ($value =~ /^(?:$Ident|-?$Constant)$/) { @@ -1992,7 +2107,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); } @@ -2040,6 +2155,11 @@ sub process { ERROR("trailing statements should be on next line\n" . $herecurr); } } +# if should not continue a brace + if ($line =~ /}\s*if\b/) { + ERROR("trailing statements should be on next line\n" . + $herecurr); + } # case and default should not have general statements after them if ($line =~ /^.\s*(?:case\s*.*|default\s*):/g && $line !~ /\G(?: @@ -2167,9 +2287,11 @@ 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/) @@ -2189,6 +2311,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) = @@ -2443,6 +2574,12 @@ 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 !~ /\bconst\b/ && + $line =~ /\bstruct\s+(file_operations|seq_operations)\b/) { + WARN("struct $1 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 @@ -2466,6 +2603,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); + } elsif ($realfile !~ m@^kernel/@) { + 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