X-Git-Url: http://pilppa.org/gitweb/gitweb.cgi?a=blobdiff_plain;f=scripts%2Fcheckpatch.pl;h=dae7d30dca0f1886b450311ee0e3c959fc3d40da;hb=cd4765efdd816ac14075fc7d5adf489502e75e1e;hp=e216d49624b76afccae5ef6981fe270dc9caa15e;hpb=c8d8170feb824875baf68f8aaecb181a6500ce81;p=linux-2.6-omap-h63xx.git diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e216d49624b..dae7d30dca0 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1,14 +1,15 @@ #!/usr/bin/perl -w # (c) 2001, Dave Jones. (the file handling bit) -# (c) 2005, Joel Scohpp (the ugly bit) +# (c) 2005, Joel Schopp (the ugly bit) # (c) 2007, Andy Whitcroft (new conditions, test suite, etc) # Licensed under the terms of the GNU GPL License version 2 use strict; my $P = $0; +$P =~ s@.*/@@g; -my $V = '0.01'; +my $V = '0.09'; use Getopt::Long qw(:config no_auto_abbrev); @@ -16,17 +17,19 @@ my $quiet = 0; my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; +my $tst_type = 0; GetOptions( 'q|quiet' => \$quiet, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!' => \$chk_patch, + 'test-type!' => \$tst_type, ) or exit; my $exit = 0; if ($#ARGV < 0) { - print "usage: patchstylecheckemail.pl [options] patchfile\n"; + print "usage: $P [options] patchfile\n"; print "version: $V\n"; print "options: -q => quiet\n"; print " --no-tree => run without a kernel tree\n"; @@ -38,30 +41,34 @@ if ($tree && !top_of_kernel_tree()) { exit(2); } -my @deprecated = (); +my @dep_includes = (); +my @dep_functions = (); my $removal = 'Documentation/feature-removal-schedule.txt'; if ($tree && -f $removal) { open(REMOVE, "<$removal") || die "$P: $removal: open failed - $!\n"; while () { - if (/^Files:\s+(.*\S)/) { - for my $file (split(/[, ]+/, $1)) { - if ($file =~ m@include/(.*)@) { - push(@deprecated, $1); + if (/^Check:\s+(.*\S)/) { + for my $entry (split(/[, ]+/, $1)) { + if ($entry =~ m@include/(.*)@) { + push(@dep_includes, $1); + + } elsif ($entry !~ m@/@) { + push(@dep_functions, $entry); } } } } } -my @lines = (); +my @rawlines = (); while (<>) { chomp; - push(@lines, $_); + push(@rawlines, $_); if (eof(ARGV)) { - if (!process($ARGV, @lines)) { + if (!process($ARGV, @rawlines)) { $exit = 1; } - @lines = (); + @rawlines = (); } } @@ -99,6 +106,155 @@ sub expand_tabs { return $res; } +sub line_stats { + my ($line) = @_; + + # Drop the diff line leader and expand tabs + $line =~ s/^.//; + $line = expand_tabs($line); + + # Pick the indent from the front of the line. + my ($white) = ($line =~ /^(\s*)/); + + return (length($line), length($white)); +} + +sub sanitise_line { + my ($line) = @_; + + my $res = ''; + my $l = ''; + + my $quote = ''; + + foreach my $c (split(//, $line)) { + if ($l ne "\\" && ($c eq "'" || $c eq '"')) { + if ($quote eq '') { + $quote = $c; + $res .= $c; + $l = $c; + next; + } elsif ($quote eq $c) { + $quote = ''; + } + } + if ($quote && $c ne "\t") { + $res .= "X"; + } else { + $res .= $c; + } + + $l = $c; + } + + return $res; +} + +sub ctx_block_get { + my ($linenr, $remain, $outer, $open, $close, $off) = @_; + my $line; + my $start = $linenr - 1; + my $blk = ''; + my @o; + my @c; + my @res = (); + + my $level = 0; + for ($line = $start; $remain > 0; $line++) { + next if ($rawlines[$line] =~ /^-/); + $remain--; + + $blk .= $rawlines[$line]; + foreach my $c (split(//, $rawlines[$line])) { + ##print "C<$c>L<$level><$open$close>O<$off>\n"; + if ($off > 0) { + $off--; + next; + } + + if ($c eq $close && $level > 0) { + $level--; + last if ($level == 0); + } elsif ($c eq $open) { + $level++; + } + } + + if (!$outer || $level <= 1) { + push(@res, $rawlines[$line]); + } + + last if ($level == 0); + } + + return ($level, @res); +} +sub ctx_block_outer { + my ($linenr, $remain) = @_; + + my ($level, @r) = ctx_block_get($linenr, $remain, 1, '{', '}', 0); + return @r; +} +sub ctx_block { + my ($linenr, $remain) = @_; + + my ($level, @r) = ctx_block_get($linenr, $remain, 0, '{', '}', 0); + return @r; +} +sub ctx_statement { + my ($linenr, $remain, $off) = @_; + + my ($level, @r) = ctx_block_get($linenr, $remain, 0, '(', ')', $off); + return @r; +} +sub ctx_block_level { + my ($linenr, $remain) = @_; + + return ctx_block_get($linenr, $remain, 0, '{', '}', 0); +} + +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*$@); + return $current_comment if (defined $current_comment); + + # Look through the context and try and figure out if there is a + # comment. + my $in_comment = 0; + $current_comment = ''; + for (my $linenr = $first_line; $linenr < $end_line; $linenr++) { + my $line = $rawlines[$linenr - 1]; + #warn " $line\n"; + if ($linenr == $first_line and $line =~ m@^.\s*\*@) { + $in_comment = 1; + } + if ($line =~ m@/\*@) { + $in_comment = 1; + } + if (!$in_comment && $current_comment ne '') { + $current_comment = ''; + } + $current_comment .= $line . "\n" if ($in_comment); + if ($line =~ m@\*/@) { + $in_comment = 0; + } + } + + chomp($current_comment); + return($current_comment); +} +sub ctx_has_comment { + my ($first_line, $end_line) = @_; + my $cmt = ctx_locate_comment($first_line, $end_line); + + ##print "LINE: $rawlines[$end_line - 1 ]\n"; + ##print "CMMT: $cmt\n"; + + return ($cmt ne ''); +} + sub cat_vet { my ($vet) = @_; @@ -108,6 +264,26 @@ sub cat_vet { return $vet; } +my @report = (); +sub report { + push(@report, $_[0]); +} +sub report_dump { + @report; +} +sub ERROR { + report("ERROR: $_[0]\n"); + our $clean = 0; +} +sub WARN { + report("WARNING: $_[0]\n"); + our $clean = 0; +} +sub CHK { + report("CHECK: $_[0]\n"); + our $clean = 0; +} + sub process { my $filename = shift; my @lines = @_; @@ -116,12 +292,12 @@ sub process { my $prevline=""; my $stashline=""; - my $lineforcounting=''; + my $length; my $indent; my $previndent=0; my $stashindent=0; - my $clean = 1; + our $clean = 1; my $signoff = 0; my $is_patch = 0; @@ -133,19 +309,80 @@ sub process { my $in_comment = 0; my $first_line = 0; + my $Ident = qr{[A-Za-z\d_]+}; + my $Storage = qr{extern|static}; + my $Sparse = qr{__user|__kernel|__force|__iomem|__must_check|__init_refok}; + my $NonptrType = qr{ + \b + (?:const\s+)? + (?:unsigned\s+)? + (?: + void| + char| + short| + int| + long| + unsigned| + float| + double| + bool| + long\s+int| + long\s+long| + long\s+long\s+int| + u8|u16|u32|u64| + s8|s16|s32|s64| + struct\s+$Ident| + union\s+$Ident| + enum\s+$Ident| + ${Ident}_t + ) + (?:\s+$Sparse)* + \b + }x; + my $Type = qr{ + \b$NonptrType\b + (?:\s*\*+\s*const|\s*\*+|(?:\s*\[\s*\])+)? + (?:\s+$Sparse)* + }x; + my $Declare = qr{(?:$Storage\s+)?$Type}; + my $Attribute = qr{const|__read_mostly|__init|__initdata|__meminit}; + + my $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; + my $Lval = qr{$Ident(?:$Member)*}; + + # Pre-scan the patch looking for any __setup documentation. + my @setup_docs = (); + my $setup_docs = 0; + foreach my $line (@lines) { + if ($line=~/^\+\+\+\s+(\S+)/) { + $setup_docs = 0; + if ($1 =~ m@Documentation/kernel-parameters.txt$@) { + $setup_docs = 1; + } + next; + } + + if ($setup_docs && $line =~ /^\+/) { + push(@setup_docs, $line); + } + } + foreach my $line (@lines) { $linenr++; + my $rawline = $line; + #extract the filename as it passes if ($line=~/^\+\+\+\s+(\S+)/) { $realfile=$1; + $realfile =~ s@^[^/]*/@@; $in_comment = 0; next; } #extract the line range in the file after the patch is applied if ($line=~/^\@\@ -\d+,\d+ \+(\d+)(,(\d+))? \@\@/) { $is_patch = 1; - $first_line = 1; + $first_line = $linenr + 1; $in_comment = 0; $realline=$1-1; if (defined $2) { @@ -156,8 +393,10 @@ sub process { next; } -#track the line number as we move through the hunk - if ($line=~/^[ \+]/) { +# track the line number as we move through the hunk, note that +# new versions of GNU diff omit the leading space on completely +# blank context lines so we need to count that too. + if ($line =~ /^( |\+|$)/) { $realline++; $realcnt-- if ($realcnt != 0); @@ -168,7 +407,7 @@ sub process { # Guestimate if this is a continuing comment. If this # is the start of a diff block and this line starts # ' *' then it is very likely a comment. - if ($first_line and $line =~ m@^.\s*\*@) { + if ($linenr == $first_line and $line =~ m@^.\s*\*@) { $in_comment = 1; } if ($line =~ m@/\*@) { @@ -178,61 +417,73 @@ sub process { $in_comment = 0; } - $lineforcounting = $line; - $lineforcounting =~ s/^\+//; - $lineforcounting = expand_tabs($lineforcounting); - - my ($white) = ($lineforcounting =~ /^(\s*)/); - $indent = length($white); + # Measure the line length and indent. + ($length, $indent) = line_stats($line); # Track the previous line. ($prevline, $stashline) = ($stashline, $line); ($previndent, $stashindent) = ($stashindent, $indent); - $first_line = 0; + } elsif ($realcnt == 1) { + $realcnt--; } #make up the handle for any error we report on this line - $here = "PATCH: $ARGV:$linenr:"; - $here .= "\nFILE: $realfile:$realline:" if ($realcnt != 0); + $here = "#$linenr: "; + $here .= "FILE: $realfile:$realline:" if ($realcnt != 0); - my $herecurr = "$here\n$line\n\n"; - my $hereprev = "$here\n$prevline\n$line\n\n"; + my $hereline = "$here\n$line\n"; + my $herecurr = "$here\n$line\n"; + my $hereprev = "$here\n$prevline\n$line\n"; #check the patch for a signoff: - if ($line =~ /^\s*Signed-off-by:\s/) { + if ($line =~ /^\s*signed-off-by:/i) { + # This is a signoff, if ugly, so do not double report. $signoff++; - - } elsif ($line =~ /^\s*signed-off-by:/i) { if (!($line =~ /^\s*Signed-off-by:/)) { - print "use Signed-off-by:\n"; - print "$herecurr"; - $clean = 0; + WARN("Signed-off-by: is the preferred form\n" . + $herecurr); } if ($line =~ /^\s*signed-off-by:\S/i) { - print "need space after Signed-off-by:\n"; - print "$herecurr"; - $clean = 0; + WARN("need space after Signed-off-by:\n" . + $herecurr); } } -#ignore lines not being added - if ($line=~/^[^\+]/) {next;} +# Check for wrappage within a valid hunk of the file + if ($realcnt != 0 && $line !~ m{^(?:\+|-| |$)}) { + ERROR("patch seems to be corrupt (line wrapped?)\n" . + $herecurr); + } + +# UTF-8 regex found at http://www.w3.org/International/questions/qa-forms-utf-8.en.php + if (($realfile =~ /^$/ || $line =~ /^\+/) && + !($line =~ m/^( + [\x09\x0A\x0D\x20-\x7E] # ASCII + | [\xC2-\xDF][\x80-\xBF] # non-overlong 2-byte + | \xE0[\xA0-\xBF][\x80-\xBF] # excluding overlongs + | [\xE1-\xEC\xEE\xEF][\x80-\xBF]{2} # straight 3-byte + | \xED[\x80-\x9F][\x80-\xBF] # excluding surrogates + | \xF0[\x90-\xBF][\x80-\xBF]{2} # planes 1-3 + | [\xF1-\xF3][\x80-\xBF]{3} # planes 4-15 + | \xF4[\x80-\x8F][\x80-\xBF]{2} # plane 16 + )*$/x )) { + ERROR("Invalid UTF-8\n" . $herecurr); + } + +#ignore lines being removed + if ($line=~/^-/) {next;} -# check we are in a valid source file *.[hcsS] if not then ignore this hunk - next if ($realfile !~ /\.[hcsS]$/); +# check we are in a valid source file if not then ignore this hunk + next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); #trailing whitespace - if ($line=~/\S\s+$/) { - my $herevet = "$here\n" . cat_vet($line) . "\n\n"; - print "trailing whitespace\n"; - print "$herevet"; - $clean = 0; + if ($line =~ /^\+.*\S\s+$/ || $line =~ /^\+\s+$/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("trailing whitespace\n" . $herevet); } #80 column limit - if (!($prevline=~/\/\*\*/) && length($lineforcounting) > 80) { - print "line over 80 characters\n"; - print "$herecurr"; - $clean = 0; + if ($line =~ /^\+/ && !($prevline=~/\/\*\*/) && $length > 80) { + WARN("line over 80 characters\n" . $herecurr); } # check we are in a valid source file *.[hc] if not then ignore this hunk @@ -241,62 +492,154 @@ sub process { # at the beginning of a line any tabs must come first and anything # more than 8 must use tabs. if ($line=~/^\+\s* \t\s*\S/ or $line=~/^\+\s* \s*/) { - my $herevet = "$here\n" . cat_vet($line) . "\n\n"; - print "use tabs not spaces\n"; - print "$herevet"; - $clean = 0; + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("use tabs not spaces\n" . $herevet); + } + +# Remove comments from the line before processing. + my $comment_edge = ($line =~ s@/\*.*\*/@@g) + + ($line =~ s@/\*.*@@) + + ($line =~ s@^(.).*\*/@$1@); + +# The rest of our checks refer specifically to C style +# only apply those _outside_ comments. Only skip +# lines in the middle of comments. + next if (!$comment_edge && $in_comment); + +# Standardise the strings and chars within the input to simplify matching. + $line = sanitise_line($line); + +# +# Checks which may be anchored in the context. +# + +# Check for switch () and associated case and default +# statements should be at the same indent. + if ($line=~/\bswitch\s*\(.*\)/) { + my $err = ''; + my $sep = ''; + my @ctx = ctx_block_outer($linenr, $realcnt); + shift(@ctx); + for my $ctx (@ctx) { + my ($clen, $cindent) = line_stats($ctx); + if ($ctx =~ /^\+\s*(case\s+|default:)/ && + $indent != $cindent) { + $err .= "$sep$ctx\n"; + $sep = ''; + } else { + $sep = "[...]\n"; + } + } + if ($err ne '') { + ERROR("switch and case should be at the same indent\n$hereline\n$err\n"); + } } - # - # The rest of our checks refer specifically to C style - # only apply those _outside_ comments. - # - next if ($in_comment); +# 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 !~ /^.#/) { + my @ctx = ctx_statement($linenr, $realcnt, 0); + my $ctx_ln = $linenr + $#ctx + 1; + my $ctx_cnt = $realcnt - $#ctx - 1; + my $ctx = join("\n", @ctx); + + while ($ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^-/) { + $ctx_ln++; + $ctx_cnt--; + } + ##warn "line<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>"; -# no C99 // comments - if ($line =~ m@//@ and !($line =~ m@\".*//.*\"@)) { - print "do not use C99 // comments\n"; - print "$herecurr"; - $clean = 0; + if ($ctx !~ /{\s*/ && $ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { + ERROR("That open brace { should be on the previous line\n" . + "$here\n$ctx\n$lines[$ctx_ln - 1]"); + } } - # Remove comments from the line before processing. - $line =~ s@/\*.*\*/@@g; - $line =~ s@/\*.*@@; - $line =~ s@.*\*/@@; +#ignore lines not being added + if ($line=~/^[^\+]/) {next;} + +# TEST: allow direct testing of the type matcher. + if ($tst_type && $line =~ /^.$Declare$/) { + ERROR("TEST: is type $Declare\n" . $herecurr); + next; + } + +# check for initialisation to aggregates open brace on the next line + if ($prevline =~ /$Declare\s*$Ident\s*=\s*$/ && + $line =~ /^.\s*{/) { + ERROR("That open brace { should be on the previous line\n" . $hereprev); + } + +# +# Checks which are anchored on the added line. +# + +# check for malformed paths in #include statements (uses RAW line) + if ($rawline =~ m{^.#\s*include\s+[<"](.*)[">]}) { + my $path = $1; + if ($path =~ m{//}) { + ERROR("malformed #include filename\n" . + $herecurr); + } + # Sanitise this special form of string. + $path = 'X' x length($path); + $line =~ s{\<.*\>}{<$path>}; + } + +# no C99 // comments + if ($line =~ m{//}) { + ERROR("do not use C99 // comments\n" . $herecurr); + } + # Remove C99 comments. $line =~ s@//.*@@; #EXPORT_SYMBOL should immediately follow its function closing }. - if (($line =~ /EXPORT_SYMBOL.*\(.*\)/) || - ($line =~ /EXPORT_UNUSED_SYMBOL.*\(.*\)/)) { + if (($line =~ /EXPORT_SYMBOL.*\((.*)\)/) || + ($line =~ /EXPORT_UNUSED_SYMBOL.*\((.*)\)/)) { + my $name = $1; if (($prevline !~ /^}/) && ($prevline !~ /^\+}/) && - ($prevline !~ /^ }/)) { - print "EXPORT_SYMBOL(func); should immediately follow its function\n"; - print "$herecurr"; - $clean = 0; + ($prevline !~ /^ }/) && + ($prevline !~ /\b\Q$name\E(?:\s+$Attribute)?\s*(?:;|=)/)) { + WARN("EXPORT_SYMBOL(foo); should immediately follow its function/variable\n" . $herecurr); } } - # check for static initialisers. - if ($line=~/\s*static\s.*=\s+(0|NULL);/) { - print "do not initialise statics to 0 or NULL\n"; - print "$herecurr"; - $clean = 0; +# check for external initialisers. + if ($line =~ /^.$Type\s*$Ident\s*=\s*(0|NULL);/) { + ERROR("do not initialise externals to 0 or NULL\n" . + $herecurr); + } +# check for static initialisers. + if ($line =~ /\s*static\s.*=\s*(0|NULL);/) { + ERROR("do not initialise statics to 0 or NULL\n" . + $herecurr); } - # check for new typedefs. - if ($line=~/\s*typedef\s/) { - print "do not add new typedefs\n"; - print "$herecurr"; - $clean = 0; +# 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 !~ /\b__bitwise(?:__|)\b/) { + WARN("do not add new typedefs\n" . $herecurr); } # * goes on variable not on type - if ($line=~/[A-Za-z\d_]+\* [A-Za-z\d_]+/) { - print "\"foo* bar\" should be \"foo *bar\"\n"; - print "$herecurr"; - $clean = 0; + 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); + + } elsif ($line =~ m{$NonptrType(\*+)(?:\s+$Attribute)?\s+[A-Za-z\d_]+}) { + ERROR("\"foo$1 bar\" should be \"foo $1bar\"\n" . + $herecurr); + + } elsif ($line =~ m{$NonptrType\s+(\*+)(?!\s+$Attribute)\s+[A-Za-z\d_]+}) { + ERROR("\"foo $1 bar\" should be \"foo $1bar\"\n" . + $herecurr); } # # no BUG() or BUG_ON() @@ -306,101 +649,130 @@ sub process { # $clean = 0; # } -# printk should use KERN_* levels - if ($line =~ /\bprintk\((?!KERN_)/) { - print "printk() should include KERN_ facility level\n"; - print "$herecurr"; - $clean = 0; +# printk should use KERN_* levels. Note that follow on printk's on the +# same line do not need a level, so we use the current block context +# to try and find and validate the current printk. In summary the current +# printk includes all preceeding printk's which have no newline on the end. +# we assume the first bad printk is the one to report. + if ($line =~ /\bprintk\((?!KERN_)\s*"/) { + my $ok = 0; + for (my $ln = $linenr - 1; $ln >= $first_line; $ln--) { + #print "CHECK<$lines[$ln - 1]\n"; + # we have a preceeding printk if it ends + # with "\n" ignore it, else it is to blame + if ($lines[$ln - 1] =~ m{\bprintk\(}) { + if ($rawlines[$ln - 1] !~ m{\\n"}) { + $ok = 1; + } + last; + } + } + if ($ok == 0) { + WARN("printk() should include KERN_ facility level\n" . $herecurr); + } } -#function brace can't be on same line, except for #defines of do while, or if closed on same line - if (($line=~/[A-Za-z\d_]+\**\s+\**[A-Za-z\d_]+\(.*\).* {/) and +# 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_]+\(.*\).* {/) and !($line=~/\#define.*do\s{/) and !($line=~/}/)) { - print "braces following function declarations go on the next line\n"; - print "$herecurr"; - $clean = 0; + ERROR("open brace '{' following function declarations go on the next line\n" . $herecurr); + } + +# check for spaces between functions and their parentheses. + if ($line =~ /($Ident)\s+\(/ && + $1 !~ /^(?:if|for|while|switch|return|volatile|__volatile__|__attribute__|format|__extension__|Copyright)$/ && + $line !~ /$Type\s+\(/ && $line !~ /^.\#\s*define\b/) { + WARN("no space between function name and open parenthesis '('\n" . $herecurr); } +# Check operator spacing. + # Note we expand the line with the leading + as the real + # line will be displayed with the leading + and the tabs + # will therefore also expand that way. my $opline = $line; - $opline =~ s/^.//; + $opline = expand_tabs($opline); + $opline =~ s/^./ /; if (!($line=~/\#\s*include/)) { - # Check operator spacing. - my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline); + my @elements = split(/(<<=|>>=|<=|>=|==|!=|\+=|-=|\*=|\/=|%=|\^=|\|=|&=|=>|->|<<|>>|<|>|=|!|~|&&|\|\||,|\^|\+\+|--|;|&|\||\+|-|\*|\/\/|\/)/, $opline); + my $off = 0; for (my $n = 0; $n < $#elements; $n += 2) { - # $wN says we have white-space before or after - # $sN says we have a separator before or after - # $oN says we have another operator before or after - my $w1 = $elements[$n] =~ /\s$/; - my $s1 = $elements[$n] =~ /(\[|\(|\s)$/; - my $o1 = $elements[$n] eq ''; + $off += length($elements[$n]); + + my $a = ''; + $a = 'V' if ($elements[$n] ne ''); + $a = 'W' if ($elements[$n] =~ /\s$/); + $a = 'B' if ($elements[$n] =~ /(\[|\()$/); + $a = 'O' if ($elements[$n] eq ''); + $a = 'E' if ($elements[$n] eq '' && $n == 0); + my $op = $elements[$n + 1]; - my $w2 = 1; - my $s2 = 1; - my $o2 = 0; - # If we have something after the operator handle it. + + my $c = ''; if (defined $elements[$n + 2]) { - $w2 = $elements[$n + 2] =~ /^\s/; - $s2 = $elements[$n + 2] =~ /^(\s|\)|\]|;)/; - $o2 = $elements[$n + 2] eq ''; + $c = 'V' if ($elements[$n + 2] ne ''); + $c = 'W' if ($elements[$n + 2] =~ /^\s/); + $c = 'B' if ($elements[$n + 2] =~ /^(\)|\]|;)/); + $c = 'O' if ($elements[$n + 2] eq ''); + $c = 'E' if ($elements[$n + 2] =~ /\s*\\$/); + } else { + $c = 'E'; } - # Generate the context. - my $at = "here: "; - for (my $m = $n; $m >= 0; $m--) { - if ($elements[$m] ne '') { - $at .= $elements[$m]; - last; - } - } - $at .= $op; - for (my $m = $n + 2; defined $elements[$m]; $m++) { - if ($elements[$m] ne '') { - $at .= $elements[$m]; - last; - } + # Pick up the preceeding and succeeding characters. + my $ca = substr($opline, 0, $off); + my $cc = ''; + if (length($opline) >= ($off + length($elements[$n + 1]))) { + $cc = substr($opline, $off + length($elements[$n + 1])); } + my $cb = "$ca$;$cc"; + + my $ctx = "${a}x${c}"; + + my $at = "(ctx:$ctx)"; + + my $ptr = (" " x $off) . "^"; + my $hereptr = "$hereline$ptr\n"; ##print "<$s1:$op:$s2> <$elements[$n]:$elements[$n + 1]:$elements[$n + 2]>\n"; - # Skip things apparently in quotes. - next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/); - # We need ; as an operator. // is a comment. - if ($op eq ';' or $op eq '//') { + # ; should have either the end of line or a space or \ after it + if ($op eq ';') { + if ($ctx !~ /.x[WEB]/ && $cc !~ /^\\/ && + $cc !~ /^;/) { + ERROR("need space after that '$op' $at\n" . $hereptr); + } + + # // is a comment + } elsif ($op eq '//') { # -> should have no spaces } elsif ($op eq '->') { - if ($s1 or $s2) { - print "no spaces around that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + if ($ctx =~ /Wx.|.xW/) { + ERROR("no spaces around that '$op' $at\n" . $hereptr); } # , must have a space on the right. } elsif ($op eq ',') { - if (!$s2) { - print "need space after that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + if ($ctx !~ /.xW|.xE/ && $cc !~ /^}/) { + ERROR("need space after that '$op' $at\n" . $hereptr); } # unary ! and unary ~ are allowed no space on the right } elsif ($op eq '!' or $op eq '~') { - if (!$s1 && !$o1) { - print "need space before that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + if ($ctx !~ /[WOEB]x./) { + ERROR("need space before that '$op' $at\n" . $hereptr); } - if ($s2) { - print "no space after that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + if ($ctx =~ /.xW/) { + ERROR("no space after that '$op' $at\n" . $hereptr); } # unary ++ and unary -- are allowed no space on one side. } elsif ($op eq '++' or $op eq '--') { - if (($s1 && $s2) || ((!$s1 && !$o1) && (!$s2 && !$o2))) { - print "need space one side of that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + if ($ctx !~ /[WOB]x[^W]/ && $ctx !~ /[^W]x[WOBE]/) { + ERROR("need space one side of that '$op' $at\n" . $hereptr); + } + if ($ctx =~ /Wx./ && $cc =~ /^;/) { + ERROR("no space before that '$op' $at\n" . $hereptr); } # & is both unary and binary @@ -415,84 +787,113 @@ sub process { # # - is the same # - # * is the same only adding: + } elsif ($op eq '&' or $op eq '-') { + if ($ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]/) { + ERROR("need space before that '$op' $at\n" . $hereptr); + } + + # * is the same as & only adding: # type: # (foo *) # (foo **) # - } elsif ($op eq '&' or $op eq '-' or $op eq '*') { - if ($w2 and !$w1) { - print "need space before that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + } elsif ($op eq '*') { + if ($ca !~ /$Type$/ && $cb !~ /(\*$;|$;\*)/ && + $ctx !~ /VxV|[EW]x[WE]|[EWB]x[VO]|OxV|WxB|BxB/) { + ERROR("need space before that '$op' $at\n" . $hereptr); } # << and >> may either have or not have spaces both sides } elsif ($op eq '<<' or $op eq '>>' or $op eq '+' or $op eq '/' or $op eq '^' or $op eq '|') { - if ($s1 != $s2) { - print "need consistent spacing around '$op' $at\n"; - print "$herecurr"; - $clean = 0; + if ($ctx !~ /VxV|WxW|VxE|WxE/) { + ERROR("need consistent spacing around '$op' $at\n" . + $hereptr); } # All the others need spaces both sides. - } elsif (!$s1 or !$s2) { - print "need spaces around that '$op' $at\n"; - print "$herecurr"; - $clean = 0; + } elsif ($ctx !~ /[EW]x[WE]/) { + # Ignore email addresses + if (!($op eq '<' && $cb =~ /$;\S+\@\S+>/) && + !($op eq '>' && $cb =~ /<\S+\@\S+$;/)) { + ERROR("need spaces around that '$op' $at\n" . $hereptr); + } } + $off += length($elements[$n + 1]); } } +# check for multiple assignments + if ($line =~ /^.\s*$Lval\s*=\s*$Lval\s*=(?!=)/) { + WARN("multiple assignments should be avoided\n" . $herecurr); + } + +## # check for multiple declarations, allowing for a function declaration +## # continuation. +## if ($line =~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Ident.*/ && +## $line !~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Type\s*$Ident.*/) { +## +## # Remove any bracketed sections to ensure we do not +## # falsly report the parameters of functions. +## my $ln = $line; +## while ($ln =~ s/\([^\(\)]*\)//g) { +## } +## if ($ln =~ /,/) { +## WARN("declaring multiple variables together should be avoided\n" . $herecurr); +## } +## } + #need space before brace following if, while, etc - if ($line=~/\(.*\){/) { - print "need a space before the brace\n"; - print "$herecurr"; - $clean = 0; + if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) || + $line =~ /do{/) { + ERROR("need a space before the open brace '{'\n" . $herecurr); + } + +# closing brace should have a space following it when it has anything +# on the line + if ($line =~ /}(?!(?:,|;|\)))\S/) { + ERROR("need a space after that close brace '}'\n" . $herecurr); + } + +# check spacing on square brackets + if ($line =~ /\[\s/ && $line !~ /\[\s*$/) { + ERROR("no space after that open square bracket '['\n" . $herecurr); + } + if ($line =~ /\s\]/) { + ERROR("no space before that close square bracket ']'\n" . $herecurr); + } + +# check spacing on paretheses + if ($line =~ /\(\s/ && $line !~ /\(\s*$/) { + ERROR("no space after that open parenthesis '('\n" . $herecurr); + } + if ($line =~ /\s\)/) { + ERROR("no space before that close parenthesis ')'\n" . $herecurr); } #goto labels aren't indented, allow a single space however - if ($line=~/^.\s+[A-Za-z\d_]+:/ and + if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { - print "labels should not be indented\n"; - print "$herecurr"; - $clean = 0; + WARN("labels should not be indented\n" . $herecurr); } # Need a space before open parenthesis after if, while etc - if ($line=~/(if|while|for|switch)\(/) { - print "need a space before the open parenthesis\n"; - print "$herecurr"; - $clean = 0; + if ($line=~/\b(if|while|for|switch)\(/) { + ERROR("need a space before the open parenthesis '('\n" . $herecurr); } # Check for illegal assignment in if conditional. - if ($line=~/(if|while)\s*\(.*[^<>!=]=[^=].*\)/) { - print "do not use assignment in if condition\n"; - print "$herecurr"; - $clean = 0; + if ($line=~/\bif\s*\(.*[^<>!=]=[^=].*\)/) { + #next if ($line=~/\".*\Q$op\E.*\"/ or $line=~/\'\Q$op\E\'/); + ERROR("do not use assignment in if condition\n" . $herecurr); } # Check for }else {, these must be at the same # indent level to be relevant to each other. if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and $previndent == $indent) { - print "else should follow close brace\n"; - print "$hereprev"; - $clean = 0; - } - - # Check for switch () {case, these must be at the - # same indent. We will only catch the first one, as our - # context is very small but people tend to be consistent - # so we will catch them out more often than not. - if ($prevline=~/\s*switch\s*\(.*\)/ and $line=~/\s*case\s+/ - and $previndent != $indent) { - print "switch and case should be at the same indent\n"; - print "$hereprev"; - $clean = 0; + ERROR("else should follow close brace '}'\n" . $hereprev); } #studly caps, commented out until figure out how to distinguish between use of existing and adding new @@ -504,85 +905,204 @@ sub process { #no spaces allowed after \ in define if ($line=~/\#define.*\\\s$/) { - print("Whitepspace after \\ makes next lines useless\n"); - print "$herecurr"; - $clean = 0; + WARN("Whitepspace after \\ makes next lines useless\n" . $herecurr); } -#warn if is #included and is available. - if ($tree && $line =~ qr|\s*\#\s*include\s*\|) { +#warn if is #included and is available (uses RAW line) + if ($tree && $rawline =~ m{^.\#\s*include\s*\}) { my $checkfile = "include/linux/$1.h"; if (-f $checkfile) { - print "Use #include instead of \n"; - print $herecurr; - $clean = 0; + CHK("Use #include instead of \n" . + $herecurr); } } -#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 ($prevline=~/(if|while|for|switch)\s*\(/) { - my @opened = $prevline=~/\(/g; - my @closed = $prevline=~/\)/g; - my $nr_line = $linenr; - my $remaining = $realcnt; - my $next_line = $line; - my $extra_lines = 0; - my $display_segment = $prevline; +# if and else should not have general statements after it + if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/ && + $1 !~ /^\s*(?:\sif|{|\\|$)/) { + ERROR("trailing statements should be on next line\n" . $herecurr); + } - while ($remaining > 0 && scalar @opened > scalar @closed) { - $prevline .= $next_line; - $display_segment .= "\n" . $next_line; - $next_line = $lines[$nr_line]; - $nr_line++; - $remaining--; +# 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 goot container + if (($prevline=~/\#define.*\\/) and + !($prevline=~/do\s+{/) and !($prevline=~/\(\{/) and + !($line=~/do.*{/) and !($line=~/\(\{/) and + !($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. + my $ln = $linenr; + my $cnt = $realcnt; + my $off = 0; + + # 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++; + } + my @ctx = ctx_statement($ln, $cnt, $off); + my $ctx_ln = $ln + $#ctx + 1; + my $ctx = join("\n", @ctx); + + # Pull in any empty extension lines. + while ($ctx =~ /\\$/ && + $lines[$ctx_ln - 1] =~ /^.\s*(?:\\)?$/) { + $ctx .= $lines[$ctx_ln - 1]; + $ctx_ln++; + } - @opened = $prevline=~/\(/g; - @closed = $prevline=~/\)/g; + if ($ctx =~ /\\$/) { + if ($ctx =~ /;/) { + ERROR("Macros with multiple statements should be enclosed in a do - while loop\n" . "$here\n$ctx\n"); + } else { + ERROR("Macros with complex values should be enclosed in parenthesis\n" . "$here\n$ctx\n"); + } } + } - if (($prevline=~/(if|while|for|switch)\s*\(.*\)\s*$/) and ($next_line=~/{/) and - !($next_line=~/(if|while|for)/) and !($next_line=~/\#define.*do.*while/)) { - print "That { should be on the previous line\n"; - print "$display_segment\n$next_line\n\n"; - $clean = 0; +# check for redundant bracing round if etc + if ($line =~ /\b(if|while|for|else)\b/) { + # Locate the end of the opening statement. + my @control = ctx_statement($linenr, $realcnt, 0); + my $nr = $linenr + (scalar(@control) - 1); + my $cnt = $realcnt - (scalar(@control) - 1); + + my $off = $realcnt - $cnt; + #print "$off: line<$line>end<" . $lines[$nr - 1] . ">\n"; + + # If this is is a braced statement group check it + if ($lines[$nr - 1] =~ /{\s*$/) { + my ($lvl, @block) = ctx_block_level($nr, $cnt); + + my $stmt = join(' ', @block); + $stmt =~ s/(^[^{]*){//; + my $before = $1; + $stmt =~ s/}([^}]*$)//; + my $after = $1; + + #print "block<" . join(' ', @block) . "><" . scalar(@block) . ">\n"; + #print "stmt<$stmt>\n\n"; + + # Count the ;'s if there is fewer than two + # then there can only be one statement, + # if there is a brace inside we cannot + # trivially detect if its one statement. + # Also nested if's often require braces to + # disambiguate the else binding so shhh there. + my @semi = ($stmt =~ /;/g); + push(@semi, "/**/") if ($stmt =~ m@/\*@); + ##print "semi<" . scalar(@semi) . ">\n"; + if ($lvl == 0 && scalar(@semi) < 2 && + $stmt !~ /{/ && $stmt !~ /\bif\b/ && + $before !~ /}/ && $after !~ /{/) { + my $herectx = "$here\n" . join("\n", @control, @block[1 .. $#block]) . "\n"; + shift(@block); + WARN("braces {} are not necessary for single statement blocks\n" . $herectx); + } } } -#multiline macros should be enclosed in a do while loop - if (($prevline=~/\#define.*\\/) and !($prevline=~/do\s+{/) and - !($prevline=~/\(\{/) and ($line=~/;\s*\\/) and - !($line=~/do.*{/) and !($line=~/\(\{/)) { - print "Macros with multiple statements should be enclosed in a do - while loop\n"; - print "$hereprev"; - $clean = 0; +# don't include deprecated include files (uses RAW line) + for my $inc (@dep_includes) { + if ($rawline =~ m@\#\s*include\s*\<$inc>@) { + ERROR("Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n" . $herecurr); + } } -# don't include deprecated include files - for my $inc (@deprecated) { - if ($line =~ m@\#\s*include\s*\<$inc>@) { - print "Don't use <$inc>: see Documentation/feature-removal-schedule.txt\n"; - print "$herecurr"; - $clean = 0; +# don't use deprecated functions + for my $func (@dep_functions) { + if ($line =~ /\b$func\b/) { + ERROR("Don't use $func(): see Documentation/feature-removal-schedule.txt\n" . $herecurr); } } -# don't use kernel_thread() - if ($line =~ /\bkernel_thread\b/) { - print "Don't use kernel_thread(), use kthread(): see Documentation/feature-removal-schedule.txt\n"; - print "$herecurr"; - $clean = 0; +# no volatiles please + if ($line =~ /\bvolatile\b/ && $line !~ /\basm\s+volatile\b/) { + WARN("Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr); + } + +# warn about #if 0 + if ($line =~ /^.#\s*if\s+0\b/) { + CHK("if this code is redundant consider removing it\n" . + $herecurr); + } + +# check for needless kfree() checks + 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 about #ifdefs in C files +# if ($line =~ /^.#\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+/) { + ERROR("exactly one space required after that #$1\n" . $herecurr); + } + +# check for spinlock_t definitions without a comment. + if ($line =~ /^.\s*(struct\s+mutex|spinlock_t)\s+\S+;/) { + my $which = $1; + if (!ctx_has_comment($first_line, $linenr)) { + CHK("$1 definition without comment\n" . $herecurr); + } + } +# check for memory barriers without a comment. + if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { + if (!ctx_has_comment($first_line, $linenr)) { + CHK("memory barrier without comment\n" . $herecurr); + } + } +# check of hardware specific defines + if ($line =~ m@^.#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { + CHK("architecture specific defines should be avoided\n" . $herecurr); + } + +# check the location of the inline attribute, that it is between +# storage class and type. + if ($line =~ /$Type\s+(?:inline|__always_inline|noinline)\b/ || + $line =~ /\b(?:inline|__always_inline|noinline)\s+$Storage/) { + ERROR("inline keyword should sit between storage class and type\n" . $herecurr); + } + +# check for new externs in .c files. + if ($line =~ /^.\s*extern\s/ && ($realfile =~ /\.c$/)) { + WARN("externs should be avoided in .c files\n" . $herecurr); + } + +# checks for new __setup's + if ($rawline =~ /\b__setup\("([^"]*)"/) { + my $name = $1; + + if (!grep(/$name/, @setup_docs)) { + CHK("__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr); + } } } if ($chk_patch && !$is_patch) { - $clean = 0; - print "Does not appear to be a unified-diff format patch\n"; + ERROR("Does not appear to be a unified-diff format patch\n"); } if ($is_patch && $chk_signoff && $signoff == 0) { - $clean = 0; - print "Missing Signed-off-by: line(s)\n"; + ERROR("Missing Signed-off-by: line(s)\n"); } + if ($clean == 0 && ($chk_patch || $is_patch)) { + print report_dump(); + } if ($clean == 1 && $quiet == 0) { print "Your patch has no obvious style problems and is ready for submission.\n" }