diff options
Diffstat (limited to 'scripts/checkpatch.pl')
-rwxr-xr-x | scripts/checkpatch.pl | 319 |
1 files changed, 248 insertions, 71 deletions
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c5ec977b9..d5c8e9a3a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -9,6 +9,7 @@ use strict; use POSIX; use File::Basename; use Cwd 'abs_path'; +use Term::ANSIColor qw(:constants); my $P = $0; my $D = dirname(abs_path($P)); @@ -24,6 +25,7 @@ my $chk_patch = 1; my $tst_only; my $emacs = 0; my $terse = 0; +my $showfile = 0; my $file = 0; my $check = 0; my $check_orig = 0; @@ -48,7 +50,8 @@ my $minimum_perl_version = 5.10.0; my $min_conf_desc_length = 4; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; -my $codespellfile = "/usr/local/share/codespell/dictionary.txt"; +my $codespellfile = "/usr/share/codespell/dictionary.txt"; +my $color = 1; sub help { my ($exitcode) = @_; @@ -64,6 +67,7 @@ Options: --patch treat FILE as patchfile (default) --emacs emacs compile window format --terse one line per report + --showfile emit diffed file position, not input file position -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests --types TYPE(,TYPE2...) show only these comma separated message types @@ -91,8 +95,9 @@ Options: --ignore-perl-version override checking of perl version. expect runtime errors. --codespell Use the codespell dictionary for spelling/typos - (default:/usr/local/share/codespell/dictionary.txt) + (default:/usr/share/codespell/dictionary.txt) --codespellfile Use this codespell dictionary + --color Use colors when output is STDOUT (default: on) -h, --help, --version display this help and exit When FILE is - read standard input. @@ -134,6 +139,7 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, + 'showfile!' => \$showfile, 'f|file!' => \$file, 'subjective!' => \$check, 'strict!' => \$check, @@ -153,6 +159,7 @@ GetOptions( 'test-only=s' => \$tst_only, 'codespell!' => \$codespell, 'codespellfile=s' => \$codespellfile, + 'color!' => \$color, 'h|help' => \$help, 'version' => \$help ) or help(1); @@ -196,12 +203,12 @@ sub hash_save_array_words { sub hash_show_words { my ($hashRef, $prefix) = @_; - if ($quiet == 0 && keys %$hashRef) { - print "NOTE: $prefix message types:"; + if (keys %$hashRef) { + print "\nNOTE: $prefix message types:"; foreach my $word (sort keys %$hashRef) { print " $word"; } - print "\n\n"; + print "\n"; } } @@ -347,15 +354,20 @@ our $UTF8 = qr{ | $NON_ASCII_UTF8 }x; +our $typeC99Typedefs = qr{(?:__)?(?:[us]_?)?int_?(?:8|16|32|64)_t}; our $typeOtherOSTypedefs = qr{(?x: u_(?:char|short|int|long) | # bsd u(?:nchar|short|int|long) # sysv )}; - -our $typeTypedefs = qr{(?x: +our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeTypedefs = qr{(?x: + $typeC99Typedefs\b| + $typeOtherOSTypedefs\b| + $typeKernelTypedefs\b +)}; our $logFunctions = qr{(?x: printk(?:_ratelimited|_once|)| @@ -418,6 +430,7 @@ our @typeList = ( qr{${Ident}_handler_fn}, @typeListMisordered, ); +our @typeListFile = (); our @typeListWithAttr = ( @typeList, qr{struct\s+$InitAttribute\s+$Ident}, @@ -427,6 +440,7 @@ our @typeListWithAttr = ( our @modifierList = ( qr{fastcall}, ); +our @modifierListFile = (); our @mode_permission_funcs = ( ["module_param", 3], @@ -510,13 +524,12 @@ if ($codespell) { $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix; sub build_types { - my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)"; - my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)"; + my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; + my $all = "(?x: \n" . join("|\n ", (@typeList, @typeListFile)) . "\n)"; my $Misordered = "(?x: \n" . join("|\n ", @typeListMisordered) . "\n)"; my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)"; $Modifier = qr{(?:$Attribute|$Sparse|$mods)}; $BasicType = qr{ - (?:$typeOtherOSTypedefs\b)| (?:$typeTypedefs\b)| (?:${all}\b) }x; @@ -524,7 +537,6 @@ sub build_types { (?:$Modifier\s+|const\s+)* (?: (?:typeof|__typeof__)\s*\([^\)]*\)| - (?:$typeOtherOSTypedefs\b)| (?:$typeTypedefs\b)| (?:${all}\b) ) @@ -542,7 +554,6 @@ sub build_types { (?: (?:typeof|__typeof__)\s*\([^\)]*\)| (?:$typeTypedefs\b)| - (?:$typeOtherOSTypedefs\b)| (?:${allWithAttr}\b) ) (?:\s+$Modifier|\s+const)* @@ -737,6 +748,13 @@ for my $filename (@ARGV) { push(@rawlines, $_); } close($FILE); + + if ($#ARGV > 0 && $quiet == 0) { + print '-' x length($vname) . "\n"; + print "$vname\n"; + print '-' x length($vname) . "\n"; + } + if (!process($filename)) { $exit = 1; } @@ -746,6 +764,29 @@ for my $filename (@ARGV) { @fixed_inserted = (); @fixed_deleted = (); $fixlinenr = -1; + @modifierListFile = (); + @typeListFile = (); + build_types(); +} + +if (!$quiet) { + hash_show_words(\%use_type, "Used"); + hash_show_words(\%ignore_type, "Ignored"); + + if ($^V lt 5.10.0) { + print << "EOM" + +NOTE: perl $^V is not modern enough to detect all possible issues. + An upgrade to at least perl v5.10.0 is suggested. +EOM + } + if ($exit) { + print << "EOM" + +NOTE: If any of the errors are false positives, please report + them to the maintainer, see CHECKPATCH in MAINTAINERS. +EOM + } } exit($exit); @@ -1001,7 +1042,7 @@ sub sanitise_line { sub get_quoted_string { my ($line, $rawline) = @_; - return "" if ($line !~ m/(\"[X\t]+\")/g); + return "" if ($line !~ m/($String)/g); return substr($rawline, $-[0], $+[0] - $-[0]); } @@ -1610,13 +1651,13 @@ sub possible { for my $modifier (split(' ', $possible)) { if ($modifier !~ $notPermitted) { warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible); - push(@modifierList, $modifier); + push(@modifierListFile, $modifier); } } } else { warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible); - push(@typeList, $possible); + push(@typeListFile, $possible); } build_types(); } else { @@ -1641,15 +1682,32 @@ sub report { (defined $tst_only && $msg !~ /\Q$tst_only\E/)) { return 0; } - my $line; + my $output = ''; + if (-t STDOUT && $color) { + if ($level eq 'ERROR') { + $output .= RED; + } elsif ($level eq 'WARNING') { + $output .= YELLOW; + } else { + $output .= GREEN; + } + } + $output .= $prefix . $level . ':'; if ($show_types) { - $line = "$prefix$level:$type: $msg\n"; - } else { - $line = "$prefix$level: $msg\n"; + $output .= BLUE if (-t STDOUT && $color); + $output .= "$type:"; } - $line = (split('\n', $line))[0] . "\n" if ($terse); + $output .= RESET if (-t STDOUT && $color); + $output .= ' ' . $msg . "\n"; - push(our @report, $line); + if ($showfile) { + my @lines = split("\n", $output, -1); + splice(@lines, 1, 1); + $output = join("\n", @lines); + } + $output = (split('\n', $output))[0] . "\n" if ($terse); + + push(our @report, $output); return 1; } @@ -1899,6 +1957,7 @@ sub process { my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch my $commit_log_long_line = 0; + my $commit_log_has_diff = 0; my $reported_maintainer_file = 0; my $non_utf8_charset = 0; @@ -2032,7 +2091,8 @@ sub process { my $rawline = $rawlines[$linenr - 1]; #extract the line range in the file after the patch is applied - if ($line=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { + if (!$in_commit_log && + $line =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { $is_patch = 1; $first_line = $linenr + 1; $realline=$1-1; @@ -2073,10 +2133,6 @@ sub process { my $hunk_line = ($realcnt != 0); -#make up the handle for any error we report on this line - $prefix = "$filename:$realline: " if ($emacs && $file); - $prefix = "$filename:$linenr: " if ($emacs && !$file); - $here = "#$linenr: " if (!$file); $here = "#$realline: " if ($file); @@ -2106,6 +2162,13 @@ sub process { $found_file = 1; } +#make up the handle for any error we report on this line + if ($showfile) { + $prefix = "$realfile:$realline: " + } elsif ($emacs) { + $prefix = "$filename:$linenr: "; + } + if ($found_file) { if ($realfile =~ m@^(drivers/net/|net/)@) { $check = 1; @@ -2123,6 +2186,17 @@ sub process { $cnt_lines++ if ($realcnt != 0); +# Check if the commit log has what seems like a diff which can confuse patch + if ($in_commit_log && !$commit_log_has_diff && + (($line =~ m@^\s+diff\b.*a/[\w/]+@ && + $line =~ m@^\s+diff\b.*a/([\w/]+)\s+b/$1\b@) || + $line =~ m@^\s*(?:\-\-\-\s+a/|\+\+\+\s+b/)@ || + $line =~ m/^\s*\@\@ \-\d+,\d+ \+\d+,\d+ \@\@/)) { + ERROR("DIFF_IN_COMMIT_MSG", + "Avoid using diff content in the commit message - patch(1) might not work\n" . $herecurr); + $commit_log_has_diff = 1; + } + # Check for incorrect file permissions if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { my $permhere = $here . "FILE: $realfile\n"; @@ -2510,16 +2584,56 @@ sub process { # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|pl|sh|dtsi|dts)$/); -#line length limit - if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && - $rawline !~ /^.\s*\*\s*\@$Ident\s/ && - !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?$String\s*(?:|,|\)\s*;)\s*$/ || - $line =~ /^\+\s*$String\s*(?:\s*|,|\)\s*;)\s*$/ || - $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) && - $length > $max_line_length) - { - WARN("LONG_LINE", - "line over $max_line_length characters\n" . $herecurr); +# line length limit (with some exclusions) +# +# There are a few types of lines that may extend beyond $max_line_length: +# logging functions like pr_info that end in a string +# lines with a single string +# #defines that are a single string +# +# There are 3 different line length message types: +# LONG_LINE_COMMENT a comment starts before but extends beyond $max_linelength +# LONG_LINE_STRING a string starts before but extends beyond $max_line_length +# LONG_LINE all other lines longer than $max_line_length +# +# if LONG_LINE is ignored, the other 2 types are also ignored +# + + if ($line =~ /^\+/ && $length > $max_line_length) { + my $msg_type = "LONG_LINE"; + + # Check the allowed long line types first + + # logging functions that end in a string that starts + # before $max_line_length + if ($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(?:KERN_\S+\s*|[^"]*))?($String\s*(?:|,|\)\s*;)\s*)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = ""; + + # lines with only strings (w/ possible termination) + # #defines with only strings + } elsif ($line =~ /^\+\s*$String\s*(?:\s*|,|\)\s*;)\s*$/ || + $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) { + $msg_type = ""; + + # Otherwise set the alternate message types + + # a comment starts before $max_line_length + } elsif ($line =~ /($;[\s$;]*)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = "LONG_LINE_COMMENT" + + # a quoted string starts before $max_line_length + } elsif ($sline =~ /\s*($String(?:\s*(?:\\|,\s*|\)\s*;\s*))?)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = "LONG_LINE_STRING" + } + + if ($msg_type ne "" && + (show_type("LONG_LINE") || show_type($msg_type))) { + WARN($msg_type, + "line over $max_line_length characters\n" . $herecurr); + } } # check for adding lines without a newline. @@ -3264,7 +3378,6 @@ sub process { $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ && $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ && $line !~ /\b$typeTypedefs\b/ && - $line !~ /\b$typeOtherOSTypedefs\b/ && $line !~ /\b__bitwise(?:__|)\b/) { WARN("NEW_TYPEDEFS", "do not add new typedefs\n" . $herecurr); @@ -4337,8 +4450,8 @@ sub process { } # Flatten any obvious string concatentation. - while ($dstat =~ s/("X*")\s*$Ident/$1/ || - $dstat =~ s/$Ident\s*("X*")/$1/) + while ($dstat =~ s/($String)\s*$Ident/$1/ || + $dstat =~ s/$Ident\s*($String)/$1/) { } @@ -4619,7 +4732,7 @@ sub process { # to grep for the string. Make exceptions when the previous string ends in a # newline (multiple lines in one string constant) or '\t', '\r', ';', or '{' # (common in inline assembly) or is a octal \123 or hexadecimal \xaf value - if ($line =~ /^\+\s*"[X\t]*"/ && + if ($line =~ /^\+\s*$String/ && $prevline =~ /"\s*$/ && $prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) { if (WARN("SPLIT_STRING", @@ -4665,13 +4778,13 @@ sub process { } # concatenated string without spaces between elements - if ($line =~ /"X+"[A-Z_]+/ || $line =~ /[A-Z_]+"X+"/) { + if ($line =~ /$String[A-Z_]/ || $line =~ /[A-Za-z0-9_]$String/) { CHK("CONCATENATED_STRING", "Concatenated strings should use spaces between elements\n" . $herecurr); } # uncoalesced string fragments - if ($line =~ /"X*"\s*"/) { + if ($line =~ /$String\s*"/) { WARN("STRING_FRAGMENTS", "Consecutive strings are generally better as a single string\n" . $herecurr); } @@ -4898,6 +5011,13 @@ 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)) { + WARN("WAITQUEUE_ACTIVE", + "waitqueue_active without comment\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", @@ -4973,6 +5093,24 @@ sub process { "Using weak declarations can have unintended link defects\n" . $herecurr); } +# check for c99 types like uint8_t used outside of uapi/ + if ($realfile !~ m@\binclude/uapi/@ && + $line =~ /\b($Declare)\s*$Ident\s*[=;,\[]/) { + my $type = $1; + if ($type =~ /\b($typeC99Typedefs)\b/) { + $type = $1; + my $kernel_type = 'u'; + $kernel_type = 's' if ($type =~ /^_*[si]/); + $type =~ /(\d+)/; + $kernel_type .= $1; + if (CHK("PREFER_KERNEL_TYPES", + "Prefer kernel type '$kernel_type' over '$type'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$type\b/$kernel_type/; + } + } + } + # check for sizeof(&) if ($line =~ /\bsizeof\s*\(\s*\&/) { WARN("SIZEOF_ADDRESS", @@ -5010,7 +5148,7 @@ sub process { # Check for misused memsets if ($^V && $^V ge 5.10.0 && defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/) { my $ms_addr = $2; my $ms_val = $7; @@ -5027,14 +5165,46 @@ sub process { # Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) if ($^V && $^V ge 5.10.0 && - $line =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) { + defined $stat && + $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { if (WARN("PREFER_ETHER_ADDR_COPY", - "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . $herecurr) && + "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && $fix) { $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; } } +# Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { + WARN("PREFER_ETHER_ADDR_EQUAL", + "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") + } + +# check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr +# check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { + + my $ms_val = $7; + + if ($ms_val =~ /^(?:0x|)0+$/i) { + if (WARN("PREFER_ETH_ZERO_ADDR", + "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && + $fix) { + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; + } + } elsif ($ms_val =~ /^(?:0xff|255)$/i) { + if (WARN("PREFER_ETH_BROADCAST_ADDR", + "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && + $fix) { + $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; + } + } + } + # typecasts on min/max could be min_t/max_t if ($^V && $^V ge 5.10.0 && defined $stat && @@ -5472,6 +5642,24 @@ sub process { } } } + +# validate content of MODULE_LICENSE against list from include/linux/module.h + if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) { + my $extracted_string = get_quoted_string($line, $rawline); + my $valid_licenses = qr{ + GPL| + GPL\ v2| + GPL\ and\ additional\ rights| + Dual\ BSD/GPL| + Dual\ MIT/GPL| + Dual\ MPL/GPL| + Proprietary + }x; + if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) { + WARN("MODULE_LICENSE", + "unknown module license " . $extracted_string . "\n" . $herecurr); + } + } } # If we have no input at all, then there is nothing to report on @@ -5492,11 +5680,11 @@ sub process { exit(0); } - if (!$is_patch) { + if (!$is_patch && $file !~ /cover-letter\.patch$/) { ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } - if ($is_patch && $chk_signoff && $signoff == 0) { + if ($is_patch && $filename ne '-' && $chk_signoff && $signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); } @@ -5507,28 +5695,21 @@ sub process { print "total: $cnt_error errors, $cnt_warn warnings, " . (($check)? "$cnt_chk checks, " : "") . "$cnt_lines lines checked\n"; - print "\n" if ($quiet == 0); } if ($quiet == 0) { - - if ($^V lt 5.10.0) { - print("NOTE: perl $^V is not modern enough to detect all possible issues.\n"); - print("An upgrade to at least perl v5.10.0 is suggested.\n\n"); - } - # If there were whitespace errors which cleanpatch can fix # then suggest that. if ($rpt_cleaners) { - print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n"; - print " scripts/cleanfile\n\n"; $rpt_cleaners = 0; + print << "EOM" + +NOTE: Whitespace errors detected. + You may wish to use scripts/cleanpatch or scripts/cleanfile +EOM } } - hash_show_words(\%use_type, "Used"); - hash_show_words(\%ignore_type, "Ignored"); - if ($clean == 0 && $fix && ("@rawlines" ne "@fixed" || $#fixed_inserted >= 0 || $#fixed_deleted >= 0)) { @@ -5556,6 +5737,7 @@ sub process { if (!$quiet) { print << "EOM"; + Wrote EXPERIMENTAL --fix correction(s) to '$newfile' Do _NOT_ trust the results written to this file. @@ -5563,22 +5745,17 @@ Do _NOT_ submit these changes without inspecting them for correctness. This EXPERIMENTAL file is simply a convenience to help rewrite patches. No warranties, expressed or implied... - EOM } } - if ($clean == 1 && $quiet == 0) { - print "$vname has no obvious style problems and is ready for submission.\n" - } - if ($clean == 0 && $quiet == 0) { - print << "EOM"; -$vname has style problems, please review. - -If any of these errors are false positives, please report -them to the maintainer, see CHECKPATCH in MAINTAINERS. -EOM + if ($quiet == 0) { + print "\n"; + if ($clean == 1) { + print "$vname has no obvious style problems and is ready for submission.\n"; + } else { + print "$vname has style problems, please review.\n"; + } } - return $clean; } |