X-Git-Url: http://ftp.safe.ca/?a=blobdiff_plain;f=scripts%2Fcheckpatch.pl;h=bd88f11b0953d161b259e80b27d6f423b12e8b1c;hb=91803b499cca2fe558abad709ce83dc896b80950;hp=fd21b5eaed3fc466878087ac416d9905ba4f57a4;hpb=171ae1a491e216ef728436f9cc958e05cccf5a27;p=safe%2Fjmp%2Flinux-2.6 diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fd21b5e..bd88f11 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) 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,2009, 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.17'; +my $V = '0.30'; use Getopt::Long qw(:config no_auto_abbrev); @@ -17,7 +18,6 @@ my $quiet = 0; my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; -my $tst_type = 0; my $tst_only; my $emacs = 0; my $terse = 0; @@ -28,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, @@ -35,7 +70,7 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, - 'file!' => \$file, + 'f|file!' => \$file, 'subjective!' => \$check, 'strict!' => \$check, 'root=s' => \$root, @@ -44,31 +79,28 @@ GetOptions( 'summary-file!' => \$summary_file, 'debug=s' => \%debug, - 'test-type!' => \$tst_type, '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); } my $dbg_values = 0; 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) { @@ -98,7 +130,10 @@ if ($tree) { my $emitted_corrupt = 0; -our $Ident = qr{[A-Za-z_][A-Za-z\d_]*}; +our $Ident = qr{ + [A-Za-z_][A-Za-z\d_]* + (?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)* + }x; our $Storage = qr{extern|static|asmlinkage}; our $Sparse = qr{ __user| @@ -107,20 +142,30 @@ our $Sparse = qr{ __iomem| __must_check| __init_refok| - __kprobes + __kprobes| + __ref }x; + +# Notes to $Attribute: +# We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| __read_mostly| __kprobes| - __(?:mem|cpu|dev|)(?:initdata|init) + __(?:mem|cpu|dev|)(?:initdata|initconst|init\b)| + ____cacheline_aligned| + ____cacheline_aligned_in_smp| + ____cacheline_internodealigned_in_smp| + __weak }x; +our $Modifier; our $Inline = qr{inline|__always_inline|noinline}; our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; 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{ <=|>=|==|!=| =>|->|<<|>>|<|>|!|~| @@ -142,20 +187,32 @@ our $UTF8 = qr { | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 }x; +our $typeTypedefs = qr{(?x: + (?:__)?(?:u|s|be|le)(?:8|16|32|64)| + atomic_t +)}; + +our $logFunctions = qr{(?x: + printk| + pr_(debug|dbg|vdbg|devel|info|warning|err|notice|alert|crit|emerg|cont)| + dev_(printk|dbg|vdbg|info|warn|err|notice|alert|crit|emerg|WARN)| + WARN| + panic +)}; + our @typeList = ( qr{void}, - qr{char}, - qr{short}, - qr{int}, - qr{long}, + qr{(?:unsigned\s+)?char}, + qr{(?:unsigned\s+)?short}, + qr{(?:unsigned\s+)?int}, + qr{(?:unsigned\s+)?long}, + qr{(?:unsigned\s+)?long\s+int}, + qr{(?:unsigned\s+)?long\s+long}, + qr{(?:unsigned\s+)?long\s+long\s+int}, qr{unsigned}, qr{float}, qr{double}, qr{bool}, - qr{long\s+int}, - qr{long\s+long}, - qr{long\s+long\s+int}, - qr{(?:__)?(?:u|s|be|le)(?:8|16|32|64)}, qr{struct\s+$Ident}, qr{union\s+$Ident}, qr{enum\s+$Ident}, @@ -163,24 +220,27 @@ our @typeList = ( qr{${Ident}_handler}, qr{${Ident}_handler_fn}, ); +our @modifierList = ( + qr{fastcall}, +); sub build_types { - my $all = "(?: \n" . join("|\n ", @typeList) . "\n)"; + my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)"; + my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)"; + $Modifier = qr{(?:$Attribute|$Sparse|$mods)}; $NonptrType = qr{ - \b - (?:const\s+)? - (?:unsigned\s+)? + (?:$Modifier\s+|const\s+)* (?: - $all| - (?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\) + (?:typeof|__typeof__)\s*\(\s*\**\s*$Ident\s*\)| + (?:$typeTypedefs\b)| + (?:${all}\b) ) - (?:\s+$Sparse|\s+const)* - \b + (?:\s+$Modifier|\s+const)* }x; $Type = qr{ - \b$NonptrType\b - (?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)? - (?:\s+$Inline|\s+$Sparse|\s+$Attribute)* + $NonptrType + (?:[\s\*]+\s*const|[\s\*]+|(?:\s*\[\s*\])+)? + (?:\s+$Inline|\s+$Modifier)* }x; $Declare = qr{(?:$Storage\s+)?$Type}; } @@ -192,9 +252,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/(.*)@) { @@ -206,17 +266,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 '-') { @@ -224,11 +288,11 @@ for my $filename (@ARGV) { } else { $vname = $filename; } - while () { + while (<$FILE>) { chomp; push(@rawlines, $_); } - close(FILE); + close($FILE); if (!process($filename)) { $exit = 1; } @@ -329,12 +393,19 @@ sub sanitise_line { $off++; next; } - if (substr($line, $off, 2) eq $sanitise_quote) { + if ($sanitise_quote eq '*/' && substr($line, $off, 2) eq '*/') { $sanitise_quote = ''; substr($res, $off, 2, "$;$;"); $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 '"') && @@ -355,9 +426,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 { @@ -365,15 +438,19 @@ sub sanitise_line { } } + if ($sanitise_quote eq '//') { + $sanitise_quote = ''; + } + # The pathname on a #include may be surrounded by '<' and '>'. - if ($res =~ /^.#\s*include\s+\<(.*)\>/) { + if ($res =~ /^.\s*\#\s*include\s+\<(.*)\>/) { my $clean = 'X' x length($1); $res =~ s@\<.*\>@<$clean>@; # The whole of a #error is a string. - } elsif ($res =~ /^.#\s*(?:error|warning)\s+(.*)\b/) { + } elsif ($res =~ /^.\s*\#\s*(?:error|warning)\s+(.*)\b/) { my $clean = 'X' x length($1); - $res =~ s@(#\s*(?:error|warning)\s+).*@$1$clean@; + $res =~ s@(\#\s*(?:error|warning)\s+).*@$1$clean@; } return $res; @@ -391,17 +468,21 @@ 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. if ($off >= $len) { for (; $remain > 0; $line++) { + last if (!defined $lines[$line]); next if ($lines[$line] =~ /^-/); $remain--; $loff = $len; @@ -421,6 +502,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 ';') { @@ -466,7 +557,9 @@ sub ctx_statement_block { } $off++; } + # We are truly at the end, so shuffle to the next line. if ($off == $len) { + $loff = $len + 1; $line++; $remain--; } @@ -565,11 +658,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) { @@ -627,7 +731,7 @@ sub ctx_locate_comment { my ($first_line, $end_line) = @_; # Catch a comment on the end of the line itself. - my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*$@); + my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); return $current_comment if (defined $current_comment); # Look through the context and try and figure out if there is a @@ -665,6 +769,22 @@ sub ctx_has_comment { return ($cmt ne ''); } +sub raw_line { + my ($linenr, $cnt) = @_; + + my $offset = $linenr - 1; + $cnt++; + + my $line; + while ($cnt) { + $line = $rawlines[$offset++]; + next if (defined($line) && $line =~ /^-/); + $cnt--; + } + + return $line; +} + sub cat_vet { my ($vet) = @_; my ($res, $coded); @@ -685,17 +805,20 @@ sub cat_vet { my $av_preprocessor = 0; my $av_pending; my @av_paren_type; +my $av_pend_colon; sub annotate_reset { $av_preprocessor = 0; $av_pending = '_'; @av_paren_type = ('E'); + $av_pend_colon = 'O'; } sub annotate_values { my ($stream, $type) = @_; my $res; + my $var = '_' x length($stream); my $cur = $stream; print "$stream\n" if ($dbg_values > 1); @@ -711,11 +834,15 @@ sub annotate_values { $av_preprocessor = 0; } - } elsif ($cur =~ /^($Type)/) { + } elsif ($cur =~ /^($Type)\s*(?:$Ident|,|\)|\()/) { print "DECLARE($1)\n" if ($dbg_values > 1); $type = 'T'; - } elsif ($cur =~ /^(#\s*define\s*$Ident)(\(?)/o) { + } elsif ($cur =~ /^($Modifier)\s*/) { + print "MODIFIER($1)\n" if ($dbg_values > 1); + $type = 'T'; + + } elsif ($cur =~ /^(\#\s*define\s*$Ident)(\(?)/o) { print "DEFINE($1,$2)\n" if ($dbg_values > 1); $av_preprocessor = 1; push(@av_paren_type, $type); @@ -724,12 +851,12 @@ sub annotate_values { } $type = 'E'; - } elsif ($cur =~ /^(#\s*undef\s*$Ident)/o) { + } elsif ($cur =~ /^(\#\s*(?:undef\s*$Ident|include\b))/o) { print "UNDEF($1)\n" if ($dbg_values > 1); $av_preprocessor = 1; push(@av_paren_type, $type); - } elsif ($cur =~ /^(#\s*(?:ifdef|ifndef|if))/o) { + } elsif ($cur =~ /^(\#\s*(?:ifdef|ifndef|if))/o) { print "PRE_START($1)\n" if ($dbg_values > 1); $av_preprocessor = 1; @@ -737,7 +864,7 @@ sub annotate_values { push(@av_paren_type, $type); $type = 'E'; - } elsif ($cur =~ /^(#\s*(?:else|elif))/o) { + } elsif ($cur =~ /^(\#\s*(?:else|elif))/o) { print "PRE_RESTART($1)\n" if ($dbg_values > 1); $av_preprocessor = 1; @@ -745,7 +872,7 @@ sub annotate_values { $type = 'E'; - } elsif ($cur =~ /^(#\s*(?:endif))/o) { + } elsif ($cur =~ /^(\#\s*(?:endif))/o) { print "PRE_END($1)\n" if ($dbg_values > 1); $av_preprocessor = 1; @@ -771,12 +898,17 @@ sub annotate_values { } $type = 'N'; - } elsif ($cur =~ /^(if|while|typeof|__typeof__|for)\b/o) { + } elsif ($cur =~ /^(if|while|for)\b/o) { print "COND($1)\n" if ($dbg_values > 1); - $av_pending = 'N'; + $av_pending = 'E'; $type = 'N'; - } elsif ($cur =~/^(return|case|else)/o) { + } elsif ($cur =~/^(case)/o) { + print "CASE($1)\n" if ($dbg_values > 1); + $av_pend_colon = 'C'; + $type = 'N'; + + } elsif ($cur =~/^(return|else|goto|typeof|__typeof__)\b/o) { print "KEYWORD($1)\n" if ($dbg_values > 1); $type = 'N'; @@ -796,10 +928,20 @@ sub annotate_values { print "PAREN('$1')\n" if ($dbg_values > 1); } - } elsif ($cur =~ /^($Ident)\(/o) { + } elsif ($cur =~ /^($Ident)\s*\(/o) { print "FUNC($1)\n" if ($dbg_values > 1); + $type = 'V'; $av_pending = 'V'; + } 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'; + } elsif ($cur =~ /^($Ident|$Constant)/o) { print "IDENT($1)\n" if ($dbg_values > 1); $type = 'V'; @@ -811,11 +953,44 @@ sub annotate_values { } elsif ($cur =~/^(;|{|})/) { print "END($1)\n" if ($dbg_values > 1); $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'; + + } elsif ($cur =~ /^(:)/o) { + print "COLON($1,$av_pend_colon)\n" if ($dbg_values > 1); - } elsif ($cur =~ /^(;|\?|:|\[)/o) { + substr($var, length($res), 1, $av_pend_colon); + if ($av_pend_colon eq 'C' || $av_pend_colon eq 'L') { + $type = 'E'; + } else { + $type = 'N'; + } + $av_pend_colon = 'O'; + + } elsif ($cur =~ /^(\[)/o) { print "CLOSE($1)\n" if ($dbg_values > 1); $type = 'N'; + } elsif ($cur =~ /^(-(?![->])|\+(?!\+)|\*|\&\&|\&)/o) { + my $variant; + + print "OPV($1)\n" if ($dbg_values > 1); + if ($type eq 'V') { + $variant = 'B'; + } else { + $variant = 'U'; + } + + substr($var, length($res), 1, $variant); + $type = 'N'; + } elsif ($cur =~ /^($Operators)/o) { print "OP($1)\n" if ($dbg_values > 1); if ($1 ne '++' && $1 ne '--') { @@ -831,21 +1006,51 @@ sub annotate_values { } } - return $res; + return ($res, $var); } sub possible { my ($possible, $line) = @_; + my $notPermitted = qr{(?: + ^(?: + $Modifier| + $Storage| + $Type| + DEFINE_\S+ + )$| + ^(?: + goto| + return| + case| + else| + asm|__asm__| + do + )(?:\s|$)| + ^(?:typedef|struct|enum)\b + )}x; + warn "CHECK<$possible> ($line)\n" if ($dbg_possible > 2); + if ($possible !~ $notPermitted) { + # Check for modifiers. + $possible =~ s/\s*$Storage\s*//g; + $possible =~ s/\s*$Sparse\s*//g; + if ($possible =~ /^\s*$/) { + + } elsif ($possible =~ /\s/) { + $possible =~ s/\s*$Type\s*//g; + for my $modifier (split(' ', $possible)) { + if ($modifier !~ $notPermitted) { + warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible); + push(@modifierList, $modifier); + } + } - #print "CHECK<$possible>\n"; - if ($possible !~ /^(?:$Storage|$Type|DEFINE_\S+)$/ && - $possible ne 'goto' && $possible ne 'return' && - $possible ne 'struct' && $possible ne 'enum' && - $possible ne 'case' && $possible ne 'else' && - $possible ne 'typedef') { - warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible); - push(@typeList, $possible); + } else { + warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible); + push(@typeList, $possible); + } build_types(); + } else { + warn "NOTPOSS: $possible ($line)\n" if ($dbg_possible > 1); } } @@ -885,6 +1090,33 @@ sub CHK { } } +sub check_absolute_file { + my ($absolute, $herecurr) = @_; + my $file = $absolute; + + ##print "absolute<$absolute>\n"; + + # See if any suffix of this path is a path within the tree. + while ($file =~ s@^[^/]*/@@) { + if (-f "$root/$file") { + ##print "file<$file>\n"; + last; + } + } + if (! -f _) { + return 0; + } + + # It is, so see if the prefix is acceptable. + my $prefix = $absolute; + substr($prefix, -length($file)) = ''; + + ##print "prefix<$prefix>\n"; + if ($prefix ne ".../") { + WARN("use relative pathname instead of absolute in changelog text\n" . $herecurr); + } +} + sub process { my $filename = shift; @@ -917,11 +1149,14 @@ sub process { my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; + my $p1_prefix = ''; my $prev_values = 'E'; # suppression flags my %suppress_ifbraces; + my %suppress_whiletrailers; + my %suppress_export; # Pre-scan the patch sanitizing the lines. # Pre-scan the patch looking for any __setup documentation. @@ -949,16 +1184,25 @@ sub process { } else { $realcnt=1+1; } + $in_comment = 0; # Guestimate if this is a continuing comment. Run # the context looking for a comment "edge". If this # edge is a close comment then we must be in a comment # at context start. my $edge; - for (my $ln = $linenr + 1; $ln < ($linenr + $realcnt); $ln++) { - next if ($line =~ /^-/); - ($edge) = ($rawlines[$ln - 1] =~ m@(/\*|\*/)@); - last if (defined $edge); + my $cnt = $realcnt; + for (my $ln = $linenr + 1; $cnt > 0; $ln++) { + next if (defined $rawlines[$ln - 1] && + $rawlines[$ln - 1] =~ /^-/); + $cnt--; + #print "RAW<$rawlines[$ln - 1]>\n"; + 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; @@ -968,7 +1212,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; } @@ -1020,6 +1264,8 @@ sub process { $prev_values = 'E'; %suppress_ifbraces = (); + %suppress_whiletrailers = (); + %suppress_export = (); next; # track the line number as we move through the hunk, note that @@ -1043,6 +1289,8 @@ sub process { $realcnt--; } + my $hunk_line = ($realcnt != 0); + #make up the handle for any error we report on this line $prefix = "$filename:$realline: " if ($emacs && $file); $prefix = "$filename:$linenr: " if ($emacs && !$file); @@ -1053,9 +1301,15 @@ sub process { # extract the filename as it passes if ($line=~/^\+\+\+\s+(\S+)/) { $realfile = $1; - $realfile =~ s@^[^/]*/@@; + $realfile =~ s@^([^/]*)/@@; - if ($realfile =~ m@include/asm/@) { + $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"); } next; @@ -1089,6 +1343,20 @@ sub process { $herecurr) if (!$emitted_corrupt++); } +# Check for absolute kernel paths. + if ($tree) { + while ($line =~ m{(?:^|\s)(/\S*)}g) { + my $file = $1; + + if ($file =~ m{^(.*?)(?::\d+)+:?$} && + check_absolute_file($1, $herecurr)) { + # + } else { + check_absolute_file($file, $herecurr); + } + } + } + # UTF-8 regex found at http://www.w3.org/International/questions/qa-forms-utf-8.en.php if (($realfile =~ /^$/ || $line =~ /^\+/) && $rawline !~ m/^$UTF8*$/) { @@ -1101,11 +1369,8 @@ sub process { ERROR("Invalid UTF-8, patch and commit message should be encoded in UTF-8\n" . $hereptr); } -#ignore lines being removed - if ($line=~/^-/) {next;} - -# check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); +# ignore non-hunk lines and lines being removed + next if (!$hunk_line || $line =~ /^-/); #trailing whitespace if ($line =~ /^\+.*\015/) { @@ -1116,18 +1381,58 @@ sub process { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; ERROR("trailing whitespace\n" . $herevet); } + +# check for Kconfig help text having a real description + if ($realfile =~ /Kconfig/ && + $line =~ /\+?\s*(---)?help(---)?$/) { + my $length = 0; + for (my $l = $linenr; defined($lines[$l]); $l++) { + my $f = $lines[$l]; + $f =~ s/#.*//; + $f =~ s/^\s+//; + next if ($f =~ /^$/); + last if ($f =~ /^\s*config\s/); + $length++; + } + WARN("please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($length < 4); + } + +# check we are in a valid source file if not then ignore this hunk + next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); + #80 column limit - if ($line =~ /^\+/ && !($prevrawline=~/\/\*\*/) && $length > 80) { + if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && + $rawline !~ /^.\s*\*\s*\@$Ident\s/ && + $line !~ /^\+\s*$logFunctions\s*\(\s*(?:KERN_\S+\s*)?"[X\t]*"\s*(?:,|\)\s*;)\s*$/ && + $length > 80) + { WARN("line over 80 characters\n" . $herecurr); } +# check for spaces before a quoted newline + if ($rawline =~ /^.*\".*\s\\n/) { + WARN("unnecessary whitespace before a quoted newline\n" . $herecurr); + } + # check for adding lines without a newline. if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) { WARN("adding a line without newline at end of file\n" . $herecurr); } -# check we are in a valid source file *.[hc] if not then ignore this hunk - next if ($realfile !~ /\.[hc]$/); +# Blackfin: use hi/lo macros + if ($realfile =~ m@arch/blackfin/.*\.S$@) { + if ($line =~ /\.[lL][[:space:]]*=.*&[[:space:]]*0x[fF][fF][fF][fF]/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("use the LO() macro, not (... & 0xFFFF)\n" . $herevet); + } + if ($line =~ /\.[hH][[:space:]]*=.*>>[[:space:]]*16/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("use the HI() macro, not (... >> 16)\n" . $herevet); + } + } + +# check we are in a valid source file C or perl if not then ignore this hunk + next if ($realfile !~ /\.(h|c|pl)$/); # at the beginning of a line any tabs must come first and anything # more than 8 must use tabs. @@ -1137,19 +1442,47 @@ sub process { ERROR("code indent should use tabs where possible\n" . $herevet); } +# check for space before tabs. + if ($rawline =~ /^\+/ && $rawline =~ / \t/) { + my $herevet = "$here\n" . cat_vet($rawline) . "\n"; + WARN("please, no space before tabs\n" . $herevet); + } + +# check we are in a valid C source file if not then ignore this hunk + next if ($realfile !~ /\.(h|c)$/); + # check for RCS/CVS revision markers if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) { WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr); } +# Blackfin: don't use __builtin_bfin_[cs]sync + if ($line =~ /__builtin_bfin_csync/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("use the CSYNC() macro in asm/blackfin.h\n" . $herevet); + } + if ($line =~ /__builtin_bfin_ssync/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("use the SSYNC() macro in asm/blackfin.h\n" . $herevet); + } + # Check for potential 'bare' types - my ($stat, $cond); - if ($realcnt) { - ($stat, $cond) = ctx_statement_block($linenr, - $realcnt, 0); + my ($stat, $cond, $line_nr_next, $remain_next, $off_next, + $realline_next); + if ($realcnt && $line =~ /.\s*\S/) { + ($stat, $cond, $line_nr_next, $remain_next, $off_next) = + ctx_statement_block($linenr, $realcnt, 0); $stat =~ s/\n./\n /g; $cond =~ s/\n./\n /g; + # Find the real next line. + $realline_next = $line_nr_next; + if (defined $realline_next && + (!defined $lines[$realline_next - 1] || + substr($lines[$realline_next - 1], $off_next) =~ /^\s*$/)) { + $realline_next++; + } + my $s = $stat; $s =~ s/{.*$//s; @@ -1159,18 +1492,22 @@ sub process { # Ignore functions being called } elsif ($s =~ /^.\s*$Ident\s*\(/s) { - # definitions in global scope can only start with types - } elsif ($s =~ /^.(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?($Ident)\b/s) { - possible($1, $s); + } elsif ($s =~ /^.\s*else\b/s) { # declarations always start with types - } elsif ($prev_values eq 'E' && $s =~ /^.\s*(?:$Storage\s+)?(?:const\s+)?($Ident)\b(:?\s+$Sparse)?\s*\**\s*$Ident\s*(?:;|=|,)/s) { - possible($1, $s); + } 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; + $type =~ s/\s+/ /g; + possible($type, "A:" . $s); + + # definitions in global scope can only start with types + } elsif ($s =~ /^.(?:$Storage\s+)?(?:$Inline\s+)?(?:const\s+)?($Ident)\b\s*(?!:)/s) { + possible($1, "B:" . $s); } # any (foo ... *) is a pointer cast, and foo is a type - while ($s =~ /\(($Ident)(?:\s+$Sparse)*\s*\*+\s*\)/sg) { - possible($1, $s); + while ($s =~ /\(($Ident)(?:\s+$Sparse)*[\s\*]+\s*\)/sg) { + possible($1, "C:" . $s); } # Check for any sort of function declaration. @@ -1184,9 +1521,9 @@ sub process { $ctx =~ s/\)[^\)]*$//; for my $arg (split(/\s*,\s*/, $ctx)) { - if ($arg =~ /^(?:const\s+)?($Ident)(?:\s+$Sparse)*\s*\**\s*(:?\b$Ident)?$/ || $arg =~ /^($Ident)$/) { + if ($arg =~ /^(?:const\s+)?($Ident)(?:\s+$Sparse)*\s*\**\s*(:?\b$Ident)?$/s || $arg =~ /^($Ident)$/s) { - possible($1, $s); + possible($1, "D:" . $s); } } } @@ -1221,25 +1558,30 @@ sub process { # 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 - if ($line =~ /(.*)\b((?:if|while|for|switch)\s*\(|do\b|else\b)/ && $line !~ /^.#/) { + if ($line =~ /(.*)\b((?:if|while|for|switch)\s*\(|do\b|else\b)/ && $line !~ /^.\s*\#/) { my $pre_ctx = "$1$2"; my ($level, @ctx) = ctx_statement_level($linenr, $realcnt, 0); - my $ctx_ln = $linenr + $#ctx + 1; my $ctx_cnt = $realcnt - $#ctx - 1; my $ctx = join("\n", @ctx); - ##warn "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n"; + my $ctx_ln = $linenr; + my $ctx_skip = $realcnt; - # Skip over any removed lines in the context following statement. - while (defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^-/) { + while ($ctx_skip > $ctx_cnt || ($ctx_skip == $ctx_cnt && + defined $lines[$ctx_ln - 1] && + $lines[$ctx_ln - 1] =~ /^-/)) { + ##print "SKIP<$ctx_skip> CNT<$ctx_cnt>\n"; + $ctx_skip-- if (!defined $lines[$ctx_ln - 1] || $lines[$ctx_ln - 1] !~ /^-/); $ctx_ln++; } - ##warn "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n"; + + #print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n"; + #print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n"; if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { ERROR("that open brace { should be on the previous line\n" . - "$here\n$ctx\n$lines[$ctx_ln - 1]"); + "$here\n$ctx\n$lines[$ctx_ln - 1]\n"); } if ($level == 0 && $pre_ctx !~ /}\s*while\s*\($/ && $ctx =~ /\)\s*\;\s*$/ && @@ -1248,19 +1590,107 @@ sub process { my ($nlength, $nindent) = line_stats($lines[$ctx_ln - 1]); if ($nindent > $indent) { WARN("trailing semicolon indicates no statements, indent implies otherwise\n" . - "$here\n$ctx\n$lines[$ctx_ln - 1]"); + "$here\n$ctx\n$lines[$ctx_ln - 1]\n"); } } } +# Check relative indent for conditionals and blocks. + if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { + my ($s, $c) = ($stat, $cond); + + substr($s, 0, length($c), ''); + + # Make sure we remove the line prefixes as we have + # none on the first line, and are going to readd them + # where necessary. + $s =~ s/\n./\n/gs; + + # Find out how long the conditional actually is. + my @newlines = ($c =~ /\n/gs); + my $cond_lines = 1 + $#newlines; + + # We want to check the first line inside the block + # starting at the end of the conditional, so remove: + # 1) any blank line termination + # 2) any opening brace { on end of the line + # 3) any do (...) { + my $continuation = 0; + my $check = 0; + $s =~ s/^.*\bdo\b//; + $s =~ s/^\s*{//; + if ($s =~ s/^\s*\\//) { + $continuation = 1; + } + if ($s =~ s/^\s*?\n//) { + $check = 1; + $cond_lines++; + } + + # Also ignore a loop construct at the end of a + # preprocessor statement. + if (($prevline =~ /^.\s*#\s*define\s/ || + $prevline =~ /\\\s*$/) && $continuation == 0) { + $check = 0; + } + + my $cond_ptr = -1; + $continuation = 0; + while ($cond_ptr != $cond_lines) { + $cond_ptr = $cond_lines; + + # If we see an #else/#elif then the code + # is not linear. + if ($s =~ /^\s*\#\s*(?:else|elif)/) { + $check = 0; + } + + # Ignore: + # 1) blank lines, they should be at 0, + # 2) preprocessor lines, and + # 3) labels. + if ($continuation || + $s =~ /^\s*?\n/ || + $s =~ /^\s*#\s*?/ || + $s =~ /^\s*$Ident\s*:/) { + $continuation = ($s =~ /^.*?\\\n/) ? 1 : 0; + if ($s =~ s/^.*?\n//) { + $cond_lines++; + } + } + } + + my (undef, $sindent) = line_stats("+" . $s); + my $stat_real = raw_line($linenr, $cond_lines); + + # Check if either of these lines are modified, else + # this is not this patch's fault. + if (!defined($stat_real) || + $stat !~ /^\+/ && $stat_real !~ /^\+/) { + $check = 0; + } + if (defined($stat_real) && $cond_lines > 1) { + $stat_real = "[...]\n$stat_real"; + } + + #print "line<$line> prevline<$prevline> indent<$indent> sindent<$sindent> check<$check> continuation<$continuation> s<$s> cond_lines<$cond_lines> stat_real<$stat_real> stat<$stat>\n"; + + if ($check && (($sindent % 8) != 0 || + ($sindent <= $indent && $s ne ''))) { + WARN("suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); + } + } + # Track the 'values' across context and added lines. my $opline = $line; $opline =~ s/^./ /; - my $curr_values = annotate_values($opline . "\n", $prev_values); + my ($curr_values, $curr_vars) = + annotate_values($opline . "\n", $prev_values); $curr_values = $prev_values . $curr_values; if ($dbg_values) { my $outline = $opline; $outline =~ s/\t/ /g; print "$linenr > .$outline\n"; print "$linenr > $curr_values\n"; + print "$linenr > $curr_vars\n"; } $prev_values = substr($curr_values, -1); @@ -1268,14 +1698,27 @@ sub process { if ($line=~/^[^\+]/) {next;} # TEST: allow direct testing of the type matcher. - if ($tst_type && $line =~ /^.$Declare$/) { - ERROR("TEST: is type $Declare\n" . $herecurr); + if ($dbg_type) { + if ($line =~ /^.\s*$Declare\s*$/) { + ERROR("TEST: is type\n" . $herecurr); + } elsif ($dbg_type > 1 && $line =~ /^.+($Declare)/) { + ERROR("TEST: is not type ($1 is)\n". $herecurr); + } + next; + } +# TEST: allow direct testing of the attribute matcher. + if ($dbg_attr) { + if ($line =~ /^.\s*$Modifier\s*$/) { + ERROR("TEST: is attr\n" . $herecurr); + } elsif ($dbg_attr > 1 && $line =~ /^.+($Modifier)/) { + ERROR("TEST: is not attr ($1 is)\n". $herecurr); + } next; } # check for initialisation to aggregates open brace on the next line - if ($prevline =~ /$Declare\s*$Ident\s*=\s*$/ && - $line =~ /^.\s*{/) { + if ($line =~ /^.\s*{/ && + $prevline =~ /(?:^|[^=])=\s*$/) { ERROR("that open brace { should be on the previous line\n" . $hereprev); } @@ -1284,7 +1727,7 @@ sub process { # # check for malformed paths in #include statements (uses RAW line) - if ($rawline =~ m{^.#\s*include\s+[<"](.*)[">]}) { + if ($rawline =~ m{^.\s*\#\s*include\s+[<"](.*)[">]}) { my $path = $1; if ($path =~ m{//}) { ERROR("malformed #include filename\n" . @@ -1300,28 +1743,48 @@ sub process { $line =~ s@//.*@@; $opline =~ s@//.*@@; -#EXPORT_SYMBOL should immediately follow its function closing }. - if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) || - ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { +# EXPORT_SYMBOL should immediately follow the thing it is exporting, consider +# the whole statement. +#print "APW <$lines[$realline_next - 1]>\n"; + if (defined $realline_next && + exists $lines[$realline_next - 1] && + !defined $suppress_export{$realline_next} && + ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*\((.*)\)/ || + $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { my $name = $1; - if (($prevline !~ /^}/) && - ($prevline !~ /^\+}/) && - ($prevline !~ /^ }/) && - ($prevline !~ /^.DECLARE_$Ident\(\Q$name\E\)/) && - ($prevline !~ /^.LIST_HEAD\(\Q$name\E\)/) && - ($prevline !~ /^.$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(/) && - ($prevline !~ /\b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=|\[)/)) { - WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr); + if ($stat !~ /(?: + \n.}\s*$| + ^.DEFINE_$Ident\(\Q$name\E\)| + ^.DECLARE_$Ident\(\Q$name\E\)| + ^.LIST_HEAD\(\Q$name\E\)| + ^.(?:$Storage\s+)?$Type\s*\(\s*\*\s*\Q$name\E\s*\)\s*\(| + \b\Q$name\E(?:\s+$Attribute)*\s*(?:;|=|\[|\() + )/x) { +#print "FOO A<$lines[$realline_next - 1]> stat<$stat> name<$name>\n"; + $suppress_export{$realline_next} = 2; + } else { + $suppress_export{$realline_next} = 1; } } + if (!defined $suppress_export{$linenr} && + $prevline =~ /^.\s*$/ && + ($line =~ /EXPORT_SYMBOL.*\((.*)\)/ || + $line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { +#print "FOO B <$lines[$linenr - 1]>\n"; + $suppress_export{$linenr} = 2; + } + if (defined $suppress_export{$linenr} && + $suppress_export{$linenr} == 2) { + WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr); + } # check for external initialisers. - if ($line =~ /^.$Type\s*$Ident\s*=\s*(0|NULL);/) { + if ($line =~ /^.$Type\s*$Ident\s*(?:\s+$Modifier)*\s*=\s*(0|NULL|false)\s*;/) { ERROR("do not initialise externals to 0 or NULL\n" . $herecurr); } # check for static initialisers. - if ($line =~ /\s*static\s.*=\s*(0|NULL);/) { + if ($line =~ /\bstatic\s.*=\s*(0|NULL|false)\s*;/) { ERROR("do not initialise statics to 0 or NULL\n" . $herecurr); } @@ -1329,27 +1792,47 @@ 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/) { WARN("do not add new typedefs\n" . $herecurr); } # * goes on variable not on type - if ($line =~ m{\($NonptrType(\*+)(?:\s+const)?\)}) { - ERROR("\"(foo$1)\" should be \"(foo $1)\"\n" . - $herecurr); - - } elsif ($line =~ m{\($NonptrType\s+(\*+)(?!\s+const)\s+\)}) { - 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); + + # 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{$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); + + # 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 /; - } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+(?:$Attribute|$Sparse))\s+[A-Za-z\d_]+}) { - ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" . - $herecurr); + #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() @@ -1388,8 +1871,8 @@ sub process { # function brace can't be on same line, except for #defines of do while, # or if closed on same line - if (($line=~/$Type\s*[A-Za-z\d_]+\(.*\).*\s{/) and - !($line=~/\#define.*do\s{/) and !($line=~/}/)) { + if (($line=~/$Type\s*$Ident\(.*\).*\s{/) and + !($line=~/\#\s*define.*do\s{/) and !($line=~/}/)) { ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr); } @@ -1399,6 +1882,19 @@ sub process { ERROR("open brace '{' following $1 go on the same line\n" . $hereprev); } +# check for spacing round square brackets; allowed: +# 1. with a type on the left -- int [] a; +# 2. at the beginning of a line for slice initialisers -- [0...10] = 5, +# 3. inside a curly brace -- = { [0...10] = 5 } + while ($line =~ /(.*?\s)\[/g) { + my ($where, $prefix) = ($-[1], $1); + if ($prefix !~ /$Type\s+$/ && + ($where != 0 || $prefix !~ /^.\s+$/) && + $prefix !~ /{\s+$/) { + ERROR("space prohibited before open square bracket '['\n" . $herecurr); + } + } + # check for spaces between functions and their parentheses. while ($line =~ /($Ident)\s+\(/g) { my $name = $1; @@ -1416,10 +1912,10 @@ sub process { # cpp #define statements have non-optional spaces, ie # if there is a space between the name and the open # parenthesis it is simply not a parameter group. - } elsif ($ctx_before =~ /^.\#\s*define\s*$/) { + } elsif ($ctx_before =~ /^.\s*\#\s*define\s*$/) { # cpp #elif statement condition may start with a ( - } elsif ($ctx =~ /^.\#\s*elif\s*$/) { + } elsif ($ctx =~ /^.\s*\#\s*elif\s*$/) { # If this whole things ends with a type its most # likely a typedef for a function. @@ -1435,7 +1931,8 @@ sub process { <<=|>>=|<=|>=|==|!=| \+=|-=|\*=|\/=|%=|\^=|\|=|&=| =>|->|<<|>>|<|>|=|!|~| - &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|% + &&|\|\||,|\^|\+\+|--|&|\||\+|-|\*|\/|%| + \?|: }x; my @elements = split(/($ops|;)/, $opline); my $off = 0; @@ -1470,7 +1967,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'; } @@ -1482,22 +1979,11 @@ sub process { my $ptr = substr($blank, 0, $off) . "^"; my $hereptr = "$hereline$ptr\n"; - # Classify operators into binary, unary, or - # definitions (* only) where they have more - # than one mode. + # Pull out the value of this operator. my $op_type = substr($curr_values, $off + 1, 1); - my $op_left = substr($curr_values, $off, 1); - my $is_unary; - if ($op_type eq 'T') { - $is_unary = 2; - } elsif ($op_left eq 'V') { - $is_unary = 0; - } else { - $is_unary = 1; - } - #if ($op eq '-' || $op eq '&' || $op eq '*') { - # print "UNARY: <$op_left$op_type $is_unary $a:$op:$c> <$ca:$op:$cc> <$unary_ctx>\n"; - #} + + # Get the full operator variant. + my $opv = $op . substr($curr_vars, $off, 1); # Ignore operators passed as parameters. if ($op_type ne 'V' && @@ -1516,8 +2002,10 @@ sub process { # // is a comment } elsif ($op eq '//') { - # -> should have no spaces - } elsif ($op eq '->') { + # No spaces for: + # -> + # : when part of a bitfield + } elsif ($op eq '->' || $opv eq ':B') { if ($ctx =~ /Wx.|.xW/) { ERROR("spaces prohibited around that '$op' $at\n" . $hereptr); } @@ -1529,18 +2017,19 @@ sub process { } # '*' as part of a type definition -- reported already. - } elsif ($op eq '*' && $is_unary == 2) { + } elsif ($opv eq '*_') { #warn "'*' is part of type\n"; # unary operators should have a space before and # none after. May be left adjacent to another # unary operator, or a cast } elsif ($op eq '!' || $op eq '~' || - ($is_unary && ($op eq '*' || $op eq '-' || $op eq '&'))) { + $opv eq '*U' || $opv eq '-U' || + $opv eq '&U' || $opv eq '&&U') { 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/) { @@ -1573,11 +2062,33 @@ sub process { $hereptr); } + # A colon needs no spaces before when it is + # terminating a case value or a label. + } elsif ($opv eq ':C' || $opv eq ':L') { + if ($ctx =~ /Wx./) { + ERROR("space prohibited before that '$op' $at\n" . $hereptr); + } + # All the others need spaces both sides. } elsif ($ctx !~ /[EWC]x[CWE]/) { + my $ok = 0; + # Ignore email addresses - if (!($op eq '<' && $cb =~ /$;\S+\@\S+>/) && - !($op eq '>' && $cb =~ /<\S+\@\S+$;/)) { + if (($op eq '<' && + $cc =~ /^\S+\@\S+>/) || + ($op eq '>' && + $ca =~ /<\S+\@\S+$/)) + { + $ok = 1; + } + + # Ignore ?: + if (($opv eq ':O' && $ca =~ /\?$/) || + ($op eq '?' && $cc =~ /^:/)) { + $ok = 1; + } + + if ($ok == 0) { ERROR("spaces required around that '$op' $at\n" . $hereptr); } } @@ -1625,13 +2136,14 @@ sub process { ERROR("space prohibited before that close square bracket ']'\n" . $herecurr); } -# check spacing on paretheses +# check spacing on parentheses if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ && $line !~ /for\s*\(\s+;/) { ERROR("space prohibited after that open parenthesis '('\n" . $herecurr); } if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ && - $line !~ /for\s*\(.*;\s+\)/) { + $line !~ /for\s*\(.*;\s+\)/ && + $line !~ /:\s+\)/) { ERROR("space prohibited before that close parenthesis ')'\n" . $herecurr); } @@ -1641,16 +2153,58 @@ sub process { WARN("labels should not be indented\n" . $herecurr); } +# Return is not a function. + if (defined($stat) && $stat =~ /^.\s*return(\s*)(\(.*);/s) { + my $spacing = $1; + my $value = $2; + + # Flatten any parentheses + $value =~ s/\)\(/\) \(/g; + while ($value =~ s/\[[^\{\}]*\]/1/ || + $value !~ /(?:$Ident|-?$Constant)\s* + $Compare\s* + (?:$Ident|-?$Constant)/x && + $value =~ s/\([^\(\)]*\)/1/) { + } + + if ($value =~ /^(?:$Ident|-?$Constant)$/) { + ERROR("return is not a function, parentheses are not required\n" . $herecurr); + + } elsif ($spacing !~ /\s+/) { + ERROR("space required before the open parenthesis '('\n" . $herecurr); + } + } + # Need a space before open parenthesis after if, while etc if ($line=~/\b(if|while|for|switch)\(/) { ERROR("space required before the open parenthesis '('\n" . $herecurr); } -# Check for illegal assignment in if conditional. - if ($line =~ /\bif\s*\(/) { +# Check for illegal assignment in if conditional -- and check for trailing +# statements after the conditional. + if ($line =~ /do\s*(?!{)/) { + my ($stat_next) = ctx_statement_block($line_nr_next, + $remain_next, $off_next); + $stat_next =~ s/\n./\n /g; + ##print "stat<$stat> stat_next<$stat_next>\n"; + + if ($stat_next =~ /^\s*while\b/) { + # If the statement carries leading newlines, + # then count those as offsets. + my ($whitespace) = + ($stat_next =~ /^((?:\s*\n[+-])*\s*)/s); + my $offset = + statement_rawlines($whitespace) - 1; + + $suppress_whiletrailers{$line_nr_next + + $offset} = 1; + } + } + if (!defined $suppress_whiletrailers{$linenr} && + $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); } @@ -1659,10 +2213,21 @@ sub process { substr($s, 0, length($c), ''); $s =~ s/\n.*//g; $s =~ s/$;//g; # Remove any comments - if (length($c) && $s !~ /^\s*({|;|)\s*\\*\s*$/ && - $c !~ /^.\#\s*if/) + if (length($c) && $s !~ /^\s*{?\s*\\*\s*$/ && + $c !~ /}\s*while\s*/) { - ERROR("trailing statements should be on next line\n" . $herecurr); + # Find out how long the conditional actually is. + my @newlines = ($c =~ /\n/gs); + my $cond_lines = 1 + $#newlines; + my $stat_real = ''; + + $stat_real = raw_line($linenr, $cond_lines) + . "\n" if ($cond_lines); + if (defined($stat_real) && $cond_lines > 1) { + $stat_real = "[...]\n$stat_real"; + } + + ERROR("trailing statements should be on next line\n" . $herecurr . $stat_real); } } @@ -1689,6 +2254,20 @@ 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(?: + (?:\s*$;*)(?:\s*{)?(?:\s*$;*)(?:\s*\\)?\s*$| + \s*return\s+ + )/xg) + { + ERROR("trailing statements should be on next line\n" . $herecurr); + } # Check for }else {, these must be at the same # indent level to be relevant to each other. @@ -1719,66 +2298,129 @@ sub process { # } #no spaces allowed after \ in define - if ($line=~/\#define.*\\\s$/) { + if ($line=~/\#\s*define.*\\\s$/) { WARN("Whitepspace after \\ makes next lines useless\n" . $herecurr); } #warn if is #included and is available (uses RAW line) - if ($tree && $rawline =~ m{^.\#\s*include\s*\}) { - my $checkfile = "$root/include/linux/$1.h"; - if (-f $checkfile && $1 ne 'irq') { - WARN("Use #include instead of \n" . - $herecurr); + if ($tree && $rawline =~ m{^.\s*\#\s*include\s*\}) { + my $file = "$1.h"; + my $checkfile = "include/linux/$file"; + if (-f "$root/$checkfile" && + $realfile ne $checkfile && + $1 ne 'irq') + { + if ($realfile =~ m{^arch/}) { + CHK("Consider using #include instead of \n" . $herecurr); + } else { + WARN("Use #include instead of \n" . $herecurr); + } } } # multi-statement macros should be enclosed in a do while loop, grab the # first statement and ensure its the whole macro if its not enclosed # in a known good container - if ($prevline =~ /\#define.*\\/ && - $prevline !~/(?:do\s+{|\(\{|\{)/ && - $line !~ /(?:do\s+{|\(\{|\{)/ && - $line !~ /^.\s*$Declare\s/) { - # Grab the first statement, if that is the entire macro - # its ok. This may start either on the #define line - # or the one below. + if ($realfile !~ m@/vmlinux.lds.h$@ && + $line =~ /^.\s*\#\s*define\s*$Ident(\()?/) { my $ln = $linenr; my $cnt = $realcnt; - my $off = 0; + my ($off, $dstat, $dcond, $rest); + my $ctx = ''; + + my $args = defined($1); - # If the macro starts on the define line start - # grabbing the statement after the identifier - $prevline =~ m{^(.#\s*define\s*$Ident(?:\([^\)]*\))?\s*)(.*)\\\s*$}; - ##print "1<$1> 2<$2>\n"; - if (defined $2 && $2 ne '') { - $off = length($1); - $ln--; - $cnt++; - while ($lines[$ln - 1] =~ /^-/) { - $ln--; - $cnt++; + # Find the end of the macro and limit our statement + # search to that. + while ($cnt > 0 && defined $lines[$ln - 1] && + $lines[$ln - 1] =~ /^(?:-|..*\\$)/) + { + $ctx .= $rawlines[$ln - 1] . "\n"; + $cnt-- if ($lines[$ln - 1] !~ /^-/); + $ln++; + } + $ctx .= $rawlines[$ln - 1]; + + ($dstat, $dcond, $ln, $cnt, $off) = + ctx_statement_block($linenr, $ln - $linenr + 1, 0); + #print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n"; + #print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n"; + + # Extract the remainder of the define (if any) and + # rip off surrounding spaces, and trailing \'s. + $rest = ''; + while ($off != 0 || ($cnt > 0 && $rest =~ /\\\s*$/)) { + #print "ADDING cnt<$cnt> $off <" . substr($lines[$ln - 1], $off) . "> rest<$rest>\n"; + if ($off != 0 || $lines[$ln - 1] !~ /^-/) { + $rest .= substr($lines[$ln - 1], $off) . "\n"; + $cnt--; } + $ln++; + $off = 0; } - my @ctx = ctx_statement($ln, $cnt, $off); - my $ctx_ln = $ln + $#ctx + 1; - my $ctx = join("\n", @ctx); + $rest =~ s/\\\n.//g; + $rest =~ s/^\s*//s; + $rest =~ s/\s*$//s; - # Pull in any empty extension lines. - while ($ctx =~ /\\$/ && - $lines[$ctx_ln - 1] =~ /^.\s*(?:\\)?$/) { - $ctx .= $lines[$ctx_ln - 1]; - $ctx_ln++; + # Clean up the original statement. + if ($args) { + substr($dstat, 0, length($dcond), ''); + } else { + $dstat =~ s/^.\s*\#\s*define\s+$Ident\s*//; + } + $dstat =~ s/$;//g; + $dstat =~ s/\\\n.//g; + $dstat =~ s/^\s*//s; + $dstat =~ s/\s*$//s; + + # Flatten any parentheses and braces + while ($dstat =~ s/\([^\(\)]*\)/1/ || + $dstat =~ s/\{[^\{\}]*\}/1/ || + $dstat =~ s/\[[^\{\}]*\]/1/) + { } - if ($ctx =~ /\\$/) { - if ($ctx =~ /;/) { + my $exceptions = qr{ + $Declare| + module_param_named| + MODULE_PARAM_DESC| + DECLARE_PER_CPU| + DEFINE_PER_CPU| + __typeof__\(| + union| + struct| + \.$Ident\s*=\s*| + ^\"|\"$ + }x; + #print "REST<$rest> dstat<$dstat>\n"; + if ($rest ne '') { + if ($rest !~ /while\s*\(/ && + $dstat !~ /$exceptions/) + { ERROR("Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n"); - } else { + } + + } elsif ($ctx !~ /;/) { + if ($dstat ne '' && + $dstat !~ /^(?:$Ident|-?$Constant)$/ && + $dstat !~ /$exceptions/ && + $dstat !~ /^\.$Ident\s*=/ && + $dstat =~ /$Operators/) + { ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n"); } } } +# 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) = @@ -1872,10 +2514,10 @@ sub process { } if ($level == 0 && $block =~ /^\s*\{/ && !$allowed) { my $herectx = $here . "\n";; - my $end = $linenr + statement_rawlines($block) - 1; + my $cnt = statement_rawlines($block); - for (my $ln = $linenr - 1; $ln < $end; $ln++) { - $herectx .= $rawlines[$ln] . "\n";; + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n";; } WARN("braces {} are not necessary for single statement blocks\n" . $herectx); @@ -1884,7 +2526,7 @@ sub process { # don't include deprecated include files (uses RAW line) for my $inc (@dep_includes) { - if ($rawline =~ m@\#\s*include\s*\<$inc>@) { + if ($rawline =~ m@^.\s*\#\s*include\s*\<$inc>@) { ERROR("Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n" . $herecurr); } } @@ -1908,7 +2550,7 @@ sub process { } # warn about #if 0 - if ($line =~ /^.#\s*if\s+0\b/) { + if ($line =~ /^.\s*\#\s*if\s+0\b/) { CHK("if this code is redundant consider removing it\n" . $herecurr); } @@ -1917,26 +2559,26 @@ sub process { if ($prevline =~ /\bif\s*\(([^\)]*)\)/) { my $expr = $1; if ($line =~ /\bkfree\(\Q$expr\E\);/) { - WARN("kfree(NULL) is safe this check is probabally not required\n" . $hereprev); + WARN("kfree(NULL) is safe this check is probably not required\n" . $hereprev); } } # check for needless usb_free_urb() checks if ($prevline =~ /\bif\s*\(([^\)]*)\)/) { my $expr = $1; if ($line =~ /\busb_free_urb\(\Q$expr\E\);/) { - WARN("usb_free_urb(NULL) is safe this check is probabally not required\n" . $hereprev); + WARN("usb_free_urb(NULL) is safe this check is probably not required\n" . $hereprev); } } # warn about #ifdefs in C files -# if ($line =~ /^.#\s*if(|n)def/ && ($realfile =~ /\.c$/)) { +# if ($line =~ /^.\s*\#\s*if(|n)def/ && ($realfile =~ /\.c$/)) { # print "#ifdef in C files should be avoided\n"; # print "$herecurr"; # $clean = 0; # } # warn about spacing in #ifdefs - if ($line =~ /^.#\s*(ifdef|ifndef|elif)\s\s+/) { + if ($line =~ /^.\s*\#\s*(ifdef|ifndef|elif)\s\s+/) { ERROR("exactly one space required after that #$1\n" . $herecurr); } @@ -1955,10 +2597,15 @@ sub process { } } # check of hardware specific defines - if ($line =~ m@^.#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { + if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { CHK("architecture specific defines should be avoided\n" . $herecurr); } +# Check that the storage class is at the beginning of a declaration + if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { + WARN("storage class should be at the beginning of the declaration\n" . $herecurr) + } + # check the location of the inline attribute, that it is between # storage class and type. if ($line =~ /\b$Type\s+$Inline\b/ || @@ -1971,23 +2618,36 @@ sub process { WARN("plain inline is preferred over $1\n" . $herecurr); } +# check for sizeof(&) + if ($line =~ /\bsizeof\s*\(\s*\&/) { + WARN("sizeof(& should be avoided\n" . $herecurr); + } + # check for new externs in .c files. if ($realfile =~ /\.c$/ && defined $stat && - $stat =~ /^.(?:extern\s+)?$Type\s+$Ident(\s*)\(/s) + $stat =~ /^.\s*(?:extern\s+)?$Type\s+($Ident)(\s*)\(/s) { - my $paren_space = $1; + my $function_name = $1; + my $paren_space = $2; my $s = $stat; if (defined $cond) { substr($s, 0, length($cond), ''); } - if ($s =~ /^\s*;/) { + if ($s =~ /^\s*;/ && + $function_name ne 'uninitialized_var') + { WARN("externs should be avoided in .c files\n" . $herecurr); } if ($paren_space =~ /\n/) { WARN("arguments for function declarations should follow identifier\n" . $herecurr); } + + } elsif ($realfile =~ /\.c$/ && defined $stat && + $stat =~ /^.\s*extern\s+/) + { + WARN("externs should be avoided in .c files\n" . $herecurr); } # checks for new __setup's @@ -2016,23 +2676,101 @@ sub process { # check for semaphores used as mutexes if ($line =~ /^.\s*init_MUTEX_LOCKED\s*\(/) { WARN("consider using a completion\n" . $herecurr); + } # recommend strict_strto* over simple_strto* if ($line =~ /\bsimple_(strto.*?)\s*\(/) { WARN("consider using strict_$1 in preference to simple_$1\n" . $herecurr); } +# check for __initcall(), use device_initcall() explicitly please + if ($line =~ /^.\s*__initcall\s*\(/) { + WARN("please use device_initcall() instead of __initcall()\n" . $herecurr); + } +# check for various ops structs, ensure they are const. + my $struct_ops = qr{acpi_dock_ops| + address_space_operations| + backlight_ops| + block_device_operations| + dentry_operations| + dev_pm_ops| + dma_map_ops| + extent_io_ops| + file_lock_operations| + file_operations| + hv_ops| + ide_dma_ops| + intel_dvo_dev_ops| + item_operations| + iwl_ops| + kgdb_arch| + kgdb_io| + kset_uevent_ops| + lock_manager_operations| + microcode_ops| + mtrr_ops| + neigh_ops| + nlmsvc_binding| + pci_raw_ops| + pipe_buf_operations| + platform_hibernation_ops| + platform_suspend_ops| + proto_ops| + rpc_pipe_ops| + seq_operations| + snd_ac97_build_ops| + soc_pcmcia_socket_ops| + stacktrace_ops| + sysfs_ops| + tty_operations| + usb_mon_operations| + wd_ops}x; + if ($line !~ /\bconst\b/ && + $line =~ /\bstruct\s+($struct_ops)\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 if ($line =~ /\bNR_CPUS\b/ && - $line !~ /^.#\s*if\b.*\bNR_CPUS\b/ && - $line !~ /^.#\s*define\b.*\bNR_CPUS\b/ && + $line !~ /^.\s*\s*#\s*if\b.*\bNR_CPUS\b/ && + $line !~ /^.\s*\s*#\s*define\b.*\bNR_CPUS\b/ && $line !~ /^.\s*$Declare\s.*\[[^\]]*NR_CPUS[^\]]*\]/ && $line !~ /\[[^\]]*\.\.\.[^\]]*NR_CPUS[^\]]*\]/ && $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/) { WARN("usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); } + +# check for %L{u,d,i} in strings + my $string; + while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { + $string = substr($rawline, $-[1], $+[1] - $-[1]); + $string =~ s/%%/__/g; + if ($string =~ /(?mutex.\n" . $herecurr); + } + } } # If we have no input at all, then there is nothing to report on