X-Git-Url: http://wpitchoune.net/gitweb/?p=psensor-pkg-debian.git;a=blobdiff_plain;f=tests%2Fcheckpatch.pl;h=4904ced676d40289356aa3358f894fa7efa4b5c0;hp=cab1691b5ad6cc683a511205c2fa7ec95aec61ae;hb=bd7036af56a4b61b2b473dccbeac5e26f78b57da;hpb=48da1ac5531838d157762882ee455cec4da05728 diff --git a/tests/checkpatch.pl b/tests/checkpatch.pl index cab1691..4904ced 100755 --- a/tests/checkpatch.pl +++ b/tests/checkpatch.pl @@ -7,9 +7,12 @@ use strict; use POSIX; +use File::Basename; +use Cwd 'abs_path'; +use Term::ANSIColor qw(:constants); my $P = $0; -$P =~ s@.*/@@g; +my $D = dirname(abs_path($P)); my $V = '0.32'; @@ -22,13 +25,17 @@ my $chk_patch = 1; my $tst_only; my $emacs = 0; my $terse = 0; +my $showfile = 0; my $file = 0; +my $git = 0; +my %git_commits = (); my $check = 0; my $check_orig = 0; my $summary = 1; my $mailback = 0; my $summary_file = 0; my $show_types = 0; +my $list_types = 0; my $fix = 0; my $fix_inplace = 0; my $root; @@ -43,6 +50,11 @@ my $configuration_file = ".checkpatch.conf"; my $max_line_length = 80; my $ignore_perl_version = 0; 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/share/codespell/dictionary.txt"; +my $color = 1; sub help { my ($exitcode) = @_; @@ -58,12 +70,25 @@ 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 + -g, --git treat FILE as a single commit or git revision range + single git commit with: + + ^ + ~n + multiple git commits with: + .. + ... + - + git merges are ignored -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests + --list-types list the possible message types --types TYPE(,TYPE2...) show only these comma separated message types --ignore TYPE(,TYPE2...) ignore various comma separated message types + --show-types show the specific message type in the output --max-line-length=n set the maximum line length, if exceeded, warn - --show-types show the message "types" in the output + --min-conf-desc-length=n set the min description length, if shorter, warn --root=PATH PATH to the kernel tree root --no-summary suppress the per-file summary --mailback only produce a report in case of warnings/errors @@ -83,6 +108,10 @@ Options: file. It's your fault if there's no backup or git --ignore-perl-version override checking of perl version. expect runtime errors. + --codespell Use the codespell dictionary for spelling/typos + (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. @@ -91,6 +120,37 @@ EOM exit($exitcode); } +sub uniq { + my %seen; + return grep { !$seen{$_}++ } @_; +} + +sub list_types { + my ($exitcode) = @_; + + my $count = 0; + + local $/ = undef; + + open(my $script, '<', abs_path($P)) or + die "$P: Can't read '$P' $!\n"; + + my $text = <$script>; + close($script); + + my @types = (); + for ($text =~ /\b(?:(?:CHK|WARN|ERROR)\s*\(\s*"([^"]+)")/g) { + push (@types, $_); + } + @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); + foreach my $type (@types) { + print(++$count . "\t" . $type . "\n"); + } + + exit($exitcode); +} + my $conf = which_conf($configuration_file); if (-f $conf) { my @conf_args; @@ -124,13 +184,17 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, + 'showfile!' => \$showfile, 'f|file!' => \$file, + 'g|git!' => \$git, 'subjective!' => \$check, 'strict!' => \$check, 'ignore=s' => \@ignore, 'types=s' => \@use, 'show-types!' => \$show_types, + 'list-types!' => \$list_types, 'max-line-length=i' => \$max_line_length, + 'min-conf-desc-length=i' => \$min_conf_desc_length, 'root=s' => \$root, 'summary!' => \$summary, 'mailback!' => \$mailback, @@ -140,12 +204,17 @@ GetOptions( 'ignore-perl-version!' => \$ignore_perl_version, 'debug=s' => \%debug, 'test-only=s' => \$tst_only, + 'codespell!' => \$codespell, + 'codespellfile=s' => \$codespellfile, + 'color!' => \$color, 'h|help' => \$help, 'version' => \$help ) or help(1); help(0) if ($help); +list_types(0) if ($list_types); + $fix = 1 if ($fix_inplace); $check_orig = $check; @@ -183,12 +252,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"; } } @@ -244,11 +313,13 @@ our $Sparse = qr{ __kernel| __force| __iomem| + __pmem| __must_check| __init_refok| __kprobes| __ref| - __rcu + __rcu| + __private }x; our $InitAttributePrefix = qr{__(?:mem|cpu|dev|net_|)}; our $InitAttributeData = qr{$InitAttributePrefix(?:initdata\b)}; @@ -272,6 +343,7 @@ our $Attribute = qr{ __noreturn| __used| __cold| + __pure| __noclone| __deprecated| __read_mostly| @@ -292,6 +364,7 @@ our $Binary = qr{(?i)0b[01]+$Int_type?}; our $Hex = qr{(?i)0x[0-9a-f]+$Int_type?}; our $Int = qr{[0-9]+$Int_type?}; our $Octal = qr{0[0-7]+$Int_type?}; +our $String = qr{"[X\t]*"}; our $Float_hex = qr{(?i)0x[0-9a-f]+p-?[0-9]+[fl]?}; our $Float_dec = qr{(?i)(?:[0-9]+\.[0-9]*|[0-9]*\.[0-9]+)(?:e-?[0-9]+)?[fl]?}; our $Float_int = qr{(?i)[0-9]+e-?[0-9]+[fl]?}; @@ -308,6 +381,7 @@ our $Operators = qr{ our $c90_Keywords = qr{do|for|while|if|else|return|goto|continue|switch|default|case|break}x; +our $BasicType; our $NonptrType; our $NonptrTypeMisordered; our $NonptrTypeWithAttr; @@ -331,10 +405,22 @@ our $UTF8 = qr{ | $NON_ASCII_UTF8 }x; -our $typeTypedefs = qr{(?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 $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeTypedefs = qr{(?x: + $typeC99Typedefs\b| + $typeOtherOSTypedefs\b| + $typeKernelTypedefs\b +)}; + +our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b}; our $logFunctions = qr{(?x: printk(?:_ratelimited|_once|)| @@ -397,6 +483,29 @@ our @typeList = ( qr{${Ident}_handler_fn}, @typeListMisordered, ); + +our $C90_int_types = qr{(?x: + long\s+long\s+int\s+(?:un)?signed| + long\s+long\s+(?:un)?signed\s+int| + long\s+long\s+(?:un)?signed| + (?:(?:un)?signed\s+)?long\s+long\s+int| + (?:(?:un)?signed\s+)?long\s+long| + int\s+long\s+long\s+(?:un)?signed| + int\s+(?:(?:un)?signed\s+)?long\s+long| + + long\s+int\s+(?:un)?signed| + long\s+(?:un)?signed\s+int| + long\s+(?:un)?signed| + (?:(?:un)?signed\s+)?long\s+int| + (?:(?:un)?signed\s+)?long| + int\s+long\s+(?:un)?signed| + int\s+(?:(?:un)?signed\s+)?long| + + int\s+(?:un)?signed| + (?:(?:un)?signed\s+)?int +)}; + +our @typeListFile = (); our @typeListWithAttr = ( @typeList, qr{struct\s+$InitAttribute\s+$Ident}, @@ -406,6 +515,7 @@ our @typeListWithAttr = ( our @modifierList = ( qr{fastcall}, ); +our @modifierListFile = (); our @mode_permission_funcs = ( ["module_param", 3], @@ -423,18 +533,81 @@ foreach my $entry (@mode_permission_funcs) { $mode_perms_search .= $entry->[0]; } +our $mode_perms_world_writable = qr{ + S_IWUGO | + S_IWOTH | + S_IRWXUGO | + S_IALLUGO | + 0[0-7][0-7][2367] +}x; + our $allowed_asm_includes = qr{(?x: irq| - memory + memory| + time| + reboot )}; # memory.h: ARM has a custom one +# Load common spelling mistakes and build regular expression list. +my $misspellings; +my %spelling_fix; + +if (open(my $spelling, '<', $spelling_file)) { + while (<$spelling>) { + my $line = $_; + + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; + + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); + + my ($suspect, $fix) = split(/\|\|/, $line); + + $spelling_fix{$suspect} = $fix; + } + close($spelling); +} else { + warn "No typos will be found - file '$spelling_file': $!\n"; +} + +if ($codespell) { + if (open(my $spelling, '<', $codespellfile)) { + while (<$spelling>) { + my $line = $_; + + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; + + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); + next if ($line =~ m/, disabled/i); + + $line =~ s/,.*$//; + + my ($suspect, $fix) = split(/->/, $line); + + $spelling_fix{$suspect} = $fix; + } + close($spelling); + } else { + warn "No codespell typos will be found - file '$codespellfile': $!\n"; + } +} + +$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{ + (?:$typeTypedefs\b)| + (?:${all}\b) + }x; $NonptrType = qr{ (?:$Modifier\s+|const\s+)* (?: @@ -483,10 +656,10 @@ our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*}; our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/; our $LvalOrFunc = qr{((?:[\&\*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*}; -our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; +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*\( )}; @@ -598,6 +771,8 @@ sub git_commit_info { $output =~ s/^\s*//gm; my @lines = split("\n", $output); + return ($id, $desc) if ($#lines < 0); + if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous\./) { # Maybe one day convert this block of bash into something that returns # all matching commit ids, but it's very slow... @@ -626,10 +801,42 @@ my @fixed_inserted = (); my @fixed_deleted = (); my $fixlinenr = -1; +# If input is git commits, extract all commits from the commit expressions. +# For example, HEAD-3 means we need check 'HEAD, HEAD~1, HEAD~2'. +die "$P: No git repository found\n" if ($git && !-e ".git"); + +if ($git) { + my @commits = (); + foreach my $commit_expr (@ARGV) { + my $git_range; + if ($commit_expr =~ m/^(.*)-(\d+)$/) { + $git_range = "-$2 $1"; + } elsif ($commit_expr =~ m/\.\./) { + $git_range = "$commit_expr"; + } else { + $git_range = "-1 $commit_expr"; + } + my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; + foreach my $line (split(/\n/, $lines)) { + $line =~ /^([0-9a-fA-F]{40,40}) (.*)$/; + next if (!defined($1) || !defined($2)); + my $sha1 = $1; + my $subject = $2; + unshift(@commits, $sha1); + $git_commits{$sha1} = $subject; + } + } + die "$P: no git commits after extraction!\n" if (@commits == 0); + @ARGV = @commits; +} + my $vname; for my $filename (@ARGV) { my $FILE; - if ($file) { + if ($git) { + open($FILE, '-|', "git format-patch -M --stdout -1 $filename") || + die "$P: $filename: git format-patch failed - $!\n"; + } elsif ($file) { open($FILE, '-|', "diff -u /dev/null $filename") || die "$P: $filename: diff failed - $!\n"; } elsif ($filename eq '-') { @@ -640,6 +847,8 @@ for my $filename (@ARGV) { } if ($filename eq '-') { $vname = 'Your patch'; + } elsif ($git) { + $vname = "Commit " . substr($filename, 0, 12) . ' ("' . $git_commits{$filename} . '")'; } else { $vname = $filename; } @@ -648,6 +857,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; } @@ -657,6 +873,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); @@ -912,7 +1151,7 @@ sub sanitise_line { sub get_quoted_string { my ($line, $rawline) = @_; - return "" if ($line !~ m/(\"[X]+\")/g); + return "" if ($line !~ m/($String)/g); return substr($rawline, $-[0], $+[0] - $-[0]); } @@ -1521,13 +1760,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 { @@ -1552,15 +1791,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:"; + } + $output .= RESET if (-t STDOUT && $color); + $output .= ' ' . $msg . "\n"; + + if ($showfile) { + my @lines = split("\n", $output, -1); + splice(@lines, 1, 1); + $output = join("\n", @lines); } - $line = (split('\n', $line))[0] . "\n" if ($terse); + $output = (split('\n', $output))[0] . "\n" if ($terse); - push(our @report, $line); + push(our @report, $output); return 1; } @@ -1601,7 +1857,7 @@ sub fix_inserted_deleted_lines { foreach my $old_line (@{$linesRef}) { my $save_line = 1; my $line = $old_line; #don't modify the array - if ($line =~ /^(?:\+\+\+\|\-\-\-)\s+\S+/) { #new filename + if ($line =~ /^(?:\+\+\+|\-\-\-)\s+\S+/) { #new filename $delta_offset = 0; } elsif ($line =~ /^\@\@ -\d+,\d+ \+\d+,\d+ \@\@/) { #new hunk $range_last_linenr = $new_linenr; @@ -1806,13 +2062,16 @@ 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; my $non_utf8_charset = 0; my $last_blank_line = 0; + my $last_coalesced_string_linenr = -1; our @report = (); our $cnt_lines = 0; @@ -1941,7 +2200,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; @@ -1982,10 +2242,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); @@ -2015,8 +2271,19 @@ sub process { $found_file = 1; } +#make up the handle for any error we report on this line + if ($showfile) { + $prefix = "$realfile:$realline: " + } elsif ($emacs) { + 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; @@ -2032,6 +2299,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"; @@ -2048,6 +2326,12 @@ sub process { $in_commit_log = 0; } +# Check if MAINTAINERS is being updated. If so, there's probably no need to +# emit the "does MAINTAINERS need updating?" message on file add/move/delete + if ($line =~ /^\s*MAINTAINERS\s*\|/) { + $reported_maintainer_file = 1; + } + # Check signature styles if (!$in_header_lines && $line =~ /^(\s*)([a-z0-9_-]+by:|$signature_tags)(\s*)(.*)/i) { @@ -2118,6 +2402,13 @@ sub process { } } +# Check email subject for common tools that don't need to be mentioned + if ($in_header_lines && + $line =~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b[^:]/i) { + WARN("EMAIL_SUBJECT", + "A patch subject line should describe the change not the tool that found it\n" . $herecurr); + } + # Check for old stable address if ($line =~ /^\s*cc:\s*.*?.*$/i) { ERROR("STABLE_ADDRESS", @@ -2130,21 +2421,92 @@ sub process { "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); } -# Check for improperly formed commit descriptions - if ($in_commit_log && - $line =~ /\bcommit\s+[0-9a-f]{5,}/i && - !($line =~ /\b[Cc]ommit [0-9a-f]{12,40} \("/ || - ($line =~ /\b[Cc]ommit [0-9a-f]{12,40}\s*$/ && - defined $rawlines[$linenr] && - $rawlines[$linenr] =~ /^\s*\("/))) { - $line =~ /\b(c)ommit\s+([0-9a-f]{5,})/i; - my $init_char = $1; - my $orig_commit = lc($2); - my $id = '01234567890ab'; - my $desc = 'commit description'; - ($id, $desc) = git_commit_info($orig_commit, $id, $desc); - ERROR("GIT_COMMIT_ID", - "Please use 12 or more chars for the git commit ID like: '${init_char}ommit $id (\"$desc\")'\n" . $herecurr); +# 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; + } + +# Check for line lengths > 75 in commit log, warn once + if ($in_commit_log && !$commit_log_long_line && + 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; + } + +# 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 && !$commit_log_possible_stack_dump && + $line !~ /^\s*(?:Link|Patchwork|http|BugLink):/i && + ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + ($line =~ /\b[0-9a-f]{12,40}\b/i && + $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/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; + my $space = 1; + my $hasdesc = 0; + my $hasparens = 0; + my $id = '0123456789ab'; + 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); + $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); + if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) { + $orig_desc = $1; + $hasparens = 1; + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i && + defined $rawlines[$linenr] && + $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) { + $orig_desc = $1; + $hasparens = 1; + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i && + defined $rawlines[$linenr] && + $rawlines[$linenr] =~ /^\s*[^"]+"\)/) { + $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i; + $orig_desc = $1; + $rawlines[$linenr] =~ /^\s*([^"]+)"\)/; + $orig_desc .= " " . $1; + $hasparens = 1; + } + + ($id, $description) = git_commit_info($orig_commit, + $id, $orig_desc); + + if ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens) { + ERROR("GIT_COMMIT_ID", + "Please use git commit description style 'commit <12+ chars of sha1> (\"\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herecurr); + } } # Check for added, moved or deleted files @@ -2215,6 +2577,24 @@ sub process { "8-bit UTF-8 used in possible commit log\n" . $herecurr); } +# Check for various typo / spelling mistakes + if (defined($misspellings) && + ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { + while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) { + my $typo = $1; + my $typo_fix = $spelling_fix{lc($typo)}; + $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/); + $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/); + my $msg_type = \&WARN; + $msg_type = \&CHK if ($file); + if (&{$msg_type}("TYPO_SPELLING", + "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/; + } + } + } + # ignore non-hunk lines and lines being removed next if (!$hunk_line || $line =~ /^-/); @@ -2283,8 +2663,10 @@ sub process { } $length++; } - WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($is_start && $is_end && $length < 4); + if ($is_start && $is_end && $length < $min_conf_desc_length) { + WARN("CONFIG_DESCRIPTION", + "please write a paragraph that describes the config symbol fully\n" . $herecurr); + } #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } @@ -2295,6 +2677,13 @@ sub process { "Use of CONFIG_EXPERIMENTAL is deprecated. For alternatives, see https://lkml.org/lkml/2012/10/23/580\n"); } +# discourage the use of boolean for type definition attributes of Kconfig options + if ($realfile =~ /Kconfig/ && + $line =~ /^\+\s*\bboolean\b/) { + WARN("CONFIG_TYPE_BOOLEAN", + "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); + } + if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { my $flag = $1; @@ -2341,44 +2730,58 @@ 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)$/); - -#line length limit - if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && - $rawline !~ /^.\s*\*\s*\@$Ident\s/ && - !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?"[X\t]*"\s*(?:|,|\)\s*;)\s*$/ || - $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) && - $length > $max_line_length) - { - WARN("LONG_LINE", - "line over $max_line_length characters\n" . $herecurr); - } + next if ($realfile !~ /\.(h|c|s|S|pl|sh|dtsi|dts)$/); -# Check for user-visible strings broken across lines, which breaks the ability -# 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*"/ && - $prevline =~ /"\s*$/ && - $prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) { - WARN("SPLIT_STRING", - "quoted string split across lines\n" . $hereprev); - } +# 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 +# -# check for missing a space in a string concatination - if ($prevrawline =~ /[^\\]\w"$/ && $rawline =~ /^\+[\t ]+"\w/) { - WARN('MISSING_SPACE', - "break quoted strings at a space character\n" . $hereprev); - } + if ($line =~ /^\+/ && $length > $max_line_length) { + my $msg_type = "LONG_LINE"; -# check for spaces before a quoted newline - if ($rawline =~ /^.*\".*\s\\n/) { - if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", - "unnecessary whitespace before a quoted newline\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/^(\+.*\".*)\s+\\n/$1\\n/; + # 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. @@ -2402,7 +2805,7 @@ sub process { } # check we are in a valid source file C or perl if not then ignore this hunk - next if ($realfile !~ /\.(h|c|pl)$/); + next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/); # at the beginning of a line any tabs must come first and anything # more than 8 must use tabs. @@ -2436,6 +2839,19 @@ sub process { "Logical continuations should be on the previous line\n" . $hereprev); } +# check indentation starts on a tab stop + if ($^V && $^V ge 5.10.0 && + $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$))/) { + my $indent = length($1); + if ($indent % 8) { + if (WARN("TABSTOP", + "Statements should start on a tabstop\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . "\t" x ($indent/8)@e; + } + } + } + # check multi-line statement indentation matches previous line if ($^V && $^V ge 5.10.0 && $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|$Ident\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { @@ -2466,7 +2882,15 @@ sub process { } } - if ($line =~ /^\+.*\(\s*$Type\s*\)[ \t]+(?!$Assignment|$Arithmetic|{)/) { +# check for space after cast like "(int) foo" or "(struct foo) bar" +# avoid checking a few false positives: +# "sizeof(<type>)" or "__alignof__(<type>)" +# function pointer declarations like "(*foo)(int) = bar;" +# structure definitions like "(struct foo) { 0 };" +# multiline macros that define functions +# known attributes or the __attribute__ keyword + if ($line =~ /^\+(.*)\(\s*$Type\s*\)([ \t]++)((?![={]|\\$|$Attribute|__attribute__))/ && + (!defined($1) || $1 !~ /\b(?:sizeof|__alignof__)\s*$/)) { if (CHK("SPACING", "No space is necessary after a cast\n" . $herecurr) && $fix) { @@ -2475,6 +2899,8 @@ sub process { } } +# Block comment styles +# Networking with an initial /* if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ && $rawline =~ /^\+[ \t]*\*/ && @@ -2483,22 +2909,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 @@ -2592,10 +3019,14 @@ sub process { next if ($realfile !~ /\.(h|c)$/); # check indentation of any line with a bare else +# (but not if it is a multiple line "if (foo) return bar; else return baz;") # if the previous line is a break or return and is indented 1 tab more... if ($sline =~ /^\+([\t]+)(?:}[ \t]*)?else(?:[ \t]*{)?\s*$/) { my $tabs = length($1) + 1; - if ($prevline =~ /^\+\t{$tabs,$tabs}(?:break|return)\b/) { + if ($prevline =~ /^\+\t{$tabs,$tabs}break\b/ || + ($prevline =~ /^\+\t{$tabs,$tabs}return\b/ && + defined $lines[$linenr] && + $lines[$linenr] !~ /^[ \+]\t{$tabs,$tabs}return/)) { WARN("UNNECESSARY_ELSE", "else is not generally useful after a break or return\n" . $hereprev); } @@ -2800,15 +3231,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 @@ -2874,8 +3312,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"); } @@ -2897,6 +3337,30 @@ sub process { #ignore lines not being added next if ($line =~ /^[^\+]/); +# check for declarations of signed or unsigned without int + while ($line =~ m{($Declare)\s*(?!char\b|short\b|int\b|long\b)\s*($Ident)?\s*[=,;\[\)\(]}g) { + my $type = $1; + my $var = $2; + $var = "" if (!defined $var); + if ($type =~ /^(?:(?:$Storage|$Inline|$Attribute)\s+)*((?:un)?signed)((?:\s*\*)*)\s*$/) { + my $sign = $1; + my $pointer = $2; + + $pointer = "" if (!defined $pointer); + + if (WARN("UNSPECIFIED_INT", + "Prefer '" . trim($sign) . " int" . rtrim($pointer) . "' to bare use of '$sign" . rtrim($pointer) . "'\n" . $herecurr) && + $fix) { + my $decl = trim($sign) . " int "; + my $comp_pointer = $pointer; + $comp_pointer =~ s/\s//g; + $decl .= $comp_pointer; + $decl = rtrim($decl) if ($var eq ""); + $fixed[$fixlinenr] =~ s@\b$sign\s*\Q$pointer\E\s*$var\b@$decl$var@; + } + } + } + # TEST: allow direct testing of the type matcher. if ($dbg_type) { if ($line =~ /^.\s*$Declare\s*$/) { @@ -3016,21 +3480,20 @@ sub process { } # check for global initialisers. - if ($line =~ /^\+(\s*$Type\s*$Ident\s*(?:\s+$Modifier))*\s*=\s*(0|NULL|false)\s*;/) { + if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) { if (ERROR("GLOBAL_INITIALISERS", - "do not initialise globals to 0 or NULL\n" . - $herecurr) && + "do not initialise globals to $1\n" . $herecurr) && $fix) { - $fixed[$fixlinenr] =~ s/($Type\s*$Ident\s*(?:\s+$Modifier))*\s*=\s*(0|NULL|false)\s*;/$1;/; + $fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*$zero_initializer\s*;/$1;/; } } # check for static initialisers. - if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) { + if ($line =~ /^\+.*\bstatic\s.*=\s*($zero_initializer)\s*;/) { if (ERROR("INITIALISED_STATIC", - "do not initialise statics to 0 or NULL\n" . + "do not initialise statics to $1\n" . $herecurr) && $fix) { - $fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/; + $fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*$zero_initializer\s*;/$1;/; } } @@ -3055,6 +3518,18 @@ sub process { $herecurr); } +# check for const <foo> const where <foo> is not a pointer or array type + if ($sline =~ /\bconst\s+($BasicType)\s+const\b/) { + my $found = $1; + if ($sline =~ /\bconst\s+\Q$found\E\s+const\b\s*\*/) { + WARN("CONST_CONST", + "'const $found const *' should probably be 'const $found * const'\n" . $herecurr); + } elsif ($sline !~ /\bconst\s+\Q$found\E\s+const\s+\w+\s*\[/) { + WARN("CONST_CONST", + "'const $found const' should probably be 'const $found'\n" . $herecurr); + } + } + # check for non-global char *foo[] = {"bar", ...} declarations. if ($line =~ /^.\s+(?:static\s+|const\s+)?char\s+\*\s*\w+\s*\[\s*\]\s*=\s*\{/) { WARN("STATIC_CONST_CHAR_ARRAY", @@ -3062,6 +3537,19 @@ sub process { $herecurr); } +# check for sizeof(foo)/sizeof(foo[0]) that could be ARRAY_SIZE(foo) + if ($line =~ m@\bsizeof\s*\(\s*($Lval)\s*\)@) { + my $array = $1; + if ($line =~ m@\b(sizeof\s*\(\s*\Q$array\E\s*\)\s*/\s*sizeof\s*\(\s*\Q$array\E\s*\[\s*0\s*\]\s*\))@) { + my $array_div = $1; + if (WARN("ARRAY_SIZE", + "Prefer ARRAY_SIZE($array)\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$array_div\E/ARRAY_SIZE($array)/; + } + } + } + # check for function declarations without arguments like "int foo()" if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) { if (ERROR("FUNCTION_WITHOUT_ARGS", @@ -3147,13 +3635,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); @@ -3162,7 +3652,7 @@ sub process { # check for uses of printk_ratelimit if ($line =~ /\bprintk_ratelimit\s*\(/) { WARN("PRINTK_RATELIMITED", -"Prefer printk_ratelimited or pr_<level>_ratelimited to printk_ratelimit\n" . $herecurr); + "Prefer printk_ratelimited or pr_<level>_ratelimited to printk_ratelimit\n" . $herecurr); } # printk should use KERN_* levels. Note that follow on printk's on the @@ -3217,10 +3707,18 @@ sub process { "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr); } +# ENOSYS means "bad syscall nr" and nothing else. This will have a small +# number of false positives, but assembly files are not checked, so at +# least the arch entry code will not trigger this warning. + if ($line =~ /\bENOSYS\b/) { + WARN("ENOSYS", + "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr); + } + # 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) { @@ -3473,7 +3971,7 @@ sub process { # Ignore operators passed as parameters. if ($op_type ne 'V' && - $ca =~ /\s$/ && $cc =~ /^\s*,/) { + $ca =~ /\s$/ && $cc =~ /^\s*[,\)]/) { # # Ignore comments # } elsif ($op =~ /^$;+$/) { @@ -3510,14 +4008,33 @@ sub process { } } - # , must have a space on the right. + # , must not have a space before and must have a space on the right. } elsif ($op eq ',') { + my $rtrim_before = 0; + my $space_after = 0; + if ($ctx =~ /Wx./) { + if (ERROR("SPACING", + "space prohibited before that '$op' $at\n" . $hereptr)) { + $line_fixed = 1; + $rtrim_before = 1; + } + } if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) { if (ERROR("SPACING", "space required after that '$op' $at\n" . $hereptr)) { - $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " "; $line_fixed = 1; $last_after = $n; + $space_after = 1; + } + } + if ($rtrim_before || $space_after) { + if ($rtrim_before) { + $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); + } else { + $good = $fix_elements[$n] . trim($fix_elements[$n + 1]); + } + if ($space_after) { + $good .= " "; } } @@ -3589,7 +4106,22 @@ sub process { $op eq '*' or $op eq '/' or $op eq '%') { - if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) { + if ($check) { + if (defined $fix_elements[$n + 2] && $ctx !~ /[EW]x[EW]/) { + if (CHK("SPACING", + "spaces preferred around that '$op' $at\n" . $hereptr)) { + $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; + $fix_elements[$n + 2] =~ s/^\s+//; + $line_fixed = 1; + } + } elsif (!defined $fix_elements[$n + 2] && $ctx !~ /Wx[OE]/) { + if (CHK("SPACING", + "space preferred before that '$op' $at\n" . $hereptr)) { + $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]); + $line_fixed = 1; + } + } + } elsif ($ctx =~ /Wx[^WCE]|[^WCE]xW/) { if (ERROR("SPACING", "need consistent spacing around '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; @@ -3624,6 +4156,14 @@ sub process { $ok = 1; } + # for asm volatile statements + # ignore a colon with another + # colon immediately before or after + if (($op eq ':') && + ($ca =~ /:$/ || $cc =~ /^:/)) { + $ok = 1; + } + # messages are ERROR, but ?: are CHK if ($ok == 0) { my $msg_type = \&ERROR; @@ -3690,8 +4230,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) { @@ -3752,7 +4292,6 @@ sub process { if (ERROR("SPACING", "space prohibited before that close parenthesis ')'\n" . $herecurr) && $fix) { - print("fixlinenr: <$fixlinenr> fixed[fixlinenr]: <$fixed[$fixlinenr]>\n"); $fixed[$fixlinenr] =~ s/\s+\)/\)/; } @@ -3762,9 +4301,27 @@ sub process { # ie: &(foo->bar) should be &foo->bar and *(foo->bar) should be *foo->bar while ($line =~ /(?:[^&]&\s*|\*)\(\s*($Ident\s*(?:$Member\s*)+)\s*\)/g) { - CHK("UNNECESSARY_PARENTHESES", - "Unnecessary parentheses around $1\n" . $herecurr); - } + my $var = $1; + if (CHK("UNNECESSARY_PARENTHESES", + "Unnecessary parentheses around $var\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\(\s*\Q$var\E\s*\)/$var/; + } + } + +# check for unnecessary parentheses around function pointer uses +# ie: (foo->bar)(); should be foo->bar(); +# but not "if (foo->bar) (" to avoid some false positives + if ($line =~ /(\bif\s*|)(\(\s*$Ident\s*(?:$Member\s*)+\))[ \t]*\(/ && $1 !~ /^if/) { + my $var = $2; + if (CHK("UNNECESSARY_PARENTHESES", + "Unnecessary parentheses around function pointer $var\n" . $herecurr) && + $fix) { + my $var2 = deparenthesize($var); + $var2 =~ s/\s//g; + $fixed[$fixlinenr] =~ s/\Q$var\E/$var2/; + } + } #goto labels aren't indented, allow a single space however if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and @@ -3820,12 +4377,41 @@ sub process { } } -# Return of what appears to be an errno should normally be -'ve - if ($line =~ /^.\s*return\s*(E[A-Z]*)\s*;/) { +# 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; if ($name ne 'EOF' && $name ne 'ERROR') { WARN("USE_NEGATIVE_ERRNO", - "return of an errno should typically be -ve (return -$1)\n" . $herecurr); + "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr); } } @@ -4004,7 +4590,9 @@ sub process { #Ignore Page<foo> variants $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && #Ignore SI style variants like nS, mV and dB (ie: max_uV, regulator_min_uA_show) - $var !~ /^(?:[a-z_]*?)_?[a-z][A-Z](?:_[a-z_]+)?$/) { + $var !~ /^(?:[a-z_]*?)_?[a-z][A-Z](?:_[a-z_]+)?$/ && +#Ignore some three character SI units explicitly, like MiB and KHz + $var !~ /^(?:[a-z_]*?)_?(?:[KMGT]iB|[KMGT]?Hz)(?:_[a-z_]+)?$/) { while ($var =~ m{($Ident)}g) { my $word = $1; next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/); @@ -4033,7 +4621,8 @@ sub process { } } -#warn if <asm/foo.h> is #included and <linux/foo.h> is available (uses RAW line) +# warn if <asm/foo.h> is #included and <linux/foo.h> is available and includes +# itself <asm/foo.h> (uses RAW line) if ($tree && $rawline =~ m{^.\s*\#\s*include\s*\<asm\/(.*)\.h\>}) { my $file = "$1.h"; my $checkfile = "include/linux/$file"; @@ -4041,12 +4630,15 @@ sub process { $realfile ne $checkfile && $1 !~ /$allowed_asm_includes/) { - if ($realfile =~ m{^arch/}) { - CHK("ARCH_INCLUDE_LINUX", - "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr); - } else { - WARN("INCLUDE_LINUX", - "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr); + my $asminclude = `grep -Ec "#include\\s+<asm/$file>" $root/$checkfile`; + if ($asminclude > 0) { + if ($realfile =~ m{^arch/}) { + CHK("ARCH_INCLUDE_LINUX", + "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr); + } else { + WARN("INCLUDE_LINUX", + "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr); + } } } } @@ -4060,12 +4652,17 @@ sub process { my $cnt = $realcnt; my ($off, $dstat, $dcond, $rest); my $ctx = ''; + my $has_flow_statement = 0; + my $has_arg_concat = 0; ($dstat, $dcond, $ln, $cnt, $off) = ctx_statement_block($linenr, $realcnt, 0); $ctx = $dstat; #print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n"; #print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n"; + $has_flow_statement = 1 if ($ctx =~ /\b(goto|return)\b/); + $has_arg_concat = 1 if ($ctx =~ /\#\#/ && $ctx !~ /\#\#\s*(?:__VA_ARGS__|args)\b/); + $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//; $dstat =~ s/$;//g; $dstat =~ s/\\\n.//g; @@ -4075,16 +4672,19 @@ sub process { # Flatten any parentheses and braces while ($dstat =~ s/\([^\(\)]*\)/1/ || $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/\[[^\[\]]*\]/1/) + $dstat =~ s/.\[[^\[\]]*\]/1/) { } # 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/) { } + # Make asm volatile uses seem like a generic function + $dstat =~ s/\b_*asm_*\s+_*volatile_*\b/asm_volatile/g; + my $exceptions = qr{ $Declare| module_param_named| @@ -4095,7 +4695,8 @@ sub process { union| struct| \.$Ident\s*=\s*| - ^\"|\"$ + ^\"|\"$| + ^\[ }x; #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n"; if ($dstat ne '' && @@ -4110,7 +4711,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*$//; @@ -4126,10 +4727,23 @@ sub process { "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx"); } else { ERROR("COMPLEX_MACRO", - "Macros with complex values should be enclosed in parenthesis\n" . "$herectx"); + "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); } } +# check for macros with flow control, but without ## concatenation +# ## concatenation is commonly a macro that defines a function so ignore those + if ($has_flow_statement && !$has_arg_concat) { + my $herectx = $here . "\n"; + my $cnt = statement_rawlines($ctx); + + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + WARN("MACRO_WITH_FLOW_CONTROL", + "Macros with flow control statements should be avoided\n" . "$herectx"); + } + # check for line continuations outside of #defines, preprocessor #, and asm } else { @@ -4157,6 +4771,7 @@ sub process { $ctx = $dstat; $dstat =~ s/\\\n.//g; + $dstat =~ s/$;/ /g; if ($dstat =~ /^\+\s*#\s*define\s+$Ident\s*${balanced_parens}\s*do\s*{(.*)\s*}\s*while\s*\(\s*0\s*\)\s*([;\s]*)\s*$/) { my $stmts = $2; @@ -4323,12 +4938,18 @@ sub process { # check for unnecessary blank lines around braces if (($line =~ /^.\s*}\s*$/ && $prevrawline =~ /^.\s*$/)) { - CHK("BRACES", - "Blank lines aren't necessary before a close brace '}'\n" . $hereprev); + if (CHK("BRACES", + "Blank lines aren't necessary before a close brace '}'\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + fix_delete_line($fixlinenr - 1, $prevrawline); + } } if (($rawline =~ /^.\s*$/ && $prevline =~ /^..*{\s*$/)) { - CHK("BRACES", - "Blank lines aren't necessary after an open brace '{'\n" . $hereprev); + if (CHK("BRACES", + "Blank lines aren't necessary after an open brace '{'\n" . $hereprev) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } # no volatiles please @@ -4338,6 +4959,89 @@ sub process { "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr); } +# Check for user-visible strings broken across lines, which breaks the ability +# 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*$String/ && + $prevline =~ /"\s*$/ && + $prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) { + if (WARN("SPLIT_STRING", + "quoted string split across lines\n" . $hereprev) && + $fix && + $prevrawline =~ /^\+.*"\s*$/ && + $last_coalesced_string_linenr != $linenr - 1) { + my $extracted_string = get_quoted_string($line, $rawline); + my $comma_close = ""; + if ($rawline =~ /\Q$extracted_string\E(\s*\)\s*;\s*$|\s*,\s*)/) { + $comma_close = $1; + } + + fix_delete_line($fixlinenr - 1, $prevrawline); + fix_delete_line($fixlinenr, $rawline); + my $fixedline = $prevrawline; + $fixedline =~ s/"\s*$//; + $fixedline .= substr($extracted_string, 1) . trim($comma_close); + fix_insert_line($fixlinenr - 1, $fixedline); + $fixedline = $rawline; + $fixedline =~ s/\Q$extracted_string\E\Q$comma_close\E//; + if ($fixedline !~ /\+\s*$/) { + fix_insert_line($fixlinenr, $fixedline); + } + $last_coalesced_string_linenr = $linenr; + } + } + +# check for missing a space in a string concatenation + if ($prevrawline =~ /[^\\]\w"$/ && $rawline =~ /^\+[\t ]+"\w/) { + WARN('MISSING_SPACE', + "break quoted strings at a space character\n" . $hereprev); + } + +# check for spaces before a quoted newline + if ($rawline =~ /^.*\".*\s\\n/) { + if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", + "unnecessary whitespace before a quoted newline\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^(\+.*\".*)\s+\\n/$1\\n/; + } + + } + +# concatenated string without spaces between elements + 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 =~ /$String\s*"/) { + WARN("STRING_FRAGMENTS", + "Consecutive strings are generally better as a single string\n" . $herecurr); + } + +# 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 =~ /(?<!%)%[\*\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 " + if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) { + WARN("LINE_CONTINUATIONS", + "Avoid line continuations in quoted strings\n" . $herecurr); + } + # warn about #if 0 if ($line =~ /^.\s*\#\s*if\s+0\b/) { CHK("REDUNDANT_CODE", @@ -4347,10 +5051,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 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/; + } + } } } @@ -4371,6 +5099,39 @@ sub process { } } +# check for logging functions with KERN_<LEVEL> + if ($line !~ /printk(?:_ratelimited|_once)?\s*\(/ && + $line =~ /\b$logFunctions\s*\(.*\b(KERN_[A-Z]+)\b/) { + my $level = $1; + if (WARN("UNNECESSARY_KERN_LEVEL", + "Possible unnecessary $level\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\s*$level\s*//; + } + } + +# check for mask then right shift without a parentheses + if ($^V && $^V ge 5.10.0 && + $line =~ /$LvalOrFunc\s*\&\s*($LvalOrFunc)\s*>>/ && + $4 !~ /^\&/) { # $LvalOrFunc may be &foo, ignore if so + WARN("MASK_THEN_SHIFT", + "Possible precedence defect with mask then right shift - may need parentheses\n" . $herecurr); + } + +# check for pointer comparisons to NULL + if ($^V && $^V ge 5.10.0) { + while ($line =~ /\b$LvalOrFunc\s*(==|\!=)\s*NULL\b/g) { + my $val = $1; + my $equal = "!"; + $equal = "" if ($4 eq "!="); + if (CHK("COMPARISON_TO_NULL", + "Comparison to NULL could be written \"${equal}${val}\"\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b\Q$val\E\s*(?:==|\!=)\s*NULL\b/$equal$val/; + } + } + } + # check for bad placement of section $InitAttribute (e.g.: __initdata) if ($line =~ /(\b$InitAttribute\b)/) { my $attr = $1; @@ -4418,6 +5179,16 @@ sub process { } } +# check for __read_mostly with const non-pointer (should just be const) + if ($line =~ /\b__read_mostly\b/ && + $line =~ /($Type)\s*$Ident/ && $1 !~ /\*\s*$/ && $1 =~ /\bconst\b/) { + if (ERROR("CONST_READ_MOSTLY", + "Invalid use of __read_mostly with const type\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\s+__read_mostly\b//; + } + } + # don't use __constant_<foo> functions outside of include/uapi/ if ($realfile !~ m@^include/uapi/@ && $line =~ /(__constant_(?:htons|ntohs|[bl]e(?:16|32|64)_to_cpu|cpu_to_[bl]e(?:16|32|64)))\s*\(/) { @@ -4493,12 +5264,69 @@ sub process { } } # 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)\(/) { + + my $barriers = qr{ + mb| + rmb| + wmb| + read_barrier_depends + }x; + my $barrier_stems = qr{ + mb__before_atomic| + mb__after_atomic| + store_release| + load_acquire| + store_mb| + (?:$barriers) + }x; + my $all_barriers = qr{ + (?:$barriers)| + smp_(?:$barrier_stems)| + virt_(?:$barrier_stems) + }x; + + if ($line =~ /\b(?:$all_barriers)\s*\(/) { if (!ctx_has_comment($first_line, $linenr)) { WARN("MEMORY_BARRIER", "memory barrier without comment\n" . $herecurr); } } + + my $underscore_smp_barriers = qr{__smp_(?:$barrier_stems)}x; + + if ($realfile !~ m@^include/asm-generic/@ && + $realfile !~ m@/barrier\.h$@ && + $line =~ m/\b(?:$underscore_smp_barriers)\s*\(/ && + $line !~ m/^.\s*\#\s*define\s+(?:$underscore_smp_barriers)\s*\(/) { + WARN("MEMORY_BARRIER", + "__smp memory barriers shouldn't be used outside barrier.h and asm-generic\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 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", @@ -4565,6 +5393,53 @@ sub process { } } +# Check for __attribute__ weak, or __weak declarations (may have link issues) + if ($^V && $^V ge 5.10.0 && + $line =~ /(?:$Declare|$DeclareMisordered)\s*$Ident\s*$balanced_parens\s*(?:$Attribute)?\s*;/ && + ($line =~ /\b__attribute__\s*\(\s*\(.*\bweak\b/ || + $line =~ /\b__weak\b/)) { + ERROR("WEAK_DECLARATION", + "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 cast of C90 native int or longer types constants + if ($line =~ /(\(\s*$C90_int_types\s*\)\s*)($Constant)\b/) { + my $cast = $1; + my $const = $2; + if (WARN("TYPECAST_INT_CONSTANT", + "Unnecessary typecast of c90 int constant\n" . $herecurr) && + $fix) { + my $suffix = ""; + my $newconst = $const; + $newconst =~ s/${Int_type}$//; + $suffix .= 'U' if ($cast =~ /\bunsigned\b/); + if ($cast =~ /\blong\s+long\b/) { + $suffix .= 'LL'; + } elsif ($cast =~ /\blong\b/) { + $suffix .= 'L'; + } + $fixed[$fixlinenr] =~ s/\Q$cast\E$const\b/$newconst$suffix/; + } + } + # check for sizeof(&) if ($line =~ /\bsizeof\s*\(\s*\&/) { WARN("SIZEOF_ADDRESS", @@ -4580,12 +5455,6 @@ sub process { } } -# check for line continuations in quoted strings with odd counts of " - if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) { - WARN("LINE_CONTINUATIONS", - "Avoid line continuations in quoted strings\n" . $herecurr); - } - # check for struct spinlock declarations if ($line =~ /^.\s*\bstruct\s+spinlock\s+\w+\s*;/) { WARN("USE_SPINLOCK_T", @@ -4595,7 +5464,8 @@ sub process { # check for seq_printf uses that could be seq_puts if ($sline =~ /\bseq_printf\s*\(.*"\s*\)\s*;\s*$/) { my $fmt = get_quoted_string($line, $rawline); - if ($fmt ne "" && $fmt !~ /[^\\]\%/) { + $fmt =~ s/%%//g; + if ($fmt !~ /%/) { if (WARN("PREFER_SEQ_PUTS", "Prefer seq_puts to seq_printf\n" . $herecurr) && $fix) { @@ -4607,7 +5477,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; @@ -4624,14 +5494,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 && @@ -4821,6 +5723,27 @@ sub process { } } +# check for #defines like: 1 << <digit> that could be BIT(digit) + if ($line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) { + my $ull = ""; + $ull = "_ULL" if (defined($1) && $1 =~ /ll/i); + if (CHK("BIT_MACRO", + "Prefer using the BIT$ull macro\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/BIT${ull}($1)/; + } + } + +# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) { + my $config = $1; + if (WARN("PREFER_IS_ENABLED", + "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; + } + } + # check for case / default statements not preceded by break/fallthrough/switch if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) { my $has_break = 0; @@ -4869,6 +5792,12 @@ sub process { } } +# check for uses of __DATE__, __TIME__, __TIMESTAMP__ + while ($line =~ /\b(__(?:DATE|TIME|TIMESTAMP)__)\b/g) { + ERROR("DATE_TIME", + "Use of the '$1' macro makes the build non-deterministic\n" . $herecurr); + } + # check for use of yield() if ($line =~ /\byield\s*\(\s*\)/) { WARN("YIELD", @@ -4920,8 +5849,9 @@ sub process { "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); } -# check for various ops structs, ensure they are const. - my $struct_ops = qr{acpi_dock_ops| +# check for various structs that are normally const (ops, kgdb, device_tree) + my $const_structs = qr{ + acpi_dock_ops| address_space_operations| backlight_ops| block_device_operations| @@ -4944,6 +5874,7 @@ sub process { mtrr_ops| neigh_ops| nlmsvc_binding| + of_device_id| pci_raw_ops| pipe_buf_operations| platform_hibernation_ops| @@ -4956,10 +5887,11 @@ sub process { stacktrace_ops| sysfs_ops| tty_operations| + uart_ops| usb_mon_operations| wd_ops}x; if ($line !~ /\bconst\b/ && - $line =~ /\bstruct\s+($struct_ops)\b/) { + $line =~ /\bstruct\s+($const_structs)\b/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); @@ -4984,16 +5916,11 @@ sub process { "#define of '$1' is wrong - use Kconfig variables or standard guards instead\n" . $herecurr); } -# check for %L{u,d,i} in strings - my $string; - while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { - $string = substr($rawline, $-[1], $+[1] - $-[1]); - $string =~ s/%%/__/g; - if ($string =~ /(?<!%)%L[udi]/) { - WARN("PRINTF_L", - "\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr); - last; - } +# likely/unlikely comparisons similar to "(likely(foo) > 0)" + if ($^V && $^V ge 5.10.0 && + $line =~ /\b((?:un)?likely)\s*\(\s*$FuncArg\s*\)\s*$Compare/) { + WARN("LIKELY_MISUSE", + "Using $1 should generally have parentheses around the comparison\n" . $herecurr); } # whine mightly about in_atomic @@ -5007,6 +5934,28 @@ sub process { } } +# whine about ACCESS_ONCE + if ($^V && $^V ge 5.10.0 && + $line =~ /\bACCESS_ONCE\s*$balanced_parens\s*(=(?!=))?\s*($FuncArg)?/) { + my $par = $1; + my $eq = $2; + my $fun = $3; + $par =~ s/^\(\s*(.*)\s*\)$/$1/; + if (defined($eq)) { + if (WARN("PREFER_WRITE_ONCE", + "Prefer WRITE_ONCE(<FOO>, <BAR>) over ACCESS_ONCE(<FOO>) = <BAR>\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)\s*$eq\s*\Q$fun\E/WRITE_ONCE($par, $fun)/; + } + } else { + if (WARN("PREFER_READ_ONCE", + "Prefer READ_ONCE(<FOO>) over ACCESS_ONCE(<FOO>)\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)/READ_ONCE($par)/; + } + } + } + # check for lockdep_set_novalidate_class if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ || $line =~ /__lockdep_no_validate__\s*\)/ ) { @@ -5018,8 +5967,8 @@ sub process { } } - if ($line =~ /debugfs_create_file.*S_IWUGO/ || - $line =~ /DEVICE_ATTR.*S_IWUGO/ ) { + if ($line =~ /debugfs_create_\w+.*\b$mode_perms_world_writable\b/ || + $line =~ /DEVICE_ATTR.*\b$mode_perms_world_writable\b/) { WARN("EXPORTED_WORLD_WRITABLE", "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } @@ -5047,10 +5996,31 @@ sub process { length($val) ne 4)) { ERROR("NON_OCTAL_PERMISSIONS", "Use 4 digit octal (0777) not decimal permissions\n" . $herecurr); + } elsif ($val =~ /^$Octal$/ && (oct($val) & 02)) { + ERROR("EXPORTED_WORLD_WRITABLE", + "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } } } } + +# 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 @@ -5071,11 +6041,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"); } @@ -5086,28 +6056,29 @@ 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 there were any defects found and not already fixing them + if (!$clean and !$fix) { + print << "EOM" - 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"); +NOTE: For some of the reported defects, checkpatch may be able to + mechanically convert to the typical style using --fix or --fix-inplace. +EOM } - # 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)) { @@ -5135,6 +6106,7 @@ sub process { if (!$quiet) { print << "EOM"; + Wrote EXPERIMENTAL --fix correction(s) to '$newfile' Do _NOT_ trust the results written to this file. @@ -5142,22 +6114,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; }