Commit 773647a0 authored by Andy Whitcroft's avatar Andy Whitcroft Committed by Linus Torvalds
Browse files

update checkpatch.pl to version 0.16



This version brings proper quote tracking across lines, and brings the
handling of comments into the same mechanism ensuring nesting is correctly
handled.  It brings the usual flurry of fixes for false positives.  It also
brings a number of new checks.  The most contentious change will likely be
the checks for NR_CPUS as this throws some new warnings in kernel/sched.c.

Of note:
 - all new quote tracking across lines
 - all new comment tracking
 - new more direct, less ambigious wording for some warnings
 - recommends mutexes and completions over semaphores
 - recommends strict_strto* over simple_strto*
 - report on direct use of NR_CPUS

Andy Whitcroft (22):
      Version: 0.16
      string quote tracking should cross line boundaries
      check spacing round -> correctly across newlines
      checks for linux/ against asm/ include files should be warnings
      standardise on 'required' and 'prohibited'
      take the first end of condition when parsing statements
      values: cope with unbalanced brackets
      preprocessor #elif is not a function
      preprocessor #if should not trigger trailing statement checks
      test: allow us to limit output to a single error
      recommend real mutexes over semaphores
      asm checks should mirror those for __asm__
      warn on semaphores being used in place of completions
      trailing ; on control structure should ignore do {} while ();
      recommend strict_strtoX over simple_strtoX
      redo comment handling as a quote type
      use of NR_CPUS is normally wrong
      consistant spacing should only be about spaces
      if brace check suppression should only apply to the top-levels
      use tr/// to align spacing for operators
      move to using four parameter form of substr
      check and report modifications to include/asm
Signed-off-by: default avatarAndy Whitcroft <apw@shadowen.org>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 3387b804
......@@ -9,7 +9,7 @@ use strict;
my $P = $0;
$P =~ s@.*/@@g;
my $V = '0.15';
my $V = '0.16';
use Getopt::Long qw(:config no_auto_abbrev);
......@@ -18,6 +18,7 @@ my $tree = 1;
my $chk_signoff = 1;
my $chk_patch = 1;
my $tst_type = 0;
my $tst_only;
my $emacs = 0;
my $terse = 0;
my $file = 0;
......@@ -44,6 +45,7 @@ GetOptions(
'debug=s' => \%debug,
'test-type!' => \$tst_type,
'test-only=s' => \$tst_only,
) or exit;
my $exit = 0;
......@@ -263,17 +265,7 @@ sub expand_tabs {
return $res;
}
sub copy_spacing {
my ($str) = @_;
my $res = '';
for my $c (split(//, $str)) {
if ($c eq "\t") {
$res .= $c;
} else {
$res .= ' ';
}
}
(my $res = shift) =~ tr/\t/ /c;
return $res;
}
......@@ -290,53 +282,76 @@ sub line_stats {
return (length($line), length($white));
}
my $sanitise_quote = '';
sub sanitise_line_reset {
my ($in_comment) = @_;
if ($in_comment) {
$sanitise_quote = '*/';
} else {
$sanitise_quote = '';
}
}
sub sanitise_line {
my ($line) = @_;
my $res = '';
my $l = '';
my $quote = '';
my $qlen = 0;
my $off = 0;
my $c;
foreach my $c (split(//, $line)) {
# The second backslash of a pair is not a "quote".
if ($l eq "\\" && $c eq "\\") {
$c = 'X';
}
if ($l ne "\\" && ($c eq "'" || $c eq '"')) {
if ($quote eq '') {
$quote = $c;
$res .= $c;
$l = $c;
$qlen = 0;
next;
} elsif ($quote eq $c) {
$quote = '';
}
# Always copy over the diff marker.
$res = substr($line, 0, 1);
for ($off = 1; $off < length($line); $off++) {
$c = substr($line, $off, 1);
# Comments we are wacking completly including the begin
# and end, all to $;.
if ($sanitise_quote eq '' && substr($line, $off, 2) eq '/*') {
$sanitise_quote = '*/';
substr($res, $off, 2, "$;$;");
$off++;
next;
}
if ($quote eq "'" && $qlen > 1) {
$quote = '';
if (substr($line, $off, 2) eq $sanitise_quote) {
$sanitise_quote = '';
substr($res, $off, 2, "$;$;");
$off++;
next;
}
if ($quote && $c ne "\t") {
$res .= "X";
$qlen++;
} else {
$res .= $c;
# A \ in a string means ignore the next character.
if (($sanitise_quote eq "'" || $sanitise_quote eq '"') &&
$c eq "\\") {
substr($res, $off, 2, 'XX');
$off++;
next;
}
# Regular quotes.
if ($c eq "'" || $c eq '"') {
if ($sanitise_quote eq '') {
$sanitise_quote = $c;
$l = $c;
}
substr($res, $off, 1, $c);
next;
} elsif ($sanitise_quote eq $c) {
$sanitise_quote = '';
}
}
# Clear out the comments.
while ($res =~ m@(/\*.*?\*/)@g) {
substr($res, $-[1], $+[1] - $-[1]) = $; x ($+[1] - $-[1]);
}
if ($res =~ m@(/\*.*)@) {
substr($res, $-[1], $+[1] - $-[1]) = $; x ($+[1] - $-[1]);
}
if ($res =~ m@^.(.*\*/)@) {
substr($res, $-[1], $+[1] - $-[1]) = $; x ($+[1] - $-[1]);
#print "SQ:$sanitise_quote\n";
if ($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 {
substr($res, $off, 1, $c);
}
}
# The pathname on a #include may be surrounded by '<' and '>'.
......@@ -359,6 +374,7 @@ sub ctx_statement_block {
my $blk = '';
my $soff = $off;
my $coff = $off - 1;
my $coff_set = 0;
my $loff = 0;
......@@ -370,7 +386,7 @@ sub ctx_statement_block {
my $remainder;
while (1) {
#warn "CSB: blk<$blk>\n";
#warn "CSB: blk<$blk> remain<$remain>\n";
# If we are about to drop off the end, pull in more
# context.
if ($off >= $len) {
......@@ -393,7 +409,7 @@ sub ctx_statement_block {
$c = substr($blk, $off, 1);
$remainder = substr($blk, $off);
#warn "CSB: c<$c> type<$type> level<$level>\n";
#warn "CSB: c<$c> type<$type> level<$level> remainder<$remainder> coff_set<$coff_set>\n";
# Statement ends at the ';' or a close '}' at the
# outermost level.
if ($level == 0 && $c eq ';') {
......@@ -401,10 +417,14 @@ sub ctx_statement_block {
}
# An else is really a conditional as long as its not else if
if ($level == 0 && (!defined($p) || $p =~ /(?:\s|\})/) &&
$remainder =~ /(else)(?:\s|{)/ &&
$remainder !~ /else\s+if\b/) {
$coff = $off + length($1);
if ($level == 0 && $coff_set == 0 &&
(!defined($p) || $p =~ /(?:\s|\}|\+)/) &&
$remainder =~ /^(else)(?:\s|{)/ &&
$remainder !~ /^else\s+if\b/) {
$coff = $off + length($1) - 1;
$coff_set = 1;
#warn "CSB: mark coff<$coff> soff<$soff> 1<$1>\n";
#warn "[" . substr($blk, $soff, $coff - $soff + 1) . "]\n";
}
if (($type eq '' || $type eq '(') && $c eq '(') {
......@@ -417,6 +437,8 @@ sub ctx_statement_block {
if ($level == 0 && $coff < $soff) {
$coff = $off;
$coff_set = 1;
#warn "CSB: mark coff<$coff>\n";
}
}
if (($type eq '' || $type eq '{') && $c eq '{') {
......@@ -444,7 +466,7 @@ sub ctx_statement_block {
#warn "STATEMENT<$statement>\n";
#warn "CONDITION<$condition>\n";
#print "off<$off> loff<$loff>\n";
#print "coff<$coff> soff<$off> loff<$loff>\n";
return ($statement, $condition,
$line, $remain + 1, $off - $loff + 1, $level);
......@@ -502,7 +524,7 @@ sub ctx_statement_full {
# Grab the first conditional/block pair.
($statement, $condition, $linenr, $remain, $off, $level) =
ctx_statement_block($linenr, $remain, $off);
#print "F: c<$condition> s<$statement>\n";
#print "F: c<$condition> s<$statement> remain<$remain>\n";
push(@chunks, [ $condition, $statement ]);
if (!($remain > 0 && $condition =~ /^\s*(?:\n[+-])?\s*(?:if|else|do)\b/s)) {
return ($level, $linenr, @chunks);
......@@ -514,7 +536,7 @@ sub ctx_statement_full {
($statement, $condition, $linenr, $remain, $off, $level) =
ctx_statement_block($linenr, $remain, $off);
#print "C: c<$condition> s<$statement> remain<$remain>\n";
last if (!($remain > 0 && $condition =~ /^\s*(?:\n[+-])?\s*(?:else|do)\b/s));
last if (!($remain > 0 && $condition =~ /^(?:\s*\n[+-])*\s*(?:else|do)\b/s));
#print "C: push\n";
push(@chunks, [ $condition, $statement ]);
}
......@@ -668,6 +690,7 @@ sub annotate_values {
print "$stream\n" if ($dbg_values > 1);
while (length($cur)) {
@av_paren_type = ('E') if ($#av_paren_type < 0);
print " <" . join('', @av_paren_type) .
"> <$type> " if ($dbg_values > 1);
if ($cur =~ /^(\s+)/o) {
......@@ -804,28 +827,34 @@ sub possible {
my $prefix = '';
sub report {
if (defined $tst_only && $_[0] !~ /\Q$tst_only\E/) {
return 0;
}
my $line = $prefix . $_[0];
$line = (split('\n', $line))[0] . "\n" if ($terse);
push(our @report, $line);
return 1;
}
sub report_dump {
our @report;
}
sub ERROR {
report("ERROR: $_[0]\n");
our $clean = 0;
our $cnt_error++;
if (report("ERROR: $_[0]\n")) {
our $clean = 0;
our $cnt_error++;
}
}
sub WARN {
report("WARNING: $_[0]\n");
our $clean = 0;
our $cnt_warn++;
if (report("WARNING: $_[0]\n")) {
our $clean = 0;
our $cnt_warn++;
}
}
sub CHK {
if ($check) {
report("CHECK: $_[0]\n");
if ($check && report("CHECK: $_[0]\n")) {
our $clean = 0;
our $cnt_chk++;
}
......@@ -867,30 +896,76 @@ sub process {
my $prev_values = 'E';
# suppression flags
my $suppress_ifbraces = 0;
my %suppress_ifbraces;
# Pre-scan the patch sanitizing the lines.
# Pre-scan the patch looking for any __setup documentation.
#
my @setup_docs = ();
my $setup_docs = 0;
sanitise_line_reset();
my $line;
foreach my $rawline (@rawlines) {
# Standardise the strings and chars within the input to
# simplify matching.
$line = sanitise_line($rawline);
push(@lines, $line);
##print "==>$rawline\n";
##print "-->$line\n";
$linenr++;
$line = $rawline;
if ($line=~/^\+\+\+\s+(\S+)/) {
if ($rawline=~/^\+\+\+\s+(\S+)/) {
$setup_docs = 0;
if ($1 =~ m@Documentation/kernel-parameters.txt$@) {
$setup_docs = 1;
}
next;
#next;
}
if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) {
$realline=$1-1;
if (defined $2) {
$realcnt=$3+1;
} else {
$realcnt=1+1;
}
# 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; $ln < ($linenr + $realcnt); $ln++) {
next if ($line =~ /^-/);
($edge) = ($rawlines[$ln - 1] =~ m@(/\*|\*/)@);
last if (defined $edge);
}
if (defined $edge && $edge eq '*/') {
$in_comment = 1;
}
# 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 (!defined $edge &&
$rawlines[$linenr] =~ m@^.\s* \*(?:\s|$)@)
{
$in_comment = 1;
}
##print "COMMENT:$in_comment edge<$edge> $rawline\n";
sanitise_line_reset($in_comment);
} elsif ($realcnt) {
# Standardise the strings and chars within the input to
# simplify matching.
$line = sanitise_line($rawline);
}
push(@lines, $line);
if ($realcnt > 1) {
$realcnt-- if ($line =~ /^(?:\+| |$)/);
} else {
$realcnt = 0;
}
#print "==>$rawline\n";
#print "-->$line\n";
if ($setup_docs && $line =~ /^\+/) {
push(@setup_docs, $line);
......@@ -899,23 +974,17 @@ sub process {
$prefix = '';
$realcnt = 0;
$linenr = 0;
foreach my $line (@lines) {
$linenr++;
my $rawline = $rawlines[$linenr - 1];
#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 = $linenr + 1;
$in_comment = 0;
$realline=$1-1;
if (defined $2) {
$realcnt=$3+1;
......@@ -925,50 +994,16 @@ sub process {
annotate_reset();
$prev_values = 'E';
$suppress_ifbraces = $linenr - 1;
%suppress_ifbraces = ();
next;
}
# 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 =~ /^( |\+|$)/) {
} elsif ($line =~ /^( |\+|$)/) {
$realline++;
$realcnt-- if ($realcnt != 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.
if ($linenr == $first_line) {
my $edge;
for (my $ln = $first_line; $ln < ($linenr + $realcnt); $ln++) {
($edge) = ($rawlines[$ln - 1] =~ m@(/\*|\*/)@);
last if (defined $edge);
}
if (defined $edge && $edge eq '*/') {
$in_comment = 1;
}
}
# 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 ($linenr == $first_line and $rawline =~ m@^.\s* \*(?:\s|$)@) {
$in_comment = 1;
}
# Find the last comment edge on _this_ line.
$comment_edge = 0;
while (($rawline =~ m@(/\*|\*/)@g)) {
if ($1 eq '/*') {
$in_comment = 1;
} else {
$in_comment = 0;
}
$comment_edge = 1;
}
# Measure the line length and indent.
($length, $indent) = line_stats($rawline);
......@@ -977,23 +1012,36 @@ sub process {
($previndent, $stashindent) = ($stashindent, $indent);
($prevrawline, $stashrawline) = ($stashrawline, $rawline);
#warn "ic<$in_comment> ce<$comment_edge> line<$line>\n";
#warn "line<$line>\n";
} elsif ($realcnt == 1) {
$realcnt--;
}
#make up the handle for any error we report on this line
$prefix = "$filename:$realline: " if ($emacs && $file);
$prefix = "$filename:$linenr: " if ($emacs && !$file);
$here = "#$linenr: " if (!$file);
$here = "#$realline: " if ($file);
# extract the filename as it passes
if ($line=~/^\+\+\+\s+(\S+)/) {
$realfile = $1;
$realfile =~ s@^[^/]*/@@;
if ($realfile =~ m@include/asm/@) {
ERROR("do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n");
}
next;
}
$here .= "FILE: $realfile:$realline:" if ($realcnt != 0);
my $hereline = "$here\n$rawline\n";
my $herecurr = "$here\n$rawline\n";
my $hereprev = "$here\n$prevrawline\n$rawline\n";
$prefix = "$filename:$realline: " if ($emacs && $file);
$prefix = "$filename:$linenr: " if ($emacs && !$file);
$cnt_lines++ if ($realcnt != 0);
#check the patch for a signoff:
......@@ -1005,7 +1053,7 @@ sub process {
$herecurr);
}
if ($line =~ /^\s*signed-off-by:\S/i) {
WARN("need space after Signed-off-by:\n" .
WARN("space required after Signed-off-by:\n" .
$herecurr);
}
}
......@@ -1072,11 +1120,6 @@ sub process {
WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr);
}
# 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);
# Check for potential 'bare' types
if ($realcnt) {
my ($s, $c) = ctx_statement_block($linenr, $realcnt, 0);
......@@ -1110,7 +1153,7 @@ sub process {
my ($name_len) = length($1);
my $ctx = $s;
substr($ctx, 0, $name_len + 1) = '';
substr($ctx, 0, $name_len + 1, '');
$ctx =~ s/\)[^\)]*$//;
for my $arg (split(/\s*,\s*/, $ctx)) {
......@@ -1151,27 +1194,33 @@ 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 !~ /^.#/) {
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";
# Skip over any removed lines in the context following statement.
while ($ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^-/) {
while (defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^-/) {
$ctx_ln++;
$ctx_cnt--;
}
##warn "line<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>";
##warn "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n";
if ($ctx !~ /{\s*/ && $ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
ERROR("That open brace { should be on the previous line\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]");
}
if ($level == 0 && $ctx =~ /\)\s*\;\s*$/ && defined $lines[$ctx_ln - 1]) {
if ($level == 0 && $pre_ctx !~ /}\s*while\s*\($/ &&
$ctx =~ /\)\s*\;\s*$/ &&
defined $lines[$ctx_ln - 1])
{
my ($nlength, $nindent) = line_stats($lines[$ctx_ln - 1]);
if ($nindent > $indent) {
WARN("Trailing semicolon indicates no statements, indent implies otherwise\n" .
WARN("trailing semicolon indicates no statements, indent implies otherwise\n" .
"$here\n$ctx\n$lines[$ctx_ln - 1]");
}
}
......@@ -1200,7 +1249,7 @@ sub process {
# 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);
ERROR("that open brace { should be on the previous line\n" . $hereprev);
}
#
......@@ -1325,22 +1374,31 @@ sub process {
# check for spaces between functions and their parentheses.
while ($line =~ /($Ident)\s+\(/g) {
my $name = $1;
my $ctx = substr($line, 0, $-[1]);
my $ctx_before = substr($line, 0, $-[1]);
my $ctx = "$ctx_before$name";
# Ignore those directives where spaces _are_ permitted.
if ($name =~ /^(?:if|for|while|switch|return|volatile|__volatile__|__attribute__|format|__extension__|Copyright|case|__asm__)$/) {
if ($name =~ /^(?:
if|for|while|switch|return|case|
volatile|__volatile__|
__attribute__|format|__extension__|
asm|__asm__)$/x)
{
# 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 =~ /^.\#\s*define\s*$/) {
} elsif ($ctx_before =~ /^.\#\s*define\s*$/) {
# cpp #elif statement condition may start with a (
} elsif ($ctx =~ /^.\#\s*elif\s*$/) {
# If this whole things ends with a type its most
# likely a typedef for a function.
} elsif ("$ctx$name" =~ /$Type$/) {
} elsif ($ctx =~ /$Type$/) {
} else {
WARN("no space between function name and open parenthesis '('\n" . $herecurr);
WARN("space prohibited between function name and open parenthesis '('\n" . $herecurr);
}
}
# Check operator spacing.
......@@ -1359,13 +1417,21 @@ sub process {
for (my $n = 0; $n < $#elements; $n += 2) {
$off += length($elements[$n]);
# 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 $a = '';
$a = 'V' if ($elements[$n] ne '');
$a = 'W' if ($elements[$n] =~ /\s$/);
$a = 'C' if ($elements[$n] =~ /$;$/);
$a = 'B' if ($elements[$n] =~ /(\[|\()$/);
$a = 'O' if ($elements[$n] eq '');
$a = 'E' if ($elements[$n] eq '' && $n == 0);
$a = 'E' if ($ca =~ /^\s*$/);
my $op = $elements[$n + 1];
......@@ -1381,14 +1447,6 @@ sub process {
$c = 'E';
}
# Pick up the preceeding and succeeding characters.