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: Andy Whitcroft <apw@shadowen.org>
Cc: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 2a7cef9..58a9494 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -9,7 +9,7 @@
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 $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 @@
'debug=s' => \%debug,
'test-type!' => \$tst_type,
+ 'test-only=s' => \$tst_only,
) or exit;
my $exit = 0;
@@ -263,17 +265,7 @@
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 @@
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';
+ # 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 ($l ne "\\" && ($c eq "'" || $c eq '"')) {
- if ($quote eq '') {
- $quote = $c;
- $res .= $c;
- $l = $c;
- $qlen = 0;
+ if (substr($line, $off, 2) eq $sanitise_quote) {
+ $sanitise_quote = '';
+ substr($res, $off, 2, "$;$;");
+ $off++;
+ next;
+ }
+
+ # 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;
+
+ substr($res, $off, 1, $c);
next;
- } elsif ($quote eq $c) {
- $quote = '';
+ } elsif ($sanitise_quote eq $c) {
+ $sanitise_quote = '';
}
}
- if ($quote eq "'" && $qlen > 1) {
- $quote = '';
- }
- if ($quote && $c ne "\t") {
- $res .= "X";
- $qlen++;
+
+ #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 {
- $res .= $c;
+ substr($res, $off, 1, $c);
}
-
- $l = $c;
- }
-
- # 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]);
}
# The pathname on a #include may be surrounded by '<' and '>'.
@@ -359,6 +374,7 @@
my $blk = '';
my $soff = $off;
my $coff = $off - 1;
+ my $coff_set = 0;
my $loff = 0;
@@ -370,7 +386,7 @@
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 @@
$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 @@
}
# 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 @@
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 @@
#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 @@
# 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 @@
($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 @@
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 @@
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 @@
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);
+ $linenr++;
+ $line = $rawline;
- ##print "==>$rawline\n";
- ##print "-->$line\n";
-
- 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 @@
$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 @@
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 @@
($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 @@
$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 @@
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 @@
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 @@
# 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);
- # Skip over any removed lines in the context following statement.
- while ($ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^-/) {
- $ctx_ln++;
- $ctx_cnt--;
- }
- ##warn "line<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>";
+ ##warn "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n";
- if ($ctx !~ /{\s*/ && $ctx_cnt > 0 && $lines[$ctx_ln - 1] =~ /^\+\s*{/) {
- ERROR("That open brace { should be on the previous line\n" .
+ # Skip over any removed lines in the context following statement.
+ while (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";
+
+ 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 @@
# 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 @@
# 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 @@
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 @@
$c = 'E';
}
- # 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)";
@@ -1424,7 +1482,7 @@
} elsif ($op eq ';') {
if ($ctx !~ /.x[WEBC]/ &&
$cc !~ /^\\/ && $cc !~ /^;/) {
- ERROR("need space after that '$op' $at\n" . $hereptr);
+ ERROR("space required after that '$op' $at\n" . $hereptr);
}
# // is a comment
@@ -1433,13 +1491,13 @@
# -> should have no spaces
} elsif ($op eq '->') {
if ($ctx =~ /Wx.|.xW/) {
- ERROR("no spaces around that '$op' $at\n" . $hereptr);
+ ERROR("spaces prohibited around that '$op' $at\n" . $hereptr);
}
# , must have a space on the right.
} elsif ($op eq ',') {
if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) {
- ERROR("need space after that '$op' $at\n" . $hereptr);
+ ERROR("space required after that '$op' $at\n" . $hereptr);
}
# '*' as part of a type definition -- reported already.
@@ -1452,20 +1510,25 @@
} elsif ($op eq '!' || $op eq '~' ||
($is_unary && ($op eq '*' || $op eq '-' || $op eq '&'))) {
if ($ctx !~ /[WEBC]x./ && $ca !~ /(?:\)|!|~|\*|-|\&|\||\+\+|\-\-|\{)$/) {
- ERROR("need space before that '$op' $at\n" . $hereptr);
+ ERROR("space required before that '$op' $at\n" . $hereptr);
}
if ($ctx =~ /.xW/) {
- ERROR("no space after that '$op' $at\n" . $hereptr);
+ ERROR("space prohibited after that '$op' $at\n" . $hereptr);
}
# unary ++ and unary -- are allowed no space on one side.
} elsif ($op eq '++' or $op eq '--') {
- if ($ctx !~ /[WOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) {
- ERROR("need space one side of that '$op' $at\n" . $hereptr);
+ if ($ctx !~ /[WEOBC]x[^W]/ && $ctx !~ /[^W]x[WOBEC]/) {
+ ERROR("space required one side of that '$op' $at\n" . $hereptr);
}
- if ($ctx =~ /WxB/ || ($ctx =~ /Wx./ && $cc =~ /^;/)) {
- ERROR("no space before that '$op' $at\n" . $hereptr);
+ if ($ctx =~ /Wx[BE]/ ||
+ ($ctx =~ /Wx./ && $cc =~ /^;/)) {
+ ERROR("space prohibited before that '$op' $at\n" . $hereptr);
}
+ if ($ctx =~ /ExW/) {
+ ERROR("space prohibited after that '$op' $at\n" . $hereptr);
+ }
+
# << and >> may either have or not have spaces both sides
} elsif ($op eq '<<' or $op eq '>>' or
@@ -1474,7 +1537,7 @@
$op eq '*' or $op eq '/' or
$op eq '%')
{
- if ($ctx !~ /VxV|WxW|VxE|WxE|VxO|Cx.|.xC/) {
+ if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) {
ERROR("need consistent spacing around '$op' $at\n" .
$hereptr);
}
@@ -1484,7 +1547,7 @@
# Ignore email addresses <foo@bar>
if (!($op eq '<' && $cb =~ /$;\S+\@\S+>/) &&
!($op eq '>' && $cb =~ /<\S+\@\S+$;/)) {
- ERROR("need spaces around that '$op' $at\n" . $hereptr);
+ ERROR("spaces required around that '$op' $at\n" . $hereptr);
}
}
$off += length($elements[$n + 1]);
@@ -1514,31 +1577,31 @@
#need space before brace following if, while, etc
if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) ||
$line =~ /do{/) {
- ERROR("need a space before the open brace '{'\n" . $herecurr);
+ ERROR("space required 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);
+ ERROR("space required 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);
+ ERROR("space prohibited after that open square bracket '['\n" . $herecurr);
}
if ($line =~ /\s\]/) {
- ERROR("no space before that close square bracket ']'\n" . $herecurr);
+ ERROR("space prohibited before that close square bracket ']'\n" . $herecurr);
}
# check spacing on paretheses
if ($line =~ /\(\s/ && $line !~ /\(\s*(?:\\)?$/ &&
$line !~ /for\s*\(\s+;/) {
- ERROR("no space after that open parenthesis '('\n" . $herecurr);
+ ERROR("space prohibited after that open parenthesis '('\n" . $herecurr);
}
if ($line =~ /(\s+)\)/ && $line !~ /^.\s*\)/ &&
$line !~ /for\s*\(.*;\s+\)/) {
- ERROR("no space before that close parenthesis ')'\n" . $herecurr);
+ ERROR("space prohibited before that close parenthesis ')'\n" . $herecurr);
}
#goto labels aren't indented, allow a single space however
@@ -1549,7 +1612,7 @@
# Need a space before open parenthesis after if, while etc
if ($line=~/\b(if|while|for|switch)\(/) {
- ERROR("need a space before the open parenthesis '('\n" . $herecurr);
+ ERROR("space required before the open parenthesis '('\n" . $herecurr);
}
# Check for illegal assignment in if conditional.
@@ -1562,10 +1625,12 @@
# Find out what is on the end of the line after the
# conditional.
- substr($s, 0, length($c)) = '';
+ substr($s, 0, length($c), '');
$s =~ s/\n.*//g;
$s =~ s/$;//g; # Remove any comments
- if (length($c) && $s !~ /^\s*({|;|)\s*\\*\s*$/) {
+ if (length($c) && $s !~ /^\s*({|;|)\s*\\*\s*$/ &&
+ $c !~ /^.\#\s*if/)
+ {
ERROR("trailing statements should be on next line\n" . $herecurr);
}
}
@@ -1607,7 +1672,7 @@
# Find out what is on the end of the line after the
# conditional.
- substr($s, 0, length($c)) = '';
+ substr($s, 0, length($c), '');
$s =~ s/\n.*//g;
if ($s =~ /^\s*;/) {
@@ -1631,7 +1696,7 @@
if ($tree && $rawline =~ m{^.\#\s*include\s*\<asm\/(.*)\.h\>}) {
my $checkfile = "$root/include/linux/$1.h";
if (-f $checkfile && $1 ne 'irq.h') {
- CHK("Use #include <linux/$1.h> instead of <asm/$1.h>\n" .
+ WARN("Use #include <linux/$1.h> instead of <asm/$1.h>\n" .
$herecurr);
}
}
@@ -1692,15 +1757,24 @@
if ($#chunks > 0 && $level == 0) {
my $allowed = 0;
my $seen = 0;
- my $herectx = $here . "\n";;
+ my $herectx = $here . "\n";
my $ln = $linenr - 1;
for my $chunk (@chunks) {
my ($cond, $block) = @{$chunk};
- $herectx .= "$rawlines[$ln]\n[...]\n";
+ # If the condition carries leading newlines, then count those as offsets.
+ my ($whitespace) = ($cond =~ /^((?:\s*\n[+-])*\s*)/s);
+ my $offset = statement_rawlines($whitespace) - 1;
+
+ #print "COND<$cond> whitespace<$whitespace> offset<$offset>\n";
+
+ # We have looked at and allowed this specific line.
+ $suppress_ifbraces{$ln + $offset} = 1;
+
+ $herectx .= "$rawlines[$ln + $offset]\n[...]\n";
$ln += statement_rawlines($block) - 1;
- substr($block, 0, length($cond)) = '';
+ substr($block, 0, length($cond), '');
$seen++ if ($block =~ /^\s*{/);
@@ -1721,16 +1795,10 @@
if ($seen && !$allowed) {
WARN("braces {} are not necessary for any arm of this statement\n" . $herectx);
}
- # Either way we have looked over this whole
- # statement and said what needs to be said.
- $suppress_ifbraces = $endln;
}
}
- if ($linenr > $suppress_ifbraces &&
+ if (!defined $suppress_ifbraces{$linenr - 1} &&
$line =~ /\b(if|while|for|else)\b/) {
- my ($level, $endln, @chunks) =
- ctx_statement_full($linenr, $realcnt, $-[0]);
-
my $allowed = 0;
# Check the pre-context.
@@ -1738,10 +1806,15 @@
#print "APW: ALLOWED: pre<$1>\n";
$allowed = 1;
}
+
+ my ($level, $endln, @chunks) =
+ ctx_statement_full($linenr, $realcnt, $-[0]);
+
# Check the condition.
my ($cond, $block) = @{$chunks[0]};
+ #print "CHECKING<$linenr> cond<$cond> block<$block>\n";
if (defined $cond) {
- substr($block, 0, length($cond)) = '';
+ substr($block, 0, length($cond), '');
}
if (statement_lines($cond) > 1) {
#print "APW: ALLOWED: cond<$cond>\n";
@@ -1759,7 +1832,7 @@
if (defined $chunks[1]) {
my ($cond, $block) = @{$chunks[1]};
if (defined $cond) {
- substr($block, 0, length($cond)) = '';
+ substr($block, 0, length($cond), '');
}
if ($block =~ /^\s*\{/) {
#print "APW: ALLOWED: chunk-1 block<$block>\n";
@@ -1882,6 +1955,28 @@
if ($line =~ /__FUNCTION__/) {
WARN("__func__ should be used instead of gcc specific __FUNCTION__\n" . $herecurr);
}
+
+# check for semaphores used as mutexes
+ if ($line =~ /\b(DECLARE_MUTEX|init_MUTEX)\s*\(/) {
+ WARN("mutexes are preferred for single holder semaphores\n" . $herecurr);
+ }
+# check for semaphores used as mutexes
+ if ($line =~ /\binit_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);
+ }
+
+# 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*define\s+NR_CPUS\s+/ &&
+ $line !~ /^.\s*$Declare\s.*\[[^\]]*NR_CPUS[^\]]*\]/)
+ {
+ WARN("usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr);
+ }
}
# If we have no input at all, then there is nothing to report on