diff options
Diffstat (limited to 'scripts/checkpatch.pl')
| -rwxr-xr-x | scripts/checkpatch.pl | 206 |
1 files changed, 167 insertions, 39 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 90e1edc8dd42..f2a1131b2f8b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -264,6 +264,7 @@ our $Sparse = qr{ __kernel| __force| __iomem| + __pmem| __must_check| __init_refok| __kprobes| @@ -584,7 +585,7 @@ our $LvalOrFunc = qr{((?:[\&\*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*}; our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)}; our $declaration_macros = qr{(?x: - (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,2}\s*\(| + (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(| (?:$Storage\s+)?LIST_HEAD\s*\(| (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\( )}; @@ -1953,9 +1954,9 @@ sub process { our $clean = 1; my $signoff = 0; my $is_patch = 0; - my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch + my $commit_log_possible_stack_dump = 0; my $commit_log_long_line = 0; my $commit_log_has_diff = 0; my $reported_maintainer_file = 0; @@ -2166,11 +2167,15 @@ sub process { if ($showfile) { $prefix = "$realfile:$realline: " } elsif ($emacs) { - $prefix = "$filename:$linenr: "; + if ($file) { + $prefix = "$filename:$realline: "; + } else { + $prefix = "$filename:$linenr: "; + } } if ($found_file) { - if ($realfile =~ m@^(drivers/net/|net/)@) { + if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) { $check = 1; } else { $check = $check_orig; @@ -2310,16 +2315,42 @@ sub process { # Check for line lengths > 75 in commit log, warn once if ($in_commit_log && !$commit_log_long_line && - length($line) > 75) { + length($line) > 75 && + !($line =~ /^\s*[a-zA-Z0-9_\/\.]+\s+\|\s+\d+/ || + # file delta changes + $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ || + # filename then : + $line =~ /^\s*(?:Fixes:|Link:)/i || + # A Fixes: or Link: line + $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr); $commit_log_long_line = 1; } +# Check if the commit log is in a possible stack dump + if ($in_commit_log && !$commit_log_possible_stack_dump && + ($line =~ /^\s*(?:WARNING:|BUG:)/ || + $line =~ /^\s*\[\s*\d+\.\d{6,6}\s*\]/ || + # timestamp + $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/)) { + # stack dump address + $commit_log_possible_stack_dump = 1; + } + +# Reset possible stack dump if a blank line is found + if ($in_commit_log && $commit_log_possible_stack_dump && + $line =~ /^\s*$/) { + $commit_log_possible_stack_dump = 0; + } + # Check for git id commit length and improperly formed commit descriptions - if ($in_commit_log && $line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i) { - my $init_char = $1; - my $orig_commit = lc($2); + if ($in_commit_log && + ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + ($line =~ /\b[0-9a-f]{12,40}\b/i && + $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) { + my $init_char = "c"; + my $orig_commit = ""; my $short = 1; my $long = 0; my $case = 1; @@ -2330,6 +2361,13 @@ sub process { my $orig_desc = "commit description"; my $description = ""; + if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { + $init_char = $1; + $orig_commit = lc($2); + } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) { + $orig_commit = lc($1); + } + $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i); $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i); $space = 0 if ($line =~ /\bcommit [0-9a-f]/i); @@ -2599,7 +2637,7 @@ sub process { # if LONG_LINE is ignored, the other 2 types are also ignored # - if ($length > $max_line_length) { + if ($line =~ /^\+/ && $length > $max_line_length) { my $msg_type = "LONG_LINE"; # Check the allowed long line types first @@ -2738,6 +2776,8 @@ sub process { } } +# Block comment styles +# Networking with an initial /* if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ && $rawline =~ /^\+[ \t]*\*/ && @@ -2746,22 +2786,23 @@ sub process { "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev); } - if ($realfile =~ m@^(drivers/net/|net/)@ && - $prevrawline =~ /^\+[ \t]*\/\*/ && #starting /* +# Block comments use * on subsequent lines + if ($prevline =~ /$;[ \t]*$/ && #ends in comment + $prevrawline =~ /^\+.*?\/\*/ && #starting /* $prevrawline !~ /\*\/[ \t]*$/ && #no trailing */ $rawline =~ /^\+/ && #line is new $rawline !~ /^\+[ \t]*\*/) { #no leading * - WARN("NETWORKING_BLOCK_COMMENT_STYLE", - "networking block comments start with * on subsequent lines\n" . $hereprev); + WARN("BLOCK_COMMENT_STYLE", + "Block comments use * on subsequent lines\n" . $hereprev); } - if ($realfile =~ m@^(drivers/net/|net/)@ && - $rawline !~ m@^\+[ \t]*\*/[ \t]*$@ && #trailing */ +# Block comments use */ on trailing lines + if ($rawline !~ m@^\+[ \t]*\*/[ \t]*$@ && #trailing */ $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/ $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ && #trailing **/ $rawline =~ m@^\+[ \t]*.+\*\/[ \t]*$@) { #non blank */ - WARN("NETWORKING_BLOCK_COMMENT_STYLE", - "networking block comments put the trailing */ on a separate line\n" . $herecurr); + WARN("BLOCK_COMMENT_STYLE", + "Block comments use a trailing */ on a separate line\n" . $herecurr); } # check for missing blank lines after struct/union declarations @@ -3067,15 +3108,22 @@ sub process { 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; + # remove inline comments + $s =~ s/$;/ /g; + $c =~ s/$;/ /g; # Find out how long the conditional actually is. my @newlines = ($c =~ /\n/gs); my $cond_lines = 1 + $#newlines; + # 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; + while ($s =~ /\n\s+\\\n/) { + $cond_lines += $s =~ s/\n\s+\\\n/\n/g; + } + # We want to check the first line inside the block # starting at the end of the conditional, so remove: # 1) any blank line termination @@ -3141,8 +3189,10 @@ sub process { #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 ''))) { + if ($check && $s ne '' && + (($sindent % 8) != 0 || + ($sindent < $indent) || + ($sindent > $indent + 8))) { WARN("SUSPECT_CODE_INDENT", "suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); } @@ -3439,13 +3489,15 @@ sub process { } } -# # no BUG() or BUG_ON() -# if ($line =~ /\b(BUG|BUG_ON)\b/) { -# print "Try to use WARN_ON & Recovery code rather than BUG() or BUG_ON()\n"; -# print "$herecurr"; -# $clean = 0; -# } +# avoid BUG() or BUG_ON() + if ($line =~ /\b(?:BUG|BUG_ON)\b/) { + my $msg_type = \&WARN; + $msg_type = \&CHK if ($file); + &{$msg_type}("AVOID_BUG", + "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr); + } +# avoid LINUX_VERSION_CODE if ($line =~ /\bLINUX_VERSION_CODE\b/) { WARN("LINUX_VERSION_CODE", "LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged\n" . $herecurr); @@ -3520,7 +3572,7 @@ 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*$Ident\(.*\).*\s*{/) and - !($line=~/\#\s*define.*do\s{/) and !($line=~/}/)) { + !($line=~/\#\s*define.*do\s\{/) and !($line=~/}/)) { if (ERROR("OPEN_BRACE", "open brace '{' following function declarations go on the next line\n" . $herecurr) && $fix) { @@ -4032,8 +4084,8 @@ sub process { ## } #need space before brace following if, while, etc - if (($line =~ /\(.*\){/ && $line !~ /\($Type\){/) || - $line =~ /do{/) { + if (($line =~ /\(.*\)\{/ && $line !~ /\($Type\){/) || + $line =~ /do\{/) { if (ERROR("SPACING", "space required before the open brace '{'\n" . $herecurr) && $fix) { @@ -4179,6 +4231,35 @@ sub process { } } +# comparisons with a constant or upper case identifier on the left +# avoid cases like "foo + BAR < baz" +# only fix matches surrounded by parentheses to avoid incorrect +# conversions like "FOO < baz() + 5" being "misfixed" to "baz() > FOO + 5" + if ($^V && $^V ge 5.10.0 && + $line =~ /^\+(.*)\b($Constant|[A-Z_][A-Z0-9_]*)\s*($Compare)\s*($LvalOrFunc)/) { + my $lead = $1; + my $const = $2; + my $comp = $3; + my $to = $4; + my $newcomp = $comp; + if ($lead !~ /$Operators\s*$/ && + $to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/ && + WARN("CONSTANT_COMPARISON", + "Comparisons should place the constant on the right side of the test\n" . $herecurr) && + $fix) { + if ($comp eq "<") { + $newcomp = ">"; + } elsif ($comp eq "<=") { + $newcomp = ">="; + } elsif ($comp eq ">") { + $newcomp = "<"; + } elsif ($comp eq ">=") { + $newcomp = "<="; + } + $fixed[$fixlinenr] =~ s/\(\s*\Q$const\E\s*$Compare\s*\Q$to\E\s*\)/($to $newcomp $const)/; + } + } + # Return of what appears to be an errno should normally be negative if ($sline =~ /\breturn(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) { my $name = $1; @@ -4480,7 +4561,7 @@ sub process { $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... - $dstat !~ /^\({/ && # ({... + $dstat !~ /^\(\{/ && # ({... $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/) { $ctx =~ s/\n*$//; @@ -4789,16 +4870,20 @@ sub process { "Consecutive strings are generally better as a single string\n" . $herecurr); } -# check for %L{u,d,i} in strings +# check for %L{u,d,i} and 0x%[udi] in strings my $string; while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { $string = substr($rawline, $-[1], $+[1] - $-[1]); $string =~ s/%%/__/g; - if ($string =~ /(?<!%)%L[udi]/) { + if ($string =~ /(?<!%)%[\*\d\.\$]*L[udi]/) { WARN("PRINTF_L", "\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr); last; } + if ($string =~ /0x%[\*\d\.\$\Llzth]*[udi]/) { + ERROR("PRINTF_0xDECIMAL", + "Prefixing 0x with decimal output is defective\n" . $herecurr); + } } # check for line continuations in quoted strings with odd counts of " @@ -4816,10 +4901,34 @@ sub process { # check for needless "if (<foo>) fn(<foo>)" uses if ($prevline =~ /\bif\s*\(\s*($Lval)\s*\)/) { - my $expr = '\s*\(\s*' . quotemeta($1) . '\s*\)\s*;'; - if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) { - WARN('NEEDLESS_IF', - "$1(NULL) is safe and this check is probably not required\n" . $hereprev); + my $tested = quotemeta($1); + my $expr = '\s*\(\s*' . $tested . '\s*\)\s*;'; + if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?|(?:kmem_cache|mempool|dma_pool)_destroy)$expr/) { + my $func = $1; + if (WARN('NEEDLESS_IF', + "$func(NULL) is safe and this check is probably not required\n" . $hereprev) && + $fix) { + my $do_fix = 1; + my $leading_tabs = ""; + my $new_leading_tabs = ""; + if ($lines[$linenr - 2] =~ /^\+(\t*)if\s*\(\s*$tested\s*\)\s*$/) { + $leading_tabs = $1; + } else { + $do_fix = 0; + } + if ($lines[$linenr - 1] =~ /^\+(\t+)$func\s*\(\s*$tested\s*\)\s*;\s*$/) { + $new_leading_tabs = $1; + if (length($leading_tabs) + 1 ne length($new_leading_tabs)) { + $do_fix = 0; + } + } else { + $do_fix = 0; + } + if ($do_fix) { + fix_delete_line($fixlinenr - 1, $prevrawline); + $fixed[$fixlinenr] =~ s/^\+$new_leading_tabs/\+$leading_tabs/; + } + } } } @@ -5011,6 +5120,7 @@ sub process { "memory barrier without comment\n" . $herecurr); } } + # check for waitqueue_active without a comment. if ($line =~ /\bwaitqueue_active\s*\(/) { if (!ctx_has_comment($first_line, $linenr)) { @@ -5018,6 +5128,24 @@ sub process { "waitqueue_active without comment\n" . $herecurr); } } + +# Check for expedited grace periods that interrupt non-idle non-nohz +# online CPUs. These expedited can therefore degrade real-time response +# if used carelessly, and should be avoided where not absolutely +# needed. It is always OK to use synchronize_rcu_expedited() and +# synchronize_sched_expedited() at boot time (before real-time applications +# start) and in error situations where real-time response is compromised in +# any case. Note that synchronize_srcu_expedited() does -not- interrupt +# other CPUs, so don't warn on uses of synchronize_srcu_expedited(). +# Of course, nothing comes for free, and srcu_read_lock() and +# srcu_read_unlock() do contain full memory barriers in payment for +# synchronize_srcu_expedited() non-interruption properties. + if ($line =~ /\b(synchronize_rcu_expedited|synchronize_sched_expedited)\(/) { + WARN("EXPEDITED_RCU_GRACE_PERIOD", + "expedited RCU grace periods should be avoided where they can degrade real-time response\n" . $herecurr); + + } + # check of hardware specific defines if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { CHK("ARCH_DEFINES", |