HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60804 )
Change subject: [TESTME]util/lint: update checkpatch.pl to latest linux version ......................................................................
[TESTME]util/lint: update checkpatch.pl to latest linux version
Change-Id: Ife95563ba61083cc19a383f3a9372466625e2ee6 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M util/lint/checkpatch.pl 1 file changed, 1,391 insertions(+), 441 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/60804/1
diff --git a/util/lint/checkpatch.pl b/util/lint/checkpatch.pl index 41aae93..a784691 100755 --- a/util/lint/checkpatch.pl +++ b/util/lint/checkpatch.pl @@ -13,6 +13,7 @@ use File::Basename; use Cwd 'abs_path'; use Term::ANSIColor qw(:constants); +use Encode qw(decode encode);
my $P = $0; my $D = dirname(abs_path($P)); @@ -22,6 +23,9 @@ use Getopt::Long qw(:config no_auto_abbrev);
my $quiet = 0; +my $verbose = 0; +my %verbose_messages = (); +my %verbose_emitted = (); my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; @@ -42,6 +46,8 @@ my $fix = 0; my $fix_inplace = 0; my $root = $P; #coreboot +my $gitroot = $ENV{'GIT_DIR'}; +$gitroot = ".git" if !defined($gitroot); my %debug; my %camelcase = (); my %use_type = (); @@ -51,18 +57,23 @@ my @exclude = (); #coreboot my $help = 0; my $configuration_file = ".checkpatch.conf"; -my $max_line_length = 80; +my $max_line_length = 100; 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 $user_codespellfile = ""; my $conststructsfile = "$D/const_structs.checkpatch"; -my $typedefsfile = ""; +my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst"; +my $typedefsfile; my $color = "auto"; -my $allow_c99_comments = 1; - +my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE +# git output parsing needs US English output, so first set backtick child process LANGUAGE +my $git_command ='export LANGUAGE=en_US.UTF-8; git'; +my $tabsize = 8; +my ${CONFIG_} = "CONFIG_"; # For coreboot jenkins # If taint mode is enabled, Untaint the path - files must be in /bin, /usr/bin or /usr/local/bin if ( ${^TAINT} ) { @@ -79,6 +90,7 @@
Options: -q, --quiet quiet + -v, --verbose verbose mode --no-tree run without a kernel tree --no-signoff do not check for 'Signed-off-by' line --patch treat FILE as patchfile (default) @@ -100,10 +112,12 @@ --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 - --exclude DIR(,DIR22...) exclude directories --show-types show the specific message type in the output - --max-line-length=n set the maximum line length, if exceeded, warn + --max-line-length=n set the maximum line length, (default $max_line_length) + if exceeded, warn on patches + requires --strict for use with --file --min-conf-desc-length=n set the min description length, if shorter, warn + --tab-size=n set the number of spaces for tab (default $tabsize) --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 @@ -124,11 +138,13 @@ --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) + (default:$codespellfile) --codespellfile Use this codespell dictionary --typedefsfile Read additional types from this file --color[=WHEN] Use colors 'always', 'never', or only when output is a terminal ('auto'). Default is 'auto'. + --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default + ${CONFIG_}) -h, --help, --version display this help and exit
When FILE is - read standard input. @@ -155,15 +171,51 @@ my $text = <$script>; close($script);
- my @types = (); + my %types = (); # Also catch when type or level is passed through a variable - for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&{$msg_level})\s*(|$msg_type\s*=)\s*"([^"]+)"/g) { - push (@types, $_); + while ($text =~ /(?:(\bCHK|\bWARN|\bERROR|&{$msg_level})\s*(|$msg_type\s*=)\s*"([^"]+)"/g) { + if (defined($1)) { + if (exists($types{$2})) { + $types{$2} .= ",$1" if ($types{$2} ne $1); + } else { + $types{$2} = $1; + } + } else { + $types{$2} = "UNDETERMINED"; + } } - @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); - foreach my $type (@types) { + if ($color) { + print(" ( Color coding: "); + print(RED . "ERROR" . RESET); + print(" | "); + print(YELLOW . "WARNING" . RESET); + print(" | "); + print(GREEN . "CHECK" . RESET); + print(" | "); + print("Multiple levels / Undetermined"); + print(" )\n\n"); + } + + foreach my $type (sort keys %types) { + my $orig_type = $type; + if ($color) { + my $level = $types{$type}; + if ($level eq "ERROR") { + $type = RED . $type . RESET; + } elsif ($level eq "WARN") { + $type = YELLOW . $type . RESET; + } elsif ($level eq "CHK") { + $type = GREEN . $type . RESET; + } + } print(++$count . "\t" . $type . "\n"); + if ($verbose && exists($verbose_messages{$orig_type})) { + my $message = $verbose_messages{$orig_type}; + $message =~ s/\n/\n\t/g; + print("\t" . $message . "\n\n"); + } }
exit($exitcode); @@ -195,6 +247,46 @@ unshift(@ARGV, @conf_args) if @conf_args; }
+sub load_docs { + open(my $docs, '<', "$docsfile") + or warn "$P: Can't read the documentation file $docsfile $!\n"; + + my $type = ''; + my $desc = ''; + my $in_desc = 0; + + while (<$docs>) { + chomp; + my $line = $_; + $line =~ s/\s+$//; + + if ($line =~ /^\s***(.+)**$/) { + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + $type = $1; + $desc = ''; + $in_desc = 1; + } elsif ($in_desc) { + if ($line =~ /^(?:\s{4,}|$)/) { + $line =~ s/^\s{4}//; + $desc .= $line; + $desc .= "\n"; + } else { + $verbose_messages{$type} = trim($desc); + $type = ''; + $desc = ''; + $in_desc = 0; + } + } + } + + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + close($docs); +} + # Perl's Getopt::Long allows options to take optional arguments after a space. # Prevent --color by itself from consuming other arguments foreach (@ARGV) { @@ -205,6 +297,7 @@
GetOptions( 'q|quiet+' => $quiet, + 'v|verbose!' => $verbose, 'tree!' => $tree, 'signoff!' => $chk_signoff, 'patch!' => $chk_patch, @@ -222,6 +315,7 @@ 'list-types!' => $list_types, 'max-line-length=i' => $max_line_length, 'min-conf-desc-length=i' => $min_conf_desc_length, + 'tab-size=i' => $tabsize, 'root=s' => $root, 'summary!' => $summary, 'mailback!' => $mailback, @@ -232,35 +326,43 @@ 'debug=s' => %debug, 'test-only=s' => $tst_only, 'codespell!' => $codespell, - 'codespellfile=s' => $codespellfile, + 'codespellfile=s' => $user_codespellfile, 'typedefsfile=s' => $typedefsfile, 'color=s' => $color, 'no-color' => $color, #keep old behaviors of -nocolor 'nocolor' => $color, #keep old behaviors of -nocolor + 'kconfig-prefix=s' => ${CONFIG_}, 'h|help' => $help, 'version' => $help -) or help(1); +) or $help = 2;
-help(0) if ($help); +if ($user_codespellfile) { + # Use the user provided codespell file unconditionally + $codespellfile = $user_codespellfile; +} elsif (!(-f $codespellfile)) { + # If /usr/share/codespell/dictionary.txt is not present, try to find it + # under codespell's install directory: <codespell_root>/data/dictionary.txt + if (($codespell || $help) && which("codespell") ne "" && which("python") ne "") { + my $python_codespell_dict = << "EOF";
-list_types(0) if ($list_types); +import os.path as op +import codespell_lib +codespell_dir = op.dirname(codespell_lib.__file__) +codespell_file = op.join(codespell_dir, 'data', 'dictionary.txt') +print(codespell_file, end='') +EOF
-$fix = 1 if ($fix_inplace); -$check_orig = $check; - -my $exit = 0; - -if ($^V && $^V lt $minimum_perl_version) { - printf "$P: requires at least perl version %vd\n", $minimum_perl_version; - if (!$ignore_perl_version) { - exit(1); + my $codespell_dict = `python -c "$python_codespell_dict" 2> /dev/null`; + $codespellfile = $codespell_dict if (-f $codespell_dict); } }
-#if no filenames are given, push '-' to read patch from stdin -if ($#ARGV < 0) { - push(@ARGV, '-'); -} +# $help is 1 if either -h, --help or --version is passed as option - exitcode: 0 +# $help is 2 if invalid option is passed - exitcode: 1 +help($help - 1) if ($help); + +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); +die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse);
if ($color =~ /^[01]$/) { $color = !$color; @@ -271,9 +373,32 @@ } elsif ($color =~ /^auto$/i) { $color = (-t STDOUT); } else { - die "Invalid color mode: $color\n"; + die "$P: Invalid color mode: $color\n"; }
+load_docs() if ($verbose); +list_types(0) if ($list_types); + +$fix = 1 if ($fix_inplace); +$check_orig = $check; + +my $exit = 0; + +my $perl_version_ok = 1; +if ($^V && $^V lt $minimum_perl_version) { + $perl_version_ok = 0; + printf "$P: requires at least perl version %vd\n", $minimum_perl_version; + exit(1) if (!$ignore_perl_version); +} + +#if no filenames are given, push '-' to read patch from stdin +if ($#ARGV < 0) { + push(@ARGV, '-'); +} + +# skip TAB size 1 to avoid additional checks on $tabsize - 1 +die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2); + sub hash_save_array_words { my ($hashRef, $arrayRef) = @_;
@@ -356,9 +481,10 @@ __force| __iomem| __must_check| - __init_refok| __kprobes| __ref| + __refconst| + __refdata| __rcu| __private }x; @@ -372,6 +498,7 @@ # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + volatile| __percpu| __nocast| __safe| @@ -388,12 +515,14 @@ __noclone| __deprecated| __read_mostly| + __ro_after_init| __kprobes| $InitAttribute| ____cacheline_aligned| ____cacheline_aligned_in_smp| ____cacheline_internodealigned_in_smp| - __weak + __weak| + __alloc_size\s*(\s*\d+\s*(?:,\s*\d+\s*)?) }x; our $Modifier; our $Inline = qr{inline|__always_inline|noinline|__inline|__inline__}; @@ -405,7 +534,7 @@ 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 $String = qr{(?:\b[Lu])?"[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]?}; @@ -473,8 +602,19 @@ seq_vprintf|seq_printf|seq_puts )};
+our $allocFunctions = qr{(?x: + (?:(?:devm_)? + (?:kv|k|v)[czm]alloc(?:_array)?(?:_node)? | + kstrdup(?:_const)? | + kmemdup(?:_nul)?) | + (?:\w+)?alloc_skb(?:_ip_align)? | + # dev_alloc_skb/netdev_alloc_skb, et al + dma_alloc_coherent +)}; + our $signature_tags = qr{(?xi: Signed-off-by:| + Co-developed-by:| Acked-by:| Tested-by:| Reviewed-by:| @@ -484,6 +624,88 @@ Cc: )};
+our $tracing_logging_tags = qr{(?xi: + [=-]*> | + <[=-]* | + [ | + ] | + start | + called | + entered | + entry | + enter | + in | + inside | + here | + begin | + exit | + end | + done | + leave | + completed | + out | + return | + [.!:\s]* +)}; + +sub edit_distance_min { + my (@arr) = @_; + my $len = scalar @arr; + if ((scalar @arr) < 1) { + # if underflow, return + return; + } + my $min = $arr[0]; + for my $i (0 .. ($len-1)) { + if ($arr[$i] < $min) { + $min = $arr[$i]; + } + } + return $min; +} + +sub get_edit_distance { + my ($str1, $str2) = @_; + $str1 = lc($str1); + $str2 = lc($str2); + $str1 =~ s/-//g; + $str2 =~ s/-//g; + my $len1 = length($str1); + my $len2 = length($str2); + # two dimensional array storing minimum edit distance + my @distance; + for my $i (0 .. $len1) { + for my $j (0 .. $len2) { + if ($i == 0) { + $distance[$i][$j] = $j; + } elsif ($j == 0) { + $distance[$i][$j] = $i; + } elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) { + $distance[$i][$j] = $distance[$i - 1][$j - 1]; + } else { + my $dist1 = $distance[$i][$j - 1]; #insert distance + my $dist2 = $distance[$i - 1][$j]; # remove + my $dist3 = $distance[$i - 1][$j - 1]; #replace + $distance[$i][$j] = 1 + edit_distance_min($dist1, $dist2, $dist3); + } + } + } + return $distance[$len1][$len2]; +} + +sub find_standard_signature { + my ($sign_off) = @_; + my @standard_signature_tags = ( + 'Signed-off-by:', 'Co-developed-by:', 'Acked-by:', 'Tested-by:', + 'Reviewed-by:', 'Reported-by:', 'Suggested-by:' + ); + foreach my $signature (@standard_signature_tags) { + return $signature if (get_edit_distance($sign_off, $signature) <= 2); + } + + return ""; +} + our @typeListMisordered = ( qr{char\s+(?:un)?signed}, qr{int\s+(?:(?:un)?signed\s+)?short\s}, @@ -572,6 +794,8 @@ ["__ATTR", 2], );
+my $word_pattern = '\b[A-Z]?[a-z]{2,}\b'; + #Create a search pattern for all these functions to speed up a loop below our $mode_perms_search = ""; foreach my $entry (@mode_permission_funcs) { @@ -580,6 +804,27 @@ } $mode_perms_search = "(?:${mode_perms_search})";
+our %deprecated_apis = ( + "synchronize_rcu_bh" => "synchronize_rcu", + "synchronize_rcu_bh_expedited" => "synchronize_rcu_expedited", + "call_rcu_bh" => "call_rcu", + "rcu_barrier_bh" => "rcu_barrier", + "synchronize_sched" => "synchronize_rcu", + "synchronize_sched_expedited" => "synchronize_rcu_expedited", + "call_rcu_sched" => "call_rcu", + "rcu_barrier_sched" => "rcu_barrier", + "get_state_synchronize_sched" => "get_state_synchronize_rcu", + "cond_synchronize_sched" => "cond_synchronize_rcu", +); + +#Create a search pattern for all these strings to speed up a loop below +our $deprecated_apis_search = ""; +foreach my $entry (keys %deprecated_apis) { + $deprecated_apis_search .= '|' if ($deprecated_apis_search ne ""); + $deprecated_apis_search .= $entry; +} +$deprecated_apis_search = "(?:${deprecated_apis_search})"; + our $mode_perms_world_writable = qr{ S_IWUGO | S_IWOTH | @@ -719,7 +964,7 @@ next; }
- $$wordsRef .= '|' if ($$wordsRef ne ""); + $$wordsRef .= '|' if (defined $$wordsRef); $$wordsRef .= $line; } close($file); @@ -729,16 +974,18 @@ return 0; }
-my $const_structs = ""; -read_words($const_structs, $conststructsfile) - or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +my $const_structs; +if (show_type("CONST_STRUCT")) { + read_words($const_structs, $conststructsfile) + or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +}
-my $typeOtherTypedefs = ""; -if (length($typedefsfile)) { +if (defined($typedefsfile)) { + my $typeOtherTypedefs; read_words($typeOtherTypedefs, $typedefsfile) or warn "No additional types will be considered - file '$typedefsfile': $!\n"; + $typeTypedefs .= '|' . $typeOtherTypedefs if (defined $typeOtherTypedefs); } -$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne "");
sub build_types { my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; @@ -777,12 +1024,12 @@ }x; $Type = qr{ $NonptrType - (?:(?:\s|*|[])+\s*const|(?:\s|*\s*(?:const\s*)?|[])+|(?:\s*[\s*])+)? + (?:(?:\s|*|[])+\s*const|(?:\s|*\s*(?:const\s*)?|[])+|(?:\s*[\s*])+){0,4} (?:\s+$Inline|\s+$Modifier)* }x; $TypeMisordered = qr{ $NonptrTypeMisordered - (?:(?:\s|*|[])+\s*const|(?:\s|*\s*(?:const\s*)?|[])+|(?:\s*[\s*])+)? + (?:(?:\s|*|[])+\s*const|(?:\s|*\s*(?:const\s*)?|[])+|(?:\s*[\s*])+){0,4} (?:\s+$Inline|\s+$Modifier)* }x; $Declare = qr{(?:$Storage\s+(?:$Inline\s+)?)?$Type}; @@ -803,10 +1050,16 @@ our $declaration_macros = qr{(?x: (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*(| (?:$Storage\s+)?[HLP]?LIST_HEAD\s*(| - (?:$Storage\s+)?${Type}\s+uninitialized_var\s*(| (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*( )};
+our %allow_repeated_words = ( + add => '', + added => '', + bad => '', + be => '', +); + sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); @@ -847,14 +1100,29 @@ } }
+our %maintained_status = (); + sub is_maintained_obsolete { my ($filename) = @_;
return 0 if (!$tree || !(-e "$root/scripts/get_maintainer.pl"));
- my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback -f $filename 2>&1`; + if (!exists($maintained_status{$filename})) { + $maintained_status{$filename} = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback -f $filename 2>&1`; + }
- return $status =~ /obsolete/i; + return $maintained_status{$filename} =~ /obsolete/i; +} + +sub is_SPDX_License_valid { + my ($license) = @_; + + return 1 if (!$tree || which("python3") eq "" || !(-x "$root/scripts/spdxcheck.py") || !(-e "$gitroot")); + + my $root_path = abs_path($root); + my $status = `cd "$root_path"; echo "$license" | scripts/spdxcheck.py -`; + return 0 if ($status ne ""); + return 1; }
my $camelcase_seeded = 0; @@ -867,8 +1135,8 @@
$camelcase_seeded = 1;
- if (-e ".git") { - my $git_last_include_commit = `git log --no-merges --pretty=format:"%h%n" -1 -- include`; + if (-e "$gitroot") { + my $git_last_include_commit = `${git_command} log --no-merges --pretty=format:"%h%n" -1 -- include`; chomp $git_last_include_commit; $camelcase_cache = ".checkpatch-camelcase.git.$git_last_include_commit"; } else { @@ -895,8 +1163,8 @@ return; }
- if (-e ".git") { - $files = `git ls-files "include/*.h"`; + if (-e "$gitroot") { + $files = `${git_command} ls-files "include/*.h"`; @include_files = split('\n', $files); }
@@ -915,18 +1183,28 @@ } }
+sub git_is_single_file { + my ($filename) = @_; + + return 0 if ((which("git") eq "") || !(-e "$gitroot")); + + my $output = `${git_command} ls-files -- $filename 2>/dev/null`; + my $count = $output =~ tr/\n//; + return $count eq 1 && $output =~ m{^${filename}$}; +} + sub git_commit_info { my ($commit, $id, $desc) = @_;
- return ($id, $desc) if ((which("git") eq "") || !(-e ".git")); + return ($id, $desc) if ((which("git") eq "") || !(-e "$gitroot"));
- my $output = `git log --no-color --format='%H %s' -1 $commit 2>&1`; + my $output = `${git_command} log --no-color --format='%H %s' -1 $commit 2>&1`; $output =~ s/^\s*//gm; my @lines = split("\n", $output);
return ($id, $desc) if ($#lines < 0);
- if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous./) { + 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... # @@ -936,7 +1214,8 @@ # git log --format='%H %s' -1 $line | # echo "commit $(cut -c 1-12,41-)" # done - } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree./) { + } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree./ || + $lines[0] =~ /^fatal: bad object $commit/) { $id = undef; } else { $id = substr($lines[0], 0, 12); @@ -957,7 +1236,7 @@
# 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"); +die "$P: No git repository found\n" if ($git && !-e "$gitroot");
if ($git) { my @commits = (); @@ -970,7 +1249,7 @@ } else { $git_range = "-1 $commit_expr"; } - my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; + my $lines = `${git_command} 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)); @@ -985,12 +1264,12 @@ }
my $vname; +$allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; - - # coreboot: Mark filename as untainted - $filename =~ /^(.*)$/s or die; $filename = $1; - + my $is_git_file = git_is_single_file($filename); + my $oldfile = $file; + $file = 1 if ($is_git_file); if ($git) { open($FILE, '-|', "git format-patch -M --stdout -1 $filename") || die "$P: $filename: git format-patch failed - $!\n"; @@ -1013,6 +1292,7 @@ while (<$FILE>) { chomp; push(@rawlines, $_); + $vname = qq("$1") if ($filename eq '-' && $_ =~ m/^Subject:\s+(.+)/i); } close($FILE);
@@ -1034,17 +1314,18 @@ @modifierListFile = (); @typeListFile = (); build_types(); + $file = $oldfile if ($is_git_file); }
if (!$quiet) { hash_show_words(%use_type, "Used"); hash_show_words(%ignore_type, "Ignored");
- if ($^V lt 5.10.0) { + if (!$perl_version_ok) { 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. + An upgrade to at least perl $minimum_perl_version is suggested. EOM } if ($exit) { @@ -1079,6 +1360,8 @@ my ($formatted_email) = @_;
my $name = ""; + my $quoted = ""; + my $name_comment = ""; my $address = ""; my $comment = "";
@@ -1109,42 +1392,76 @@ } }
- $name = trim($name); - $name =~ s/^"|"$//g; + # Extract comments from names excluding quoted parts + # "John D. (Doe)" - Do not extract + if ($name =~ s/"(.+)"//) { + $quoted = $1; + } + while ($name =~ s/\s*($balanced_parens)\s*/ /) { + $name_comment .= trim($1); + } + $name =~ s/^[ "]+|[ "]+$//g; + $name = trim("$quoted $name"); + $address = trim($address); $address =~ s/^<|>$//g; + $comment = trim($comment);
if ($name =~ /[^\w -]/i) { ##has "must quote" chars $name =~ s/(?<!\)"/\"/g; ##escape quotes $name = ""$name""; }
- return ($name, $address, $comment); + return ($name, $name_comment, $address, $comment); }
sub format_email { - my ($name, $address) = @_; + my ($name, $name_comment, $address, $comment) = @_;
my $formatted_email;
- $name = trim($name); - $name =~ s/^"|"$//g; + $name =~ s/^[ "]+|[ "]+$//g; $address = trim($address); + $address =~ s/(?:.|,|")+$//; ##trailing commas, dots or quotes
if ($name =~ /[^\w -]/i) { ##has "must quote" chars $name =~ s/(?<!\)"/\"/g; ##escape quotes $name = ""$name""; }
+ $name_comment = trim($name_comment); + $name_comment = " $name_comment" if ($name_comment ne ""); + $comment = trim($comment); + $comment = " $comment" if ($comment ne ""); + if ("$name" eq "") { $formatted_email = "$address"; } else { - $formatted_email = "$name <$address>"; + $formatted_email = "$name$name_comment <$address>"; } - + $formatted_email .= "$comment"; return $formatted_email; }
+sub reformat_email { + my ($email) = @_; + + my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); + return format_email($email_name, $name_comment, $email_address, $comment); +} + +sub same_email_addresses { + my ($email1, $email2) = @_; + + my ($email1_name, $name1_comment, $email1_address, $comment1) = parse_email($email1); + my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2); + + return $email1_name eq $email2_name && + $email1_address eq $email2_address && + $name1_comment eq $name2_comment && + $comment1 eq $comment2; +} + sub which { my ($bin) = @_;
@@ -1178,7 +1495,7 @@ if ($c eq "\t") { $res .= ' '; $n++; - for (; ($n % 8) != 0; $n++) { + for (; ($n % $tabsize) != 0; $n++) { $res .= ' '; } next; @@ -1197,12 +1514,8 @@ sub line_stats { my ($line) = @_;
- # Drop the diff line leader + # Drop the diff line leader and expand tabs $line =~ s/^.//; - - # Treat labels like whitespace when counting indentation - $line =~ s/^( ?$Ident:)/" " x length($1)/e; - $line = expand_tabs($line);
# Pick the indent from the front of the line. @@ -1335,13 +1648,15 @@
my $type = ''; my $level = 0; - my @stack = (['', $level]); + my @stack = (); my $p; my $c; my $len = 0;
my $remainder; while (1) { + @stack = (['', 0]) if ($#stack == -1); + #warn "CSB: blk<$blk> remain<$remain>\n"; # If we are about to drop off the end, pull in more # context. @@ -1375,9 +1690,9 @@ # Handle nested #if/#else. if ($remainder =~ /^#\s*(?:ifndef|ifdef|if)\s/) { push(@stack, [ $type, $level ]); - } elsif ($remainder =~ /^#\s*(?:else|elif)\b/ && $#stack > 0) { - ($type, $level) = @{$stack[$#stack]}; - } elsif ($remainder =~ /^#\s*endif\b/ && $#stack > 0) { + } elsif ($remainder =~ /^#\s*(?:else|elif)\b/) { + ($type, $level) = @{$stack[$#stack - 1]}; + } elsif ($remainder =~ /^#\s*endif\b/) { ($type, $level) = @{pop(@stack)}; }
@@ -1547,9 +1862,9 @@ # Handle nested #if/#else. if ($lines[$line] =~ /^.\s*#\s*(?:ifndef|ifdef|if)\s/) { push(@stack, $level); - } elsif ($lines[$line] =~ /^.\s*#\s*(?:else|elif)\b/ && $#stack > 0) { - $level = $stack[$#stack]; - } elsif ($lines[$line] =~ /^.\s*#\s*endif\b/ && $#stack > 0) { + } elsif ($lines[$line] =~ /^.\s*#\s*(?:else|elif)\b/) { + $level = $stack[$#stack - 1]; + } elsif ($lines[$line] =~ /^.\s*#\s*endif\b/) { $level = pop(@stack); }
@@ -1609,8 +1924,16 @@ sub ctx_locate_comment { my ($first_line, $end_line) = @_;
+ # If c99 comment on the current line, or the line before or after + my ($current_comment) = ($rawlines[$end_line - 1] =~ m@^+.*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line - 2] =~ m@^[+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line] =~ m@^[+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + # Catch a comment on the end of the line itself. - my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/*.**/)\s*(?:\\s*)?$@); + ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/*.**/)\s*(?:\\s*)?$@); return $current_comment if (defined $current_comment);
# Look through the context and try and figure out if there is a @@ -2010,7 +2333,16 @@ splice(@lines, 1, 1); $output = join("\n", @lines); } - $output = (split('\n', $output))[0] . "\n" if ($terse); + + if ($terse) { + $output = (split('\n', $output))[0] . "\n"; + } + + if ($verbose && exists($verbose_messages{$type}) && + !exists($verbose_emitted{$type})) { + $output .= $verbose_messages{$type} . "\n\n"; + $verbose_emitted{$type} = 1; + }
push(our @report, $output);
@@ -2199,7 +2531,7 @@ sub tabify { my ($leading) = @_;
- my $source_indent = 8; + my $source_indent = $tabsize; my $max_spaces_before_tab = $source_indent - 1; my $spaces_to_tab = " " x $source_indent;
@@ -2241,6 +2573,28 @@ return length(expand_tabs(substr($line, 0, $last_openparen))) + 1; }
+sub get_raw_comment { + my ($line, $rawline) = @_; + my $comment = ''; + + for my $i (0 .. (length($line) - 1)) { + if (substr($line, $i, 1) eq "$;") { + $comment .= substr($rawline, $i, 1); + } + } + + return $comment; +} + +sub exclude_global_initialisers { + my ($realfile) = @_; + + # Do not check for BPF programs (tools/testing/selftests/bpf/progs/*.c, samples/bpf/*_kern.c, *.bpf.c). + return $realfile =~ m@^tools/testing/selftests/bpf/progs/.*.c$@ || + $realfile =~ m@^samples/bpf/.*_kern.c$@ || + $realfile =~ m@/bpf/.*.bpf.c$@; +} + sub process { my $filename = shift;
@@ -2257,16 +2611,24 @@
our $clean = 1; my $signoff = 0; + my $author = ''; + my $authorsignoff = 0; + my $author_sob = ''; my $is_patch = 0; + my $is_binding_patch = -1; my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch + my $has_patch_separator = 0; #Found a --- line my $has_commit_log = 0; #Encountered lines before patch + my $commit_log_lines = 0; #Number of commit log lines 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_git_commit_id_linenr = -1; + my $last_blank_line = 0; my $last_coalesced_string_linenr = -1;
@@ -2317,7 +2679,7 @@
if ($rawline=~/^+++\s+(\S+)/) { $setup_docs = 0; - if ($1 =~ m@Documentation/admin-guide/kernel-parameters.rst$@) { + if ($1 =~ m@Documentation/admin-guide/kernel-parameters.txt$@) { $setup_docs = 1; } #next; @@ -2398,6 +2760,7 @@ $sline =~ s/$;/ /g; #with comments as spaces
my $rawline = $rawlines[$linenr - 1]; + my $raw_comment = get_raw_comment($line, $rawline);
# check if it's a mode change, rename or start of a patch if (!$in_commit_log && @@ -2518,6 +2881,19 @@ $check = $check_orig; } $checklicenseline = 1; + + if ($realfile !~ /^MAINTAINERS/) { + my $last_binding_patch = $is_binding_patch; + + $is_binding_patch = () = $realfile =~ m@^(?:Documentation/devicetree/|include/dt-bindings/)@; + + if (($last_binding_patch != -1) && + ($last_binding_patch ^ $is_binding_patch)) { + WARN("DT_SPLIT_BINDING_PATCH", + "DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst\n"); + } + } + next; }
@@ -2529,10 +2905,22 @@
$cnt_lines++ if ($realcnt != 0);
+# Verify the existence of a commit log if appropriate +# 2 is used because a $signature is counted in $commit_log_lines + if ($in_commit_log) { + if ($line !~ /^\s*$/) { + $commit_log_lines++; #could be a $signature + } + } elsif ($has_commit_log && $commit_log_lines < 2) { + WARN("COMMIT_MESSAGE", + "Missing commit description - Add an appropriate one\n"); + $commit_log_lines = 2; #warn only once + } + # 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+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", @@ -2550,10 +2938,61 @@ } }
+# Check the patch for a From: + if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) { + $author = $1; + my $curline = $linenr; + while(defined($rawlines[$curline]) && ($rawlines[$curline++] =~ /^[ \t]\s*(.*)/)) { + $author .= $1; + } + $author = encode("utf8", $author) if ($line =~ /=?utf-8?/i); + $author =~ s/"//g; + $author = reformat_email($author); + } + # Check the patch for a signoff: - if ($line =~ /^\s*signed-off-by:/i) { + if ($line =~ /^\s*signed-off-by:\s*(.*)/i) { $signoff++; $in_commit_log = 0; + if ($author ne '' && $authorsignoff != 1) { + if (same_email_addresses($1, $author)) { + $authorsignoff = 1; + } else { + my $ctx = $1; + my ($email_name, $email_comment, $email_address, $comment1) = parse_email($ctx); + my ($author_name, $author_comment, $author_address, $comment2) = parse_email($author); + + if (lc $email_address eq lc $author_address && $email_name eq $author_name) { + $author_sob = $ctx; + $authorsignoff = 2; + } elsif (lc $email_address eq lc $author_address) { + $author_sob = $ctx; + $authorsignoff = 3; + } elsif ($email_name eq $author_name) { + $author_sob = $ctx; + $authorsignoff = 4; + + my $address1 = $email_address; + my $address2 = $author_address; + + if ($address1 =~ /(\S+)+\S+(@.*)/) { + $address1 = "$1$2"; + } + if ($address2 =~ /(\S+)+\S+(@.*)/) { + $address2 = "$1$2"; + } + if ($address1 eq $address2) { + $authorsignoff = 5; + } + } + } + } + } + +# Check for patch separator + if ($line =~ /^---$/) { + $has_patch_separator = 1; + $in_commit_log = 0; }
# Check if MAINTAINERS is being updated. If so, there's probably no need to @@ -2572,8 +3011,17 @@ my $ucfirst_sign_off = ucfirst(lc($sign_off));
if ($sign_off !~ /$signature_tags/) { - WARN("BAD_SIGN_OFF", - "Non-standard signature: $sign_off\n" . $herecurr); + my $suggested_signature = find_standard_signature($sign_off); + if ($suggested_signature eq "") { + WARN("BAD_SIGN_OFF", + "Non-standard signature: $sign_off\n" . $herecurr); + } else { + if (WARN("BAD_SIGN_OFF", + "Non-standard signature: '$sign_off' - perhaps '$suggested_signature'?\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/; + } + } } if (defined $space_before && $space_before ne "") { if (WARN("BAD_SIGN_OFF", @@ -2601,8 +3049,8 @@ } }
- my ($email_name, $email_address, $comment) = parse_email($email); - my $suggested_email = format_email(($email_name, $email_address)); + my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); + my $suggested_email = format_email(($email_name, $name_comment, $email_address, $comment)); if ($suggested_email eq "") { ERROR("BAD_SIGN_OFF", "Unrecognized email address: '$email'\n" . $herecurr); @@ -2612,11 +3060,77 @@ $dequoted =~ s/" </ </; # Don't force email to have quotes # Allow just an angle bracketed address - if ("$dequoted$comment" ne $email && - "<$email_address>$comment" ne $email && - "$suggested_email$comment" ne $email) { + if (!same_email_addresses($email, $suggested_email)) { + if (WARN("BAD_SIGN_OFF", + "email address '$email' might be better as '$suggested_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/; + } + } + + # Address part shouldn't have comments + my $stripped_address = $email_address; + $stripped_address =~ s/([^()]*)//g; + if ($email_address ne $stripped_address) { + if (WARN("BAD_SIGN_OFF", + "address part of email should not have comments: '$email_address'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/; + } + } + + # Only one name comment should be allowed + my $comment_count = () = $name_comment =~ /([^)]+)/g; + if ($comment_count > 1) { WARN("BAD_SIGN_OFF", - "email address '$email' might be better as '$suggested_email$comment'\n" . $herecurr); + "Use a single name comment in email: '$email'\n" . $herecurr); + } + + + # stable@vger.kernel.org or stable@kernel.org shouldn't + # have an email name. In addition comments should strictly + # begin with a # + if ($email =~ /^.*stable@(?:vger.)?kernel.org/i) { + if (($comment ne "" && $comment !~ /^#.+/) || + ($email_name ne "")) { + my $cur_name = $email_name; + my $new_comment = $comment; + $cur_name =~ s/[a-zA-Z\s-"]+//g; + + # Remove brackets enclosing comment text + # and # from start of comments to get comment text + $new_comment =~ s/^((.*))$/$1/; + $new_comment =~ s/^[(.*)]$/$1/; + $new_comment =~ s/^[\s#]+|\s+$//g; + + $new_comment = trim("$new_comment $cur_name") if ($cur_name ne $new_comment); + $new_comment = " # $new_comment" if ($new_comment ne ""); + my $new_email = "$email_address$new_comment"; + + if (WARN("BAD_STABLE_ADDRESS_STYLE", + "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/; + } + } + } elsif ($comment ne "" && $comment !~ /^(?:#.+|(.+))$/) { + my $new_comment = $comment; + + # Extract comment text from within brackets or + # c89 style /*...*/ comments + $new_comment =~ s/^[(.*)]$/$1/; + $new_comment =~ s/^/*(.*)*/$/$1/; + + $new_comment = trim($new_comment); + $new_comment =~ s/^[^\w]$//; # Single lettered comment with non word character is usually a typo + $new_comment = "($new_comment)" if ($new_comment ne ""); + my $new_email = format_email($email_name, $name_comment, $email_address, $new_comment); + + if (WARN("BAD_SIGN_OFF", + "Unexpected content after email: '$email', should be: '$new_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/; + } } }
@@ -2630,6 +3144,24 @@ } else { $signatures{$sig_nospace} = 1; } + +# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email + if ($sign_off =~ /^co-developed-by:$/i) { + if ($email eq $author) { + WARN("BAD_SIGN_OFF", + "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . "$here\n" . $rawline); + } + if (!defined $lines[$linenr]) { + WARN("BAD_SIGN_OFF", + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); + } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) { + WARN("BAD_SIGN_OFF", + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); + } elsif ($1 ne $email) { + WARN("BAD_SIGN_OFF", + "Co-developed-by and Signed-off-by: name/email do not match \n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); + } + } }
# Check email subject for common tools that don't need to be mentioned @@ -2639,10 +3171,13 @@ "A patch subject line should describe the change not the tool that found it\n" . $herecurr); }
-# Check for unwanted Gerrit info - if ($in_commit_log && $line =~ /^\s*change-id:/i) { - ERROR("GERRIT_CHANGE_ID", - "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); +# Check for Gerrit Change-Ids not in any patch context + if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) { + if (ERROR("GERRIT_CHANGE_ID", + "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } }
# Check if the commit log is in a possible stack dump @@ -2650,8 +3185,10 @@ ($line =~ /^\s*(?:WARNING:|BUG:)/ || $line =~ /^\s*[\s*\d+.\d{6,6}\s*]/ || # timestamp - $line =~ /^\s*[<[0-9a-fA-F]{8,}>]/)) { - # stack dump address + $line =~ /^\s*[<[0-9a-fA-F]{8,}>]/) || + $line =~ /^(?:\s+\w+:\s+[0-9a-fA-F]+){3,3}/ || + $line =~ /^\s*#\d+\s*[[0-9a-fA-F]+]\s*\w+ at [0-9a-fA-F]+/) { + # stack dump address styles $commit_log_possible_stack_dump = 1; }
@@ -2662,8 +3199,8 @@ # file delta changes $line =~ /^\s*(?:[\w.-]+/)++[\w.-]+:/ || # filename then : - $line =~ /^\s*(?:Fixes:|Link:)/i || - # A Fixes: or Link: line + $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i || + # A Fixes: or Link: line or signature tag line $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 72 chars per line)\n" . $herecurr); @@ -2676,11 +3213,30 @@ $commit_log_possible_stack_dump = 0; }
+# Check for lines starting with a # + if ($in_commit_log && $line =~ /^#/) { + if (WARN("COMMIT_COMMENT_SYMBOL", + "Commit log lines starting with '#' are dropped by git as comments\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^/ /; + } + } + # 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|https|BugLink):/i && +# A correctly formed commit description is: +# commit <SHA-1 hash length 12+ chars> ("Complete commit subject") +# with the commit subject '("' prefix and '")' suffix +# This is a fairly compilicated block as it tests for what appears to be +# bare SHA-1 hash with minimum length of 5. It also avoids several types of +# possible SHA-1 matches. +# A commit match can span multiple lines so this block attempts to find a +# complete typical commit on a maximum of 3 lines + if ($perl_version_ok && + $in_commit_log && !$commit_log_possible_stack_dump && + $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink|base-commit):/i && $line !~ /^This reverts commit [0-9a-f]{7,40}/ && - ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + (($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + ($line =~ /\bcommit\s*$/i && defined($rawlines[$linenr]) && $rawlines[$linenr] =~ /^\s*[0-9a-f]{5,}\b/i)) || ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'([]|$)/i && $line !~ /[<[][0-9a-f]{12,40}[>]]/i && $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) { @@ -2690,49 +3246,56 @@ 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 = ""; + my $herectx = $herecurr; + my $has_parens = 0; + my $has_quotes = 0;
- 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); + my $input = $line; + if ($line =~ /(?:\bcommit\s+[0-9a-f]{5,}|\bcommit\s*$)/i) { + for (my $n = 0; $n < 2; $n++) { + if ($input =~ /\bcommit\s+[0-9a-f]{5,}\s*($balanced_parens)/i) { + $orig_desc = $1; + $has_parens = 1; + # Always strip leading/trailing parens then double quotes if existing + $orig_desc = substr($orig_desc, 1, -1); + if ($orig_desc =~ /^".*"$/) { + $orig_desc = substr($orig_desc, 1, -1); + $has_quotes = 1; + } + last; + } + last if ($#lines < $linenr + $n); + $input .= " " . trim($rawlines[$linenr + $n]); + $herectx .= "$rawlines[$linenr + $n]\n"; + } + $herectx = $herecurr if (!$has_parens); }
- $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; + if ($input =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { + $init_char = $1; + $orig_commit = lc($2); + $short = 0 if ($input =~ /\bcommit\s+[0-9a-f]{12,40}/i); + $long = 1 if ($input =~ /\bcommit\s+[0-9a-f]{41,}/i); + $space = 0 if ($input =~ /\bcommit [0-9a-f]/i); + $case = 0 if ($input =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); + } elsif ($input =~ /\b([0-9a-f]{12,40})\b/i) { + $orig_commit = lc($1); }
($id, $description) = git_commit_info($orig_commit, $id, $orig_desc);
if (defined($id) && - ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens)) { + ($short || $long || $space || $case || ($orig_desc ne $description) || !$has_quotes) && + $last_git_commit_id_linenr != $linenr - 1) { ERROR("GIT_COMMIT_ID", - "Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: '${init_char}ommit $id ("$description")'\n" . $herecurr); + "Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: '${init_char}ommit $id ("$description")'\n" . $herectx); } + #don't report the next line if this line ends in commit and the sha1 hash is the next line + $last_git_commit_id_linenr = $linenr if ($line =~ /\bcommit\s*$/i); }
# Check for added, moved or deleted files @@ -2747,6 +3310,14 @@ "added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr); }
+# Check for adding new DT bindings not in schema format + if (!$in_commit_log && + ($line =~ /^new file mode\s*\d+\s*$/) && + ($realfile =~ m@^Documentation/devicetree/bindings/.*.txt$@)) { + WARN("DT_SCHEMA_BINDING_PATCH", + "DT bindings should be in DT schema format. See: Documentation/devicetree/bindings/writing-schema.rst\n"); + } + # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:+|-| |\ No newline|$)}) { ERROR("CORRUPTED_PATCH", @@ -2808,21 +3379,89 @@ # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:+|Subject:)/i)) { - while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) { + while ($rawline =~ /(?:^|[^\w-'`])($misspellings)(?:[^\w-'`]|$)/gi) { my $typo = $1; + my $blank = copy_spacing($rawline); + my $ptr = substr($blank, 0, $-[1]) . "^" x length($typo); + my $hereptr = "$hereline$ptr\n"; 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_level = &WARN; $msg_level = &CHK if ($file); if (&{$msg_level}("TYPO_SPELLING", - "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && + "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $hereptr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/; } } }
+# check for invalid commit id + if ($in_commit_log && $line =~ /(^fixes:|\bcommit)\s+([0-9a-f]{6,40})\b/i) { + my $id; + my $description; + ($id, $description) = git_commit_info($2, undef, undef); + if (!defined($id)) { + WARN("UNKNOWN_COMMIT_ID", + "Unknown commit id '$2', maybe rebased or not pulled?\n" . $herecurr); + } + } + +# check for repeated words separated by a single space +# avoid false positive from list command eg, '-rw-r--r-- 1 root root' + if (($rawline =~ /^+/ || $in_commit_log) && + $rawline !~ /[bcCdDlMnpPs?-][rwxsStT-]{9}/) { + pos($rawline) = 1 if (!$in_commit_log); + while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) { + + my $first = $1; + my $second = $2; + my $start_pos = $-[1]; + my $end_pos = $+[2]; + if ($first =~ /(?:struct|union|enum)/) { + pos($rawline) += length($first) + length($second) + 1; + next; + } + + next if (lc($first) ne lc($second)); + next if ($first eq 'long'); + + # check for character before and after the word matches + my $start_char = ''; + my $end_char = ''; + $start_char = substr($rawline, $start_pos - 1, 1) if ($start_pos > ($in_commit_log ? 0 : 1)); + $end_char = substr($rawline, $end_pos, 1) if ($end_pos < length($rawline)); + + next if ($start_char =~ /^\S$/); + next if (index(" \t.,;?!", $end_char) == -1); + + # avoid repeating hex occurrences like 'ff ff fe 09 ...' + if ($first =~ /\b[0-9a-f]{2,}\b/i) { + next if (!exists($allow_repeated_words{lc($first)})); + } + + if (WARN("REPEATED_WORD", + "Possible repeated word: '$first'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$first $second\b/$first/; + } + } + + # if it's a repeated word on consecutive lines in a comment block + if ($prevline =~ /$;+\s*$/ && + $prevrawline =~ /($word_pattern)\s*$/) { + my $last_word = $1; + if ($rawline =~ /^+\s**\s*$last_word /) { + if (WARN("REPEATED_WORD", + "Possible repeated word: '$last_word'\n" . $hereprev) && + $fix) { + $fixed[$fixlinenr] =~ s/(+\s**\s*)$last_word /$1/; + } + } + } + } + # ignore non-hunk lines and lines being removed next if (!$hunk_line || $line =~ /^-/);
@@ -2881,11 +3520,7 @@
if ($lines[$ln - 1] =~ /^+\s*(?:bool|tristate|prompt)\s*["']/) { $is_start = 1; - } elsif ($lines[$ln - 1] =~ /^+\s*(?:help|---help---)\s*$/) { - if ($lines[$ln - 1] =~ "---help---") { - WARN("CONFIG_DESCRIPTION", - "prefer 'help' over '---help---' for new help texts\n" . $herecurr); - } + } elsif ($lines[$ln - 1] =~ /^+\s*(?:---)?help(?:---)?$/) { $length = -1; }
@@ -2912,22 +3547,44 @@ #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; }
-# check for MAINTAINERS entries that don't have the right form - if ($realfile =~ /^MAINTAINERS$/ && - $rawline =~ /^+[A-Z]:/ && - $rawline !~ /^+[A-Z]:\t\S/) { - if (WARN("MAINTAINERS_STYLE", - "MAINTAINERS entries use one tab after TYPE:\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/^(+[A-Z]):\s*/$1:\t/; +# check MAINTAINERS entries + if ($realfile =~ /^MAINTAINERS$/) { +# check MAINTAINERS entries for the right form + if ($rawline =~ /^+[A-Z]:/ && + $rawline !~ /^+[A-Z]:\t\S/) { + if (WARN("MAINTAINERS_STYLE", + "MAINTAINERS entries use one tab after TYPE:\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^(+[A-Z]):\s*/$1:\t/; + } } - } - -# 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); +# check MAINTAINERS entries for the right ordering too + my $preferred_order = 'MRLSWQBCPTFXNK'; + if ($rawline =~ /^+[A-Z]:/ && + $prevrawline =~ /^[+ ][A-Z]:/) { + $rawline =~ /^+([A-Z]):\s*(.*)/; + my $cur = $1; + my $curval = $2; + $prevrawline =~ /^[+ ]([A-Z]):\s*(.*)/; + my $prev = $1; + my $prevval = $2; + my $curindex = index($preferred_order, $cur); + my $previndex = index($preferred_order, $prev); + if ($curindex < 0) { + WARN("MAINTAINERS_STYLE", + "Unknown MAINTAINERS entry type: '$cur'\n" . $herecurr); + } else { + if ($previndex >= 0 && $curindex < $previndex) { + WARN("MAINTAINERS_STYLE", + "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev); + } elsif ((($prev eq 'F' && $cur eq 'F') || + ($prev eq 'X' && $cur eq 'X')) && + ($prevval cmp $curval) > 0) { + WARN("MAINTAINERS_STYLE", + "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev); + } + } + } }
if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && @@ -2952,7 +3609,7 @@ my @compats = $rawline =~ /"([a-zA-Z0-9-,.+_]+)"/g;
my $dt_path = $root . "/Documentation/devicetree/bindings/"; - my $vp_file = $dt_path . "vendor-prefixes.txt"; + my $vp_file = $dt_path . "vendor-prefixes.yaml";
foreach my $compat (@compats) { my $compat2 = $compat; @@ -2967,7 +3624,7 @@
next if $compat !~ /^([a-zA-Z0-9-]+),/; my $vendor = $1; - `grep -Eq "^$vendor\b" $vp_file`; + `grep -Eq "\"\^\Q$vendor\E,\.\*\":" $vp_file`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", "DT compatible string vendor "$vendor" appears un-documented -- check $vp_file\n" . $herecurr); @@ -2985,23 +3642,62 @@ $comment = '/*'; } elsif ($realfile =~ /.(c|dts|dtsi)$/) { $comment = '//'; - } elsif (($checklicenseline == 2) || $realfile =~ /.(sh|pl|py|awk|tc)$/) { + } elsif (($checklicenseline == 2) || $realfile =~ /.(sh|pl|py|awk|tc|yaml)$/) { $comment = '#'; } elsif ($realfile =~ /.rst$/) { $comment = '..'; }
+# check SPDX comment style for .[chsS] files + if ($realfile =~ /.[chsS]$/ && + $rawline =~ /SPDX-License-Identifier:/ && + $rawline !~ m@^+\s*\Q$comment\E\s*@) { + WARN("SPDX_LICENSE_TAG", + "Improper SPDX comment style for '$realfile', please use '$comment' instead\n" . $herecurr); + } + if ($comment !~ /^$/ && - $rawline !~ /^+\Q$comment\E SPDX-License-Identifier: /) { + $rawline !~ m@^+\Q$comment\E SPDX-License-Identifier: @) { WARN("SPDX_LICENSE_TAG", "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr); + } elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) { + my $spdx_license = $1; + if (!is_SPDX_License_valid($spdx_license)) { + WARN("SPDX_LICENSE_TAG", + "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr); + } + if ($realfile =~ m@^Documentation/devicetree/bindings/@ && + not $spdx_license =~ /GPL-2.0.*BSD-2-Clause/) { + my $msg_level = &WARN; + $msg_level = &CHK if ($file); + if (&{$msg_level}("SPDX_LICENSE_TAG", + + "DT binding documents should be licensed (GPL-2.0-only OR BSD-2-Clause)\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/SPDX-License-Identifier: .*/SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)/; + } + } } } }
+# check for embedded filenames + if ($rawline =~ /^+.*\Q$realfile\E/) { + WARN("EMBEDDED_FILENAME", + "It's generally not useful to have the filename in the file\n" . $herecurr); + } + # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /.(h|c|s|S|sh|dtsi|dts)$/);
+# check for using SPDX-License-Identifier on the wrong line number + if ($realline != $checklicenseline && + $rawline =~ /\bSPDX-License-Identifier:/ && + substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { + WARN("SPDX_LICENSE_TAG", + "Misplaced SPDX-License-Identifier tag - use line $checklicenseline instead\n" . $herecurr); + } + # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -3059,22 +3755,34 @@
if ($msg_type ne "" && (show_type("LONG_LINE") || show_type($msg_type))) { - WARN($msg_type, - "line over $max_line_length characters\n" . $herecurr); + my $msg_level = &WARN; + $msg_level = &CHK if ($file); + &{$msg_level}($msg_type, + "line length of $length exceeds $max_line_length columns\n" . $herecurr); } }
# check for adding lines without a newline. if ($line =~ /^+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\ No newline at end of file/) { - WARN("MISSING_EOF_NEWLINE", - "adding a line without newline at end of file\n" . $herecurr); + if (WARN("MISSING_EOF_NEWLINE", + "adding a line without newline at end of file\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr+1, "No newline at end of file"); + } + } + +# check for .L prefix local symbols in .S files + if ($realfile =~ /.S$/ && + $line =~ /^+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*(\s*.L/) { + WARN("AVOID_L_PREFIX", + "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr); }
# check we are in a valid source file C or perl if not then ignore this hunk 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. +# more than $tabsize must use tabs. if ($rawline =~ /^+\s* \t\s*\S/ || $rawline =~ /^+\s* \s*/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; @@ -3093,7 +3801,7 @@ "please, no space before tabs\n" . $herevet) && $fix) { while ($fixed[$fixlinenr] =~ - s/(^+.*) {8,8}\t/$1\t\t/) {} + s/(^+.*) {$tabsize,$tabsize}\t/$1\t\t/) {} while ($fixed[$fixlinenr] =~ s/(^+.*) +\t/$1\t/) {} } @@ -3101,31 +3809,45 @@
# check for assignments on the start of a line if ($sline =~ /^+\s+($Assignment)[^=]/) { - CHK("ASSIGNMENT_CONTINUATIONS", - "Assignment operator '$1' should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("ASSIGNMENT_CONTINUATIONS", + "Assignment operator '$1' should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^+/) { + # add assignment operator to the previous line, remove from current line + $fixed[$fixlinenr - 1] .= " $operator"; + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } }
# check for && or || at the start of a line if ($rawline =~ /^+\s*(&&|||)/) { - CHK("LOGICAL_CONTINUATIONS", - "Logical continuations should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("LOGICAL_CONTINUATIONS", + "Logical continuations should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^+/) { + # insert logical operator at last non-comment, non-whitepsace char on previous line + $prevline =~ /[\s$;]*$/; + my $line_end = substr($prevrawline, $-[0]); + $fixed[$fixlinenr - 1] =~ s/\Q$line_end\E$/ $operator$line_end/; + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } }
# check indentation starts on a tab stop - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $sline =~ /^+\t+( +)(?:$c90_Keywords\b|{\s*$|}\s*(?:else\b|while\b|\s*$)|$Declare\s*$Ident\s*[;=])/) { my $indent = length($1); - if ($indent % 8) { + if ($indent % $tabsize) { if (WARN("TABSTOP", "Statements should start on a tabstop\n" . $herecurr) && $fix) { - $fixed[$fixlinenr] =~ s@(^+\t+) +@$1 . "\t" x ($indent/8)@e; + $fixed[$fixlinenr] =~ s@(^+\t+) +@$1 . "\t" x ($indent/$tabsize)@e; } } }
# check multi-line statement indentation matches previous line - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $prevline =~ /^+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|(\s**\s*$Ident\s*))\s*|(?:*\s*)*$Lval\s*=\s*$Ident\s*)(.*(&&||||,)\s*$/) { $prevline =~ /^+(\t*)(.*)$/; my $oldindent = $1; @@ -3137,8 +3859,8 @@ my $newindent = $2;
my $goodtabindent = $oldindent . - "\t" x ($pos / 8) . - " " x ($pos % 8); + "\t" x ($pos / $tabsize) . + " " x ($pos % $tabsize); my $goodspaceindent = $oldindent . " " x $pos;
if ($newindent ne $goodtabindent && @@ -3176,7 +3898,7 @@ if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^+[ \t]*/*[ \t]*$/ && $rawline =~ /^+[ \t]**/ && - $realline > 2) { + $realline > 3) { # Do not warn about the initial copyright comment block after SPDX-License-Identifier WARN("NETWORKING_BLOCK_COMMENT_STYLE", "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev); } @@ -3258,43 +3980,48 @@ }
# check for missing blank lines after declarations - if ($sline =~ /^+\s+\S/ && #Not at char 1 - # actual declarations - ($prevline =~ /^+\s+$Declare\s*$Ident\s*[=,;:[]/ || +# (declarations must have the same indentation and not be at the start of line) + if (($prevline =~ /+(\s+)\S/) && $sline =~ /^+$1\S/) { + # use temporaries + my $sl = $sline; + my $pl = $prevline; + # remove $Attribute/$Sparse uses to simplify comparisons + $sl =~ s/\b(?:$Attribute|$Sparse)\b//g; + $pl =~ s/\b(?:$Attribute|$Sparse)\b//g; + if (($pl =~ /^+\s+$Declare\s*$Ident\s*[=,;:[]/ || # function pointer declarations - $prevline =~ /^+\s+$Declare\s*(\s**\s*$Ident\s*)\s*[=,;:[(]/ || + $pl =~ /^+\s+$Declare\s*(\s**\s*$Ident\s*)\s*[=,;:[(]/ || # foo bar; where foo is some local typedef or #define - $prevline =~ /^+\s+$Ident(?:\s+|\s**\s*)$Ident\s*[=,;[]/ || + $pl =~ /^+\s+$Ident(?:\s+|\s**\s*)$Ident\s*[=,;[]/ || # known declaration macros - $prevline =~ /^+\s+$declaration_macros/) && + $pl =~ /^+\s+$declaration_macros/) && # for "else if" which can look like "$Ident $Ident" - !($prevline =~ /^+\s+$c90_Keywords\b/ || + !($pl =~ /^+\s+$c90_Keywords\b/ || # other possible extensions of declaration lines - $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + $pl =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || # not starting a section or a macro "" extended line - $prevline =~ /(?:{\s*|\)$/) && + $pl =~ /(?:{\s*|\)$/) && # looks like a declaration - !($sline =~ /^+\s+$Declare\s*$Ident\s*[=,;:[]/ || + !($sl =~ /^+\s+$Declare\s*$Ident\s*[=,;:[]/ || # function pointer declarations - $sline =~ /^+\s+$Declare\s*(\s**\s*$Ident\s*)\s*[=,;:[(]/ || + $sl =~ /^+\s+$Declare\s*(\s**\s*$Ident\s*)\s*[=,;:[(]/ || # foo bar; where foo is some local typedef or #define - $sline =~ /^+\s+$Ident(?:\s+|\s**\s*)$Ident\s*[=,;[]/ || + $sl =~ /^+\s+$Ident(?:\s+|\s**\s*)$Ident\s*[=,;[]/ || # known declaration macros - $sline =~ /^+\s+$declaration_macros/ || + $sl =~ /^+\s+$declaration_macros/ || # start of struct or union or enum - $sline =~ /^+\s+(?:union|struct|enum|typedef)\b/ || + $sl =~ /^+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || # start or end of block or continuation of declaration - $sline =~ /^+\s+(?:$|[{}.#"?:([])/ || + $sl =~ /^+\s+(?:$|[{}.#"?:([])/ || # bitfield continuation - $sline =~ /^+\s+$Ident\s*:\s*\d+\s*[,;]/ || + $sl =~ /^+\s+$Ident\s*:\s*\d+\s*[,;]/ || # other possible extensions of declaration lines - $sline =~ /^+\s+(?\s*(?:$Compare|$Assignment|$Operators)/) && - # indentation of previous and current line are the same - (($prevline =~ /+(\s+)\S/) && $sline =~ /^+$1\S/)) { - if (WARN("LINE_SPACING", - "Missing a blank line after declarations\n" . $hereprev) && - $fix) { - fix_insert_line($fixlinenr, "+"); + $sl =~ /^+\s+(?\s*(?:$Compare|$Assignment|$Operators)/)) { + if (WARN("LINE_SPACING", + "Missing a blank line after declarations\n" . $hereprev) && + $fix) { + fix_insert_line($fixlinenr, "+"); + } } }
@@ -3347,12 +4074,16 @@ }
# check indentation of a line with a break; -# if the previous line is a goto or return and is indented the same # of tabs +# if the previous line is a goto, return or break +# and is indented the same # of tabs if ($sline =~ /^+([\t]+)break\s*;\s*$/) { my $tabs = $1; - if ($prevline =~ /^+$tabs(?:goto|return)\b/) { - WARN("UNNECESSARY_BREAK", - "break is not useful after a goto or return\n" . $hereprev); + if ($prevline =~ /^+$tabs(goto|return|break)\b/) { + if (WARN("UNNECESSARY_BREAK", + "break is not useful after a $1\n" . $hereprev) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } }
@@ -3609,58 +4340,14 @@ #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 && $s ne '' && - (($sindent % 8) != 0 || + (($sindent % $tabsize) != 0 || ($sindent < $indent) || ($sindent == $indent && ($s !~ /^\s*(?:}|{|else\b)/)) || - ($sindent > $indent + 8))) { + ($sindent > $indent + $tabsize))) { WARN("SUSPECT_CODE_INDENT", "suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); } - -# Also check if the next statement after the previous condition has the same indent - my ($stat_next, undef, $line_nr_next_next) = - ctx_statement_block($line_nr_next, $remain_next, $off_next); - my $s_next = $stat_next; - - # Remove line prefixes - $s_next =~ s/\n./\n/g; - - # Remove any comments - $s_next =~ s/$;//g; - - # Remove any leading labels - $s_next =~ s/\n( ?$Ident:)/"\n" . " " x length($1)/eg; - - # Skip this check for in case next statement starts with 'else' - if ($s_next !~ /\belse\b/) { - - # Remove while that belongs to a do {} while - if ($stat =~ /\bdo\b/) { - $s_next =~ s/^.*\bwhile\b\s*($balanced_parens)\s*?//; - } - - # Remove blank lines - $s_next =~ s/\s*\?\n//g; - - # Get the real next lines - my $next_nof_lines = $line_nr_next_next - $line_nr_next; - my $stat_next_real = raw_line($line_nr_next, $next_nof_lines); - if (!defined($stat_next_real)) { - $stat_next_real = ""; - } elsif ($next_nof_lines > 1) { - $stat_next_real = "[...]\n$stat_next_real"; - } - my (undef, $nindent) = line_stats('+' . $s_next); - - #print "stat_next<$stat_next> stat<$stat> indent<$indent> nindent<$nindent> s_next<$s_next> stat_next_real<$stat_next_real>\n"; - - if ($nindent > $indent) { - WARN("SUSPICIOUS_CODE_INDENT", - "suspicious code indentation after conditional statements\n" . - $herecurr . "$stat_real\n$stat_next_real\n"); - } - } }
# Track the 'values' across context and added lines. @@ -3679,6 +4366,17 @@ #ignore lines not being added next if ($line =~ /^[^+]/);
+# check for self assignments used to avoid compiler warnings +# e.g.: int foo = foo, *bar = NULL; +# struct foo bar = *(&(bar)); + if ($line =~ /^+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) { + my $var = $1; + if ($line =~ /^+\s*(?:$Declare)?$var\s*=\s*(?:$var|*\s*(?\s*&\s*(?\s*$var\s*)?\s*)?)\s*[;,]/) { + WARN("SELF_ASSIGNMENT", + "Do not use self-assignments to avoid compiler warnings\n" . $herecurr); + } + } + # check for dereferences that span multiple lines if ($prevline =~ /^+.*$Lval\s*(?:.|->)\s*$/ && $line =~ /^+\s*(?!#\s*(?!define\s+|if))\s*$Lval/) { @@ -3794,13 +4492,13 @@ if (defined $realline_next && exists $lines[$realline_next - 1] && !defined $suppress_export{$realline_next} && - ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*((.*))/ || - $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*((.*))/)) { + ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*((.*))/)) { # Handle definitions which produce identifiers with # a prefix: # XXX(foo); # EXPORT_SYMBOL(something_foo); my $name = $1; + $name =~ s/^\s*($Ident).*/$1/; if ($stat =~ /^(?:.\s*}\s*\n)?.([A-Z_]+)\s*(\s*($Ident)/ && $name =~ /^${Ident}_$2/) { #print "FOO C name<$name>\n"; @@ -3822,8 +4520,7 @@ } if (!defined $suppress_export{$linenr} && $prevline =~ /^.\s*$/ && - ($line =~ /EXPORT_SYMBOL.*((.*))/ || - $line =~ /EXPORT_UNUSED_SYMBOL.*((.*))/)) { + ($line =~ /EXPORT_SYMBOL.*((.*))/)) { #print "FOO B <$lines[$linenr - 1]>\n"; $suppress_export{$linenr} = 2; } @@ -3834,7 +4531,8 @@ }
# check for global initialisers. - if ($line =~ /^+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) { + if ($line =~ /^+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/ && + !exclude_global_initialisers($realfile)) { if (ERROR("GLOBAL_INITIALISERS", "do not initialise globals to $1\n" . $herecurr) && $fix) { @@ -3858,19 +4556,48 @@ "type '$tmp' should be specified in [[un]signed] [short|int|long|long long] order\n" . $herecurr); }
+# check for unnecessary <signed> int declarations of short/long/long long + while ($sline =~ m{\b($TypeMisordered(\s**)*|$C90_int_types)\b}g) { + my $type = trim($1); + next if ($type !~ /\bint\b/); + next if ($type !~ /\b(?:short|long\s+long|long)\b/); + my $new_type = $type; + $new_type =~ s/\b\s*int\s*\b/ /; + $new_type =~ s/\b\s*(?:un)?signed\b\s*/ /; + $new_type =~ s/^const\s+//; + $new_type = "unsigned $new_type" if ($type =~ /\bunsigned\b/); + $new_type = "const $new_type" if ($type =~ /^const\b/); + $new_type =~ s/\s+/ /g; + $new_type = trim($new_type); + if (WARN("UNNECESSARY_INT", + "Prefer '$new_type' over '$type' as the int is unnecessary\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b\Q$type\E\b/$new_type/; + } + } + # check for static const char * arrays. if ($line =~ /\bstatic\s+const\s+char\s**\s*(\w+)\s*[\s*]\s*=\s*/) { WARN("STATIC_CONST_CHAR_ARRAY", "static const char * array should probably be static const char * const\n" . $herecurr); - } + } + +# check for initialized const char arrays that should be static const + if ($line =~ /^+\s*const\s+(char|unsigned\s+char|_*u8|(?:[us]_)?int8_t)\s+\w+\s*[\s*(?:\w+\s*)?]\s*=\s*"/) { + if (WARN("STATIC_CONST_CHAR_ARRAY", + "const array should probably be static const\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/(^.\s*)const\b/${1}static const/; + } + }
# check for static char foo[] = "bar" declarations. if ($line =~ /\bstatic\s+char\s+(\w+)\s*[\s*]\s*=\s*"/) { WARN("STATIC_CONST_CHAR_ARRAY", "static char array declaration should probably be static const char\n" . $herecurr); - } + }
# check for const <foo> const where <foo> is not a pointer or array type if ($sline =~ /\bconst\s+($BasicType)\s+const\b/) { @@ -3884,12 +4611,24 @@ } }
+# check for const static or static <non ptr type> const declarations +# prefer 'static const <foo>' over 'const static <foo>' and 'static <foo> const' + if ($sline =~ /^+\s*const\s+static\s+($Type)\b/ || + $sline =~ /^+\s*static\s+($BasicType)\s+const\b/) { + if (WARN("STATIC_CONST", + "Move const after static - use 'static const $1'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bconst\s+static\b/static const/; + $fixed[$fixlinenr] =~ s/\bstatic\s+($BasicType)\s+const\b/static const $1/; + } + } + # 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", "char * array declaration might be better as static const\n" . $herecurr); - } + }
# check for sizeof(foo)/sizeof(foo[0]) that could be ARRAY_SIZE(foo) if ($line =~ m@\bsizeof\s*(\s*($Lval)\s*)@) { @@ -3905,7 +4644,7 @@ }
# check for function declarations without arguments like "int foo()" - if ($line =~ /(\b$Type\s+$Ident)\s*(\s*)/) { + if ($line =~ /(\b$Type\s*$Ident)\s*(\s*)/) { if (ERROR("FUNCTION_WITHOUT_ARGS", "Bad function definition - $1() should probably be $1(void)\n" . $herecurr) && $fix) { @@ -4006,25 +4745,23 @@ "printk() should include KERN_<LEVEL> facility level\n" . $herecurr); }
- if ($line =~ /\bprintk\s*(\s*KERN_([A-Z]+)/) { - my $orig = $1; +# prefer variants of (subsystem|netdev|dev|pr)_<level> to printk(KERN_<LEVEL> + if ($line =~ /\b(printk(_once|_ratelimited)?)\s*(\s*KERN_([A-Z]+)/) { + my $printk = $1; + my $modifier = $2; + my $orig = $3; + $modifier = "" if (!defined($modifier)); my $level = lc($orig); $level = "warn" if ($level eq "warning"); my $level2 = $level; $level2 = "dbg" if ($level eq "debug"); + $level .= $modifier; + $level2 .= $modifier; WARN("PREFER_PR_LEVEL", - "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then pr_$level(... to printk(KERN_$orig ...\n" . $herecurr); + "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then pr_$level(... to $printk(KERN_$orig ...\n" . $herecurr); }
- if ($line =~ /\bpr_warning\s*(/) { - if (WARN("PREFER_PR_LEVEL", - "Prefer pr_warn(... to pr_warning(...\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ - s/\bpr_warning\b/pr_warn/; - } - } - +# prefer dev_<level> to dev_printk(KERN_<LEVEL> if ($line =~ /\bdev_printk\s*(\s*KERN_([A-Z]+)/) { my $orig = $1; my $level = lc($orig); @@ -4034,6 +4771,12 @@ "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr); }
+# trace_printk should not be used in production code. + if ($line =~ /\b(trace_printk|trace_puts|ftrace_vprintk)\s*(/) { + WARN("TRACE_PRINTK", + "Do not use $1() in production code (this can be ignored if built only with a debug config option)\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. @@ -4042,9 +4785,20 @@ "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr); }
+# ENOTSUPP is not a standard error code and should be avoided in new patches. +# Folks usually mean EOPNOTSUPP (also called ENOTSUP), when they type ENOTSUPP. +# Similarly to ENOSYS warning a small number of false positives is expected. + if (!$file && $line =~ /\bENOTSUPP\b/) { + if (WARN("ENOTSUPP", + "ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bENOTSUPP\b/EOPNOTSUPP/; + } + } + # function brace can't be on same line, except for #defines of do while, # or if closed on same line - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $sline =~ /$Type\s*$Ident\s*$balanced_parens\s*{/ && $sline !~ /#\s*define\b.*do\s*{/ && $sline !~ /}/) { @@ -4053,7 +4807,7 @@ $fix) { fix_delete_line($fixlinenr, $rawline); my $fixed_line = $rawline; - $fixed_line =~ /(^..*$Type\s*$Ident(.*)\s*){(.*)$/; + $fixed_line =~ /(^..*$Type\s*$Ident(.*)\s*){(.*)$/; my $line1 = $1; my $line2 = $2; fix_insert_line($fixlinenr, ltrim($line1)); @@ -4465,7 +5219,7 @@ # A colon needs no spaces before when it is # terminating a case value or a label. } elsif ($opv eq ':C' || $opv eq ':L') { - if ($ctx =~ /Wx./) { + if ($ctx =~ /Wx./ and $realfile !~ m@.*.lds.h$@) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); @@ -4483,7 +5237,7 @@ ($op eq '>' && $ca =~ /<\S+@\S+$/)) { - $ok = 1; + $ok = 1; }
# for asm volatile statements @@ -4549,7 +5303,7 @@ ## $line !~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Type\s*$Ident.*/) { ## ## # Remove any bracketed sections to ensure we do not -## # falsly report the parameters of functions. +## # falsely report the parameters of functions. ## my $ln = $line; ## while ($ln =~ s/([^()]*)//g) { ## } @@ -4561,12 +5315,12 @@
#need space before brace following if, while, etc if (($line =~ /(.*){/ && $line !~ /($Type){/) || - $line =~ /do{/) { + $line =~ /\b(?:else|do){/) { if (ERROR("SPACING", "space required before the open brace '{'\n" . $herecurr) && $fix) { #coreboot - Open braces must be escaped in regex - $fixed[$fixlinenr] =~ s/^(+.*(?:do|))){/$1 {/; + $fixed[$fixlinenr] =~ s/^(+.*(?:do|else|))){/$1 {/; } }
@@ -4580,7 +5334,7 @@
# closing brace should have a space following it when it has anything # on the line - if ($line =~ /}(?!(?:,|;|)))\S/) { + if ($line =~ /}(?!(?:,|;|)|}))\S/) { if (ERROR("SPACING", "space required after that close brace '}'\n" . $herecurr) && $fix) { @@ -4657,7 +5411,7 @@ # check for unnecessary parentheses around comparisons in if uses # when !drivers/staging or command-line uses --strict if (($realfile !~ m@^(?:drivers/staging/)@ || $check_orig) && - $^V && $^V ge 5.10.0 && defined($stat) && + $perl_version_ok && defined($stat) && $stat =~ /(^.\s*if\s*($balanced_parens))/) { my $if_stat = $1; my $test = substr($2, 1, -1); @@ -4680,9 +5434,13 @@ } }
-#goto labels aren't indented, allow a single space however - if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and - !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { +# check that goto labels aren't indented (allow a single space indentation) +# and ignore bitfield definitions like foo:1 +# Strictly, labels can have whitespace after the identifier and before the : +# but this is not allowed here as many ?: uses would appear to be labels + if ($sline =~ /^.\s+[A-Za-z_][A-Za-z\d_]*:(?!\s*\d+)/ && + $sline !~ /^. [A-Za-z\d_][A-Za-z\d_]*:/ && + $sline !~ /^.\s+default:/) { if (WARN("INDENTED_LABEL", "labels should not be indented\n" . $herecurr) && $fix) { @@ -4691,10 +5449,21 @@ } }
+# check if a statement with a comma should be two statements like: +# foo = bar(), /* comma should be semicolon */ +# bar = baz(); + if (defined($stat) && + $stat =~ /^+\s*(?:$Lval\s*$Assignment\s*)?$FuncArg\s*,\s*(?:$Lval\s*$Assignment\s*)?$FuncArg\s*;\s*$/) { + my $cnt = statement_rawlines($stat); + my $herectx = get_stat_here($linenr, $cnt, $here); + WARN("SUSPECT_COMMA_SEMICOLON", + "Possible comma where semicolon could be used\n" . $herectx); + } + # return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)(/s) { my $spacing = $1; - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $stat =~ /^.\s*return\s*($balanced_parens)\s*;\s*$/) { my $value = $1; $value = deparenthesize($value); @@ -4718,10 +5487,10 @@ $lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { WARN("RETURN_VOID", "void function return statements are not generally useful\n" . $hereprev); - } + }
# if statements using unnecessary parentheses - ie: if ((foo == bar)) - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /\bif\s*((?:(\s*){2,})/) { my $openparens = $1; my $count = $openparens =~ tr@(@(@; @@ -4738,7 +5507,7 @@ # 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 && + if ($perl_version_ok && $line =~ /^+(.*)\b($Constant|[A-Z_][A-Z0-9_]*)\s*($Compare)\s*($LvalOrFunc)/) { my $lead = $1; my $const = $2; @@ -4766,7 +5535,7 @@ # 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') { + if ($name ne 'EOF' && $name ne 'ERROR' && $name !~ /^EPOLL/) { WARN("USE_NEGATIVE_ERRNO", "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr); } @@ -4811,15 +5580,37 @@ my ($s, $c) = ($stat, $cond);
if ($c =~ /\bif\s*(.*[^<>!=]=[^=].*/s) { - ERROR("ASSIGN_IN_IF", - "do not use assignment in if condition\n" . $herecurr); + if (ERROR("ASSIGN_IN_IF", + "do not use assignment in if condition\n" . $herecurr) && + $fix && $perl_version_ok) { + if ($rawline =~ /^+(\s+)if\s*(\s*(!)?\s*(\s*(($Lval)\s*=\s*$LvalOrFunc)\s*)\s*(?:($Compare)\s*($FuncArg))?\s*)\s*({)?\s*$/) { + my $space = $1; + my $not = $2; + my $statement = $3; + my $assigned = $4; + my $test = $8; + my $against = $9; + my $brace = $15; + fix_delete_line($fixlinenr, $rawline); + fix_insert_line($fixlinenr, "$space$statement;"); + my $newline = "${space}if ("; + $newline .= '!' if defined($not); + $newline .= '(' if (defined $not && defined($test) && defined($against)); + $newline .= "$assigned"; + $newline .= " $test $against" if (defined($test) && defined($against)); + $newline .= ')' if (defined $not && defined($test) && defined($against)); + $newline .= ')'; + $newline .= " {" if (defined($brace)); + fix_insert_line($fixlinenr + 1, $newline); + } + } }
# Find out what is on the end of the line after the # conditional. substr($s, 0, length($c), ''); $s =~ s/\n.*//g; - $s =~ s/$;//g; # Remove any comments + $s =~ s/$;//g; # Remove any comments if (length($c) && $s !~ /^\s*{?\s*\*\s*$/ && $c !~ /}\s*while\s*/) { @@ -4858,7 +5649,7 @@ # if and else should not have general statements after it if ($line =~ /^.\s*(?:}\s*)?else\b(.*)/) { my $s = $1; - $s =~ s/$;//g; # Remove any comments + $s =~ s/$;//g; # Remove any comments if ($s !~ /^\s*(?:\sif|(?:{|)\s*\?\s*$)/) { ERROR("TRAILING_STATEMENTS", "trailing statements should be on next line\n" . $herecurr); @@ -4930,24 +5721,16 @@ while ($line =~ m{($Constant|$Lval)}g) { my $var = $1;
-#gcc binary extension - if ($var =~ /^$Binary$/) { - if (WARN("GCC_BINARY_CONSTANT", - "Avoid gcc v4.3+ binary constant extension: <$var>\n" . $herecurr) && - $fix) { - my $hexval = sprintf("0x%x", oct($var)); - $fixed[$fixlinenr] =~ - s/\b$var\b/$hexval/; - } - } - #CamelCase if ($var !~ /^$Constant$/ && $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && +#Ignore some autogenerated defines and enum values + $var !~ /^(?:[A-Z]+_){1,5}[A-Z]{1,3}[a-z]/ && #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_]+)?$/ && +#Ignore SI style variants like nS, mV and dB +#(ie: max_uV, regulator_min_uA_show, RANGE_mA_VALUE) + $var !~ /^(?:[a-z0-9_]*|[A-Z0-9_]*)?_?[a-z][A-Z](?:_[a-z0-9_]+|_[A-Z0-9_]+)?$/ && #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) { @@ -5028,6 +5811,7 @@ if (defined $define_args && $define_args ne "") { $define_args = substr($define_args, 1, length($define_args) - 2); $define_args =~ s/\s*//g; + $define_args =~ s/\+?//g; @def_args = split(",", $define_args); }
@@ -5037,13 +5821,13 @@ $dstat =~ s/\s*$//s;
# Flatten any parentheses and braces - while ($dstat =~ s/([^()]*)/1/ || - $dstat =~ s/{[^{}]*}/1/ || - $dstat =~ s/.[[^[]]*]/1/) + while ($dstat =~ s/([^()]*)/1u/ || + $dstat =~ s/{[^{}]*}/1u/ || + $dstat =~ s/.[[^[]]*]/1u/) { }
- # Flatten any obvious string concatentation. + # Flatten any obvious string concatenation. while ($dstat =~ s/($String)\s*$Ident/$1/ || $dstat =~ s/$Ident\s*($String)/$1/) { @@ -5080,6 +5864,7 @@ $dstat !~ /^.$Ident\s*=/ && # .foo = $dstat !~ /^(?:#\s*$Ident|#\s*$Constant)\s*$/ && # stringification #foo $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...) + $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ && # while (...) {...} $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... @@ -5121,7 +5906,7 @@ next if ($arg =~ /.../); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*(\s*$Type\s*,|#+)\s*(*\s*$arg\s*)*\b//g; + $tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|typeof|__typeof__|__builtin\w+|typecheck\s*(\s*$Type\s*,|#+)\s*(*\s*$arg\s*)*\b//g; $tmp_stmt =~ s/#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*##//g; my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g; @@ -5163,7 +5948,7 @@ # do {} while (0) macro tests: # single-statement macros do not need to be enclosed in do while (0) loop, # macro should not end with a semicolon - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $realfile !~ m@/vmlinux.lds.h$@ && $line =~ /^.\s*#\s*define\s+$Ident(()?/) { my $ln = $linenr; @@ -5398,6 +6183,17 @@ "Prefer using '"%s...", __func__' to using '$context_function', this function's name, in a string\n" . $herecurr); }
+# check for unnecessary function tracing like uses +# This does not use $logFunctions because there are many instances like +# 'dprintk(FOO, "%s()\n", __func__);' which do not match $logFunctions + if ($rawline =~ /^+.*([^"]*"$tracing_logging_tags{0,3}%s(?:\s*(\s*)\s*)?$tracing_logging_tags{0,3}(?:\n)?"\s*,\s*__func__\s*)\s*;/) { + if (WARN("TRACING_LOGGING", + "Unnecessary ftrace-like logging - prefer using ftrace\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + } + # check for spaces before a quoted newline if ($rawline =~ /^.*".*\s\n/) { if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", @@ -5409,15 +6205,29 @@ }
# 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); + if ($line =~ /$String[A-Z_]/ || + ($line =~ /([A-Za-z0-9_]+)$String/ && $1 !~ /^[Lu]$/)) { + if (CHK("CONCATENATED_STRING", + "Concatenated strings should use spaces between elements\n" . $herecurr) && + $fix) { + while ($line =~ /($String)/g) { + my $extracted_string = substr($rawline, $-[0], $+[0] - $-[0]); + $fixed[$fixlinenr] =~ s/\Q$extracted_string\E([A-Za-z0-9_])/$extracted_string $1/; + $fixed[$fixlinenr] =~ s/([A-Za-z0-9_])\Q$extracted_string\E/$1 $extracted_string/; + } + } }
# uncoalesced string fragments - if ($line =~ /$String\s*"/) { - WARN("STRING_FRAGMENTS", - "Consecutive strings are generally better as a single string\n" . $herecurr); + if ($line =~ /$String\s*[Lu]?"/) { + if (WARN("STRING_FRAGMENTS", + "Consecutive strings are generally better as a single string\n" . $herecurr) && + $fix) { + while ($line =~ /($String)(?=\s*")/g) { + my $extracted_string = substr($rawline, $-[0], $+[0] - $-[0]); + $fixed[$fixlinenr] =~ s/\Q$extracted_string\E\s*"/substr($extracted_string, 0, -1)/e; + } + } }
# check for non-standard and hex prefixed decimal printf formats @@ -5453,9 +6263,14 @@
# warn about #if 0 if ($line =~ /^.\s*#\s*if\s+0\b/) { - CHK("REDUNDANT_CODE", - "if this code is redundant consider removing it\n" . - $herecurr); + WARN("IF_0", + "Consider removing the code enclosed by this #if 0 and its #endif\n" . $herecurr); + } + +# warn about #if 1 + if ($line =~ /^.\s*#\s*if\s+1\b/) { + WARN("IF_1", + "Consider removing the #if 1 and its #endif\n" . $herecurr); }
# check for needless "if (<foo>) fn(<foo>)" uses @@ -5502,7 +6317,8 @@ my ($s, $c) = ctx_statement_block($linenr - 3, $realcnt, 0); # print("line: <$line>\nprevline: <$prevline>\ns: <$s>\nc: <$c>\n\n\n");
- if ($s =~ /(?:^|\n)[ +]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:([^)]*)\s*)?\s*(?:devm_)?(?:[kv][czm]alloc(?:_node|_array)?\b|kstrdup|kmemdup|(?:dev_)?alloc_skb)/) { + if ($s =~ /(?:^|\n)[ +]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:([^)]*)\s*)?\s*$allocFunctions\s*(/ && + $s !~ /\b__GFP_NOWARN\b/ ) { WARN("OOM_MESSAGE", "Possible unnecessary 'out of memory' message\n" . $hereprev); } @@ -5525,8 +6341,30 @@ "Avoid logging continuation uses where feasible\n" . $herecurr); }
+# check for unnecessary use of %h[xudi] and %hh[xudi] in logging functions + if (defined $stat && + $line =~ /\b$logFunctions\s*(/ && + index($stat, '"') >= 0) { + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = get_stat_real($linenr, $lc); + pos($stat_real) = index($stat_real, '"'); + while ($stat_real =~ /[^"%]*(%[#\d.*-]*(h+)[idux])/g) { + my $pspec = $1; + my $h = $2; + my $lineoff = substr($stat_real, 0, $-[1]) =~ tr@\n@@; + if (WARN("UNNECESSARY_MODIFIER", + "Integer promotion: Using '$h' in '$pspec' is unnecessary\n" . "$here\n$stat_real\n") && + $fix && $fixed[$fixlinenr + $lineoff] =~ /^+/) { + my $nspec = $pspec; + $nspec =~ s/h//g; + $fixed[$fixlinenr + $lineoff] =~ s/\Q$pspec\E/$nspec/; + } + } + } + # check for mask then right shift without a parentheses - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /$LvalOrFunc\s*&\s*($LvalOrFunc)\s*>>/ && $4 !~ /^&/) { # $LvalOrFunc may be &foo, ignore if so WARN("MASK_THEN_SHIFT", @@ -5534,7 +6372,7 @@ }
# check for pointer comparisons to NULL - if ($^V && $^V ge 5.10.0) { + if ($perl_version_ok) { while ($line =~ /\b$LvalOrFunc\s*(==|!=)\s*NULL\b/g) { my $val = $1; my $equal = "!"; @@ -5623,7 +6461,7 @@ # ignore udelay's < 10, however if (! ($delay < 10) ) { CHK("USLEEP_RANGE", - "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr); + "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst\n" . $herecurr); } if ($delay > 2000) { WARN("LONG_UDELAY", @@ -5635,7 +6473,7 @@ if ($line =~ /\bmsleep\s*((\d+));/) { if ($1 < 20) { WARN("MSLEEP", - "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr); + "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst\n" . $herecurr); } }
@@ -5683,8 +6521,7 @@ my $barriers = qr{ mb| rmb| - wmb| - read_barrier_depends + wmb }x; my $barrier_stems = qr{ mb__before_atomic| @@ -5725,10 +6562,12 @@ } }
-# check for smp_read_barrier_depends and read_barrier_depends - if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*(/) { - WARN("READ_BARRIER_DEPENDS", - "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr); +# check for data_race without a comment. + if ($line =~ /\bdata_race\s*(/) { + if (!ctx_has_comment($first_line, $linenr)) { + WARN("DATA_RACE", + "data_race without comment\n" . $herecurr); + } }
# check of hardware specific defines @@ -5770,43 +6609,73 @@ } }
-# Check for __attribute__ packed, prefer __packed +# Check for compiler attributes if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*(\s*(.*\bpacked\b/) { - WARN("PREFER_PACKED", - "__packed is preferred over __attribute__((packed))\n" . $herecurr); - } + $rawline =~ /\b__attribute__\s*(\s*($balanced_parens)\s*)/) { + my $attr = $1; + $attr =~ s/\s*(\s*(.*))\s*/$1/;
-# Check for __attribute__ aligned, prefer __aligned - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*(\s*(.*aligned/) { - WARN("PREFER_ALIGNED", - "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); - } + my %attr_list = ( + "alias" => "__alias", + "aligned" => "__aligned", + "always_inline" => "__always_inline", + "assume_aligned" => "__assume_aligned", + "cold" => "__cold", + "const" => "__attribute_const__", + "copy" => "__copy", + "designated_init" => "__designated_init", + "externally_visible" => "__visible", + "format" => "printf|scanf", + "gnu_inline" => "__gnu_inline", + "malloc" => "__malloc", + "mode" => "__mode", + "no_caller_saved_registers" => "__no_caller_saved_registers", + "noclone" => "__noclone", + "noinline" => "noinline", + "nonstring" => "__nonstring", + "noreturn" => "__noreturn", + "packed" => "__packed", + "pure" => "__pure", + "section" => "__section", + "used" => "__used", + "weak" => "__weak" + );
-# Check for __attribute__ format(printf, prefer __printf - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*(\s*(\s*format\s*(\s*printf/) { - if (WARN("PREFER_PRINTF", - "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*(\s*(\s*format\s*(\s*printf\s*,\s*(.*))\s*)\s*)/"__printf(" . trim($1) . ")"/ex; - + while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { + my $orig_attr = $1; + my $params = ''; + $params = $2 if defined($2); + my $curr_attr = $orig_attr; + $curr_attr =~ s/^[\s_]+|[\s_]+$//g; + if (exists($attr_list{$curr_attr})) { + my $new = $attr_list{$curr_attr}; + if ($curr_attr eq "format" && $params) { + $params =~ /^\s*(\s*(\w+)\s*,\s*(.*)/; + $new = "__$1($2"; + } else { + $new = "$new$params"; + } + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "Prefer $new over __attribute__(($orig_attr$params))\n" . $herecurr) && + $fix) { + my $remove = "\Q$orig_attr\E" . '\s*' . "\Q$params\E" . '(?:\s*,\s*)?'; + $fixed[$fixlinenr] =~ s/$remove//; + $fixed[$fixlinenr] =~ s/\b__attribute__/$new __attribute__/; + $fixed[$fixlinenr] =~ s/}\Q$new\E/} $new/; + $fixed[$fixlinenr] =~ s/ __attribute__\s*(\s*(\s*)\s*)//; + } + } } - }
-# Check for __attribute__ format(scanf, prefer __scanf - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*(\s*(\s*format\s*(\s*scanf\b/) { - if (WARN("PREFER_SCANF", - "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*(\s*(\s*format\s*(\s*scanf\s*,\s*(.*))\s*)\s*)/"__scanf(" . trim($1) . ")"/ex; + # Check for __attribute__ unused, prefer __always_unused or __maybe_unused + if ($attr =~ /^_*unused/) { + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr); } }
# Check for __attribute__ weak, or __weak declarations (may have link issues) - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /(?:$Declare|$DeclareMisordered)\s*$Ident\s*$balanced_parens\s*(?:$Attribute)?\s*;/ && ($line =~ /\b__attribute__\s*(\s*(.*\bweak\b/ || $line =~ /\b__weak\b/)) { @@ -5837,18 +6706,18 @@ if ($line =~ /((\s*$C90_int_types\s*)\s*)($Constant)\b/) { my $cast = $1; my $const = $2; + 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'; + } if (WARN("TYPECAST_INT_CONSTANT", - "Unnecessary typecast of c90 int constant\n" . $herecurr) && + "Unnecessary typecast of c90 int constant - '$cast$const' could be '$const$suffix'\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/; } } @@ -5888,7 +6757,7 @@ }
# check for vsprintf extension %p<foo> misuses - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^+(?![^{]*{\s*).*\b(\w+)\s*(.*$String\s*,/s && $1 !~ /^_*volatile_*$/) { @@ -5899,14 +6768,20 @@ for (my $count = $linenr; $count <= $lc; $count++) { my $specifier; my $extension; + my $qualifier; my $bad_specifier = ""; my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g;
- while ($fmt =~ /(%[*\d.]*p(\w))/g) { + while ($fmt =~ /(%[*\d.]*p(\w)(\w*))/g) { $specifier = $1; $extension = $2; - if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOx]/) { + $qualifier = $3; + if ($extension !~ /[4SsBKRraEehMmIiUDdgVCbGNOxtf]/ || + ($extension eq "f" && + defined $qualifier && $qualifier !~ /^w/) || + ($extension eq "4" && + defined $qualifier && $qualifier !~ /^cc/)) { $bad_specifier = $specifier; last; } @@ -5923,7 +6798,6 @@ my $ext_type = "Invalid"; my $use = ""; if ($bad_specifier =~ /p[Ff]/) { - $ext_type = "Deprecated"; $use = " - use %pS instead"; $use =~ s/pS/ps/ if ($bad_specifier =~ /pf/); } @@ -5935,7 +6809,7 @@ }
# Check for misused memsets - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^+(?:.*?)\bmemset\s*(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*$FuncArg\s*)/) {
@@ -5953,7 +6827,7 @@ }
# Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) -# if ($^V && $^V ge 5.10.0 && +# if ($perl_version_ok && # defined $stat && # $stat =~ /^+(?:.*?)\bmemcpy\s*(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*)/) { # if (WARN("PREFER_ETHER_ADDR_COPY", @@ -5964,7 +6838,7 @@ # }
# Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) -# if ($^V && $^V ge 5.10.0 && +# if ($perl_version_ok && # defined $stat && # $stat =~ /^+(?:.*?)\bmemcmp\s*(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*)/) { # WARN("PREFER_ETHER_ADDR_EQUAL", @@ -5973,7 +6847,7 @@
# 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 && +# if ($perl_version_ok && # defined $stat && # $stat =~ /^+(?:.*?)\bmemset\s*(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*)/) { # @@ -5994,8 +6868,14 @@ # } # }
+# strlcpy uses that should likely be strscpy + if ($line =~ /\bstrlcpy\s*(/) { + WARN("STRLCPY", + "Prefer strscpy over strlcpy - see: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmkn..." . $herecurr); + } + # typecasts on min/max could be min_t/max_t - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^+(?:.*?)\b(min|max)\s*(\s*$FuncArg\s*,\s*$FuncArg\s*)/) { if (defined $2 || defined $7) { @@ -6019,23 +6899,23 @@ }
# check usleep_range arguments - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^+(?:.*?)\busleep_range\s*(\s*($FuncArg)\s*,\s*($FuncArg)\s*)/) { my $min = $1; my $max = $7; if ($min eq $max) { WARN("USLEEP_RANGE", - "usleep_range should not use min == max args; see Documentation/timers/timers-howto.txt\n" . "$here\n$stat\n"); + "usleep_range should not use min == max args; see Documentation/timers/timers-howto.rst\n" . "$here\n$stat\n"); } elsif ($min =~ /^\d+$/ && $max =~ /^\d+$/ && $min > $max) { WARN("USLEEP_RANGE", - "usleep_range args reversed, use min then max; see Documentation/timers/timers-howto.txt\n" . "$here\n$stat\n"); + "usleep_range args reversed, use min then max; see Documentation/timers/timers-howto.rst\n" . "$here\n$stat\n"); } }
# check for naked sscanf - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $line =~ /\bsscanf\b/ && ($stat !~ /$Ident\s*=\s*sscanf\s*$balanced_parens/ && @@ -6049,7 +6929,7 @@ }
# check for simple sscanf that should be kstrto<foo> - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $line =~ /\bsscanf\b/) { my $lc = $stat =~ tr@\n@@; @@ -6087,8 +6967,7 @@ if (defined $cond) { substr($s, 0, length($cond), ''); } - if ($s =~ /^\s*;/ && - $function_name ne 'uninitialized_var') + if ($s =~ /^\s*;/) { WARN("AVOID_EXTERNS", "externs should be avoided in .c files\n" . $herecurr); @@ -6121,7 +7000,7 @@ }
# check for function definitions - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) { $context_function = $1; @@ -6149,26 +7028,26 @@
if (!grep(/$name/, @setup_docs)) { CHK("UNDOCUMENTED_SETUP", - "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.rst\n" . $herecurr); + "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.txt\n" . $herecurr); } }
-# check for pointless casting of kmalloc return - if ($line =~ /*\s*)\s*[kv][czm]alloc(_node){0,1}\b/) { +# check for pointless casting of alloc functions + if ($line =~ /*\s*)\s*$allocFunctions\b/) { WARN("UNNECESSARY_CASTS", "unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html%5Cn" . $herecurr); }
# alloc style # p = alloc(sizeof(struct foo), ...) should be p = alloc(sizeof(*p), ...) - if ($^V && $^V ge 5.10.0 && - $line =~ /\b($Lval)\s*=\s*(?:$balanced_parens)?\s*([kv][mz]alloc(?:_node)?)\s*(\s*(sizeof\s*(\s*struct\s+$Lval\s*))/) { + if ($perl_version_ok && + $line =~ /\b($Lval)\s*=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc(?:_node)?)\s*(\s*(sizeof\s*(\s*struct\s+$Lval\s*))/) { CHK("ALLOC_SIZEOF_STRUCT", "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); }
# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^+\s*($Lval)\s*=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*(\s*($FuncArg)\s**\s*($FuncArg)\s*,/) { my $oldfunc = $3; @@ -6197,14 +7076,15 @@ }
# check for krealloc arg reuse - if ($^V && $^V ge 5.10.0 && - $line =~ /\b($Lval)\s*=\s*(?:$balanced_parens)?\s*krealloc\s*(\s*\1\s*,/) { + if ($perl_version_ok && + $line =~ /\b($Lval)\s*=\s*(?:$balanced_parens)?\s*krealloc\s*(\s*($Lval)\s*,/ && + $1 eq $3) { WARN("KREALLOC_ARG_REUSE", "Reusing the krealloc arg is almost always a bug\n" . $herecurr); }
# check for alloc argument mismatch - if ($line =~ /\b(kcalloc|kmalloc_array)\s*(\s*sizeof\b/) { + if ($line =~ /\b((?:devm_)?(?:kcalloc|kmalloc_array))\s*(\s*sizeof\b/) { WARN("ALLOC_ARRAY_ARGS", "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } @@ -6230,33 +7110,46 @@ } }
-# check for case / default statements not preceded by break/fallthrough/switch - if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) { - my $has_break = 0; - my $has_statement = 0; - my $count = 0; - my $prevline = $linenr; - while ($prevline > 1 && ($file || $count < 3) && !$has_break) { - $prevline--; - my $rline = $rawlines[$prevline - 1]; - my $fline = $lines[$prevline - 1]; - last if ($fline =~ /^@@/); - next if ($fline =~ /^-/); - next if ($fline =~ /^.(?:\s*(?:case\s+(?:$Ident|$Constant)[\s$;]*|default):[\s$;]*)*$/); - $has_break = 1 if ($rline =~ /fall[\s_-]*(through|thru)/i); - next if ($fline =~ /^.[\s$;]*$/); - $has_statement = 1; - $count++; - $has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|exit\s*(\b|return\b|goto\b|continue\b)/); +# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too) + if ($rawline =~ /\bIS_ENABLED\s*(\s*(\w+)\s*)/ && $1 !~ /^${CONFIG_}/) { + WARN("IS_ENABLED_CONFIG", + "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr); + } + +# 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)"; } - if (!$has_break && $has_statement) { - WARN("MISSING_BREAK", - "Possible switch case/default not preceded by break or fallthrough comment\n" . $herecurr); + } + +# check for /* fallthrough */ like comment, prefer fallthrough; + my @fallthroughs = ( + 'fallthrough', + '@fallthrough@', + 'lint -fallthrough[ \t]*', + 'intentional(?:ly)?[ \t]*fall(?:(?:s | |-)[Tt]|t)hr(?:ough|u|ew)', + '(?:else,?\s*)?FALL(?:S | |-)?THR(?:OUGH|U|EW)[ \t.!]*(?:-[^\n\r]*)?', + 'Fall(?:(?:s | |-)[Tt]|t)hr(?:ough|u|ew)[ \t.!]*(?:-[^\n\r]*)?', + 'fall(?:s | |-)?thr(?:ough|u|ew)[ \t.!]*(?:-[^\n\r]*)?', + ); + if ($raw_comment ne '') { + foreach my $ft (@fallthroughs) { + if ($raw_comment =~ /$ft/) { + my $msg_level = &WARN; + $msg_level = &CHK if ($file); + &{$msg_level}("PREFER_FALLTHROUGH", + "Prefer 'fallthrough;' over fallthrough comment\n" . $herecurr); + last; + } } }
# check for switch/default statements without a break; - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /^+[$;\s]*(?:case[$;\s]+\w+[$;\s]*:[$;\s]*|)*[$;\s]*\bdefault[$;\s]*:[$;\s]*;/g) { my $cnt = statement_rawlines($stat); @@ -6314,12 +7207,6 @@ } }
-# check for bool bitfields - if ($sline =~ /^.\s+bool\s*$Ident\s*:\s*\d+\s*;/) { - WARN("BOOL_BITFIELD", - "Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>\n" . $herecurr); - } - # check for semaphores initialized locked if ($line =~ /^.\s*sema_init.+,\W?0\W?)/) { WARN("CONSIDER_COMPLETION", @@ -6338,9 +7225,24 @@ "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); }
+# check for spin_is_locked(), suggest lockdep instead + if ($line =~ /\bspin_is_locked(/) { + WARN("USE_LOCKDEP", + "Where possible, use lockdep_assert_held instead of assertions based on spin_is_locked\n" . $herecurr); + } + +# check for deprecated apis + if ($line =~ /\b($deprecated_apis_search)\b\s*(/) { + my $deprecated_api = $1; + my $new_api = $deprecated_apis{$deprecated_api}; + WARN("DEPRECATED_API", + "Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr); + } + # check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' - if ($line !~ /\bconst\b/ && + if (defined($const_structs) && + $line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b(?!\s*{)/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); @@ -6348,12 +7250,14 @@
# use of NR_CPUS is usually wrong # ignore definitions of NR_CPUS and usage to define arrays as likely right +# ignore designated initializers using NR_CPUS if ($line =~ /\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*if\b.*\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*define\b.*\bNR_CPUS\b/ && $line !~ /^.\s*$Declare\s.*[[^]]*NR_CPUS[^]]*]/ && $line !~ /[[^]]*...[^]]*NR_CPUS[^]]*]/ && - $line !~ /[[^]]*NR_CPUS[^]]*...[^]]*]/) + $line !~ /[[^]]*NR_CPUS[^]]*...[^]]*]/ && + $line !~ /^.\s*.\w+\s*=\s*.*\bNR_CPUS\b/) { WARN("NR_CPUS", "usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); @@ -6366,12 +7270,29 @@ }
# likely/unlikely comparisons similar to "(likely(foo) > 0)" - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && $line =~ /\b((?:un)?likely)\s*(\s*$FuncArg\s*)\s*$Compare/) { WARN("LIKELY_MISUSE", "Using $1 should generally have parentheses around the comparison\n" . $herecurr); }
+# return sysfs_emit(foo, fmt, ...) fmt without newline + if ($line =~ /\breturn\s+sysfs_emit\s*(\s*$FuncArg\s*,\s*($String)/ && + substr($rawline, $-[6], $+[6] - $-[6]) !~ /\n"$/) { + my $offset = $+[6] - 1; + if (WARN("SYSFS_EMIT", + "return sysfs_emit(...) formats should include a terminating newline\n" . $herecurr) && + $fix) { + substr($fixed[$fixlinenr], $offset, 0) = '\n'; + } + } + +# nested likely/unlikely calls + if ($line =~ /\b(?:(?:un)?likely)\s*(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { + WARN("LIKELY_MISUSE", + "nested (un)?likely() calls, $1 already uses unlikely() internally\n" . $herecurr); + } + # whine mightly about in_atomic if ($line =~ /\bin_atomic\s*(/) { if ($realfile =~ m@^drivers/@) { @@ -6383,12 +7304,6 @@ } }
-# check for mutex_trylock_recursive usage - if ($line =~ /mutex_trylock_recursive/) { - ERROR("LOCKING", - "recursive locking is bad, do not use this ever.\n" . $herecurr); - } - # check for lockdep_set_novalidate_class if ($line =~ /^.\s*lockdep_set_novalidate_class\s*(/ || $line =~ /__lockdep_no_validate__\s*)/ ) { @@ -6409,7 +7324,7 @@ # check for DEVICE_ATTR uses that could be DEVICE_ATTR_<FOO> # and whether or not function naming is typical and if # DEVICE_ATTR permissions uses are unusual too - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $stat =~ /\bDEVICE_ATTR\s*(\s*(\w+)\s*,\s*(?\s*(\s*(?:${multi_mode_perms_string_search}|0[0-7]{3,3})\s*)\s*)?\s*,\s*(\w+)\s*,\s*(\w+)\s*)/) { my $var = $1; @@ -6469,7 +7384,7 @@ # specific definition of not visible in sysfs. # o Ignore proc_create*(...) uses with a decimal 0 permission as that means # use the default permissions - if ($^V && $^V ge 5.10.0 && + if ($perl_version_ok && defined $stat && $line =~ /$mode_perms_search/) { foreach my $entry (@mode_permission_funcs) { @@ -6531,6 +7446,12 @@ "unknown module license " . $extracted_string . "\n" . $herecurr); } } + +# check for sysctl duplicate constants + if ($line =~ /.extra[12]\s*=\s*&(zero|one|int_max)\b/) { + WARN("DUPLICATED_SYSCTL_CONST", + "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr); + } }
# If we have no input at all, then there is nothing to report on @@ -6545,7 +7466,7 @@ exit(0); }
- # This is not a patch, and we are are in 'no-patch' mode so + # This is not a patch, and we are in 'no-patch' mode so # just keep quiet. if (!$chk_patch && !$is_patch) { exit(0); @@ -6555,9 +7476,38 @@ ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } - if ($is_patch && $has_commit_log && $chk_signoff && $signoff == 0) { - ERROR("MISSING_SIGN_OFF", - "Missing Signed-off-by: line(s)\n"); + if ($is_patch && $has_commit_log && $chk_signoff) { + if ($signoff == 0) { + ERROR("MISSING_SIGN_OFF", + "Missing Signed-off-by: line(s)\n"); + } elsif ($authorsignoff != 1) { + # authorsignoff values: + # 0 -> missing sign off + # 1 -> sign off identical + # 2 -> names and addresses match, comments mismatch + # 3 -> addresses match, names different + # 4 -> names match, addresses different + # 5 -> names match, addresses excluding subaddress details (refer RFC 5233) match + + my $sob_msg = "'From: $author' != 'Signed-off-by: $author_sob'"; + + if ($authorsignoff == 0) { + ERROR("NO_AUTHOR_SIGN_OFF", + "Missing Signed-off-by: line by nominal patch author '$author'\n"); + } elsif ($authorsignoff == 2) { + CHK("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email comments mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 3) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email name mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 4) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email address mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 5) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email subaddress mismatch: $sob_msg\n"); + } + } }
print report_dump();