diff --git a/android/GKI_VERSION b/android/GKI_VERSION index e351f5baec42..5881df6e8950 100644 --- a/android/GKI_VERSION +++ b/android/GKI_VERSION @@ -1 +1 @@ -LTS_5.4.259_81334f26ac70 +LTS_5.4.259_70db018a109 diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c index df7a911d303f..e8c7135c4c11 100644 --- a/drivers/nvme/target/tcp.c +++ b/drivers/nvme/target/tcp.c @@ -321,6 +321,15 @@ static void nvmet_tcp_fatal_error(struct nvmet_tcp_queue *queue) kernel_sock_shutdown(queue->sock, SHUT_RDWR); } +static void nvmet_tcp_socket_error(struct nvmet_tcp_queue *queue, int status) +{ + queue->rcv_state = NVMET_TCP_RECV_ERR; + if (status == -EPIPE || status == -ECONNRESET) + kernel_sock_shutdown(queue->sock, SHUT_RDWR); + else + nvmet_tcp_fatal_error(queue); +} + static int nvmet_tcp_map_data(struct nvmet_tcp_cmd *cmd) { struct nvme_sgl_desc *sgl = &cmd->req.cmd->common.dptr.sgl; @@ -714,11 +723,15 @@ static int nvmet_tcp_try_send(struct nvmet_tcp_queue *queue, for (i = 0; i < budget; i++) { ret = nvmet_tcp_try_send_one(queue, i == budget - 1); - if (ret <= 0) + if (unlikely(ret < 0)) { + nvmet_tcp_socket_error(queue, ret); + goto done; + } else if (ret == 0) { break; + } (*sends)++; } - +done: return ret; } @@ -816,15 +829,11 @@ static int nvmet_tcp_handle_icreq(struct nvmet_tcp_queue *queue) iov.iov_len = sizeof(*icresp); ret = kernel_sendmsg(queue->sock, &msg, &iov, 1, iov.iov_len); if (ret < 0) - goto free_crypto; + return ret; /* queue removal will cleanup */ queue->state = NVMET_TCP_Q_LIVE; nvmet_prepare_receive_pdu(queue); return 0; -free_crypto: - if (queue->hdr_digest || queue->data_digest) - nvmet_tcp_free_crypto(queue); - return ret; } static void nvmet_tcp_handle_req_failure(struct nvmet_tcp_queue *queue, @@ -1167,11 +1176,15 @@ static int nvmet_tcp_try_recv(struct nvmet_tcp_queue *queue, for (i = 0; i < budget; i++) { ret = nvmet_tcp_try_recv_one(queue); - if (ret <= 0) + if (unlikely(ret < 0)) { + nvmet_tcp_socket_error(queue, ret); + goto done; + } else if (ret == 0) { break; + } (*recvs)++; } - +done: return ret; } @@ -1196,27 +1209,16 @@ static void nvmet_tcp_io_work(struct work_struct *w) pending = false; ret = nvmet_tcp_try_recv(queue, NVMET_TCP_RECV_BUDGET, &ops); - if (ret > 0) { + if (ret > 0) pending = true; - } else if (ret < 0) { - if (ret == -EPIPE || ret == -ECONNRESET) - kernel_sock_shutdown(queue->sock, SHUT_RDWR); - else - nvmet_tcp_fatal_error(queue); + else if (ret < 0) return; - } ret = nvmet_tcp_try_send(queue, NVMET_TCP_SEND_BUDGET, &ops); - if (ret > 0) { - /* transmitted message/data */ + if (ret > 0) pending = true; - } else if (ret < 0) { - if (ret == -EPIPE || ret == -ECONNRESET) - kernel_sock_shutdown(queue->sock, SHUT_RDWR); - else - nvmet_tcp_fatal_error(queue); + else if (ret < 0) return; - } } while (pending && ops < NVMET_TCP_IO_WORK_BUDGET); diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c index 88211a4b3855..c0f922c32a53 100644 --- a/net/ipv4/igmp.c +++ b/net/ipv4/igmp.c @@ -218,8 +218,10 @@ static void igmp_start_timer(struct ip_mc_list *im, int max_delay) int tv = prandom_u32() % max_delay; im->tm_running = 1; - if (!mod_timer(&im->timer, jiffies+tv+2)) - refcount_inc(&im->refcnt); + if (refcount_inc_not_zero(&im->refcnt)) { + if (mod_timer(&im->timer, jiffies + tv + 2)) + ip_ma_put(im); + } } static void igmp_gq_start_timer(struct in_device *in_dev) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 538be9fa39fe..b38660c0c85e 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -30,6 +30,9 @@ my $V = '0.32'; 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; @@ -50,6 +53,8 @@ my $list_types = 0; my $fix = 0; my $fix_inplace = 0; my $root; +my $gitroot = $ENV{'GIT_DIR'}; +$gitroot = ".git" if !defined($gitroot); my %debug; my %camelcase = (); my %use_type = (); @@ -65,13 +70,18 @@ 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 $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst"; my $typedefsfile; my $color = "auto"; 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_"; + +my %maybe_linker_symbol; # for externs in c exceptions, when seen in *vmlinux.lds.h sub help { my ($exitcode) = @_; @@ -82,6 +92,7 @@ Version: $V 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) @@ -129,11 +140,13 @@ Options: --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. @@ -160,15 +173,51 @@ sub list_types { 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); @@ -200,6 +249,46 @@ if (-f $conf) { 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) { @@ -210,6 +299,7 @@ foreach (@ARGV) { GetOptions( 'q|quiet+' => \$quiet, + 'v|verbose!' => \$verbose, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!' => \$chk_patch, @@ -237,24 +327,62 @@ GetOptions( '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: /data/dictionary.txt + if (($codespell || $help) && which("python3") ne "") { + my $python_codespell_dict = << "EOF"; +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 + + my $codespell_dict = `python3 -c "$python_codespell_dict" 2> /dev/null`; + $codespellfile = $codespell_dict if (-f $codespell_dict); + } +} + +# $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; +} elsif ($color =~ /^always$/i) { + $color = 1; +} elsif ($color =~ /^never$/i) { + $color = 0; +} elsif ($color =~ /^auto$/i) { + $color = (-t STDOUT); +} else { + 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; -die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); - my $exit = 0; my $perl_version_ok = 1; @@ -269,18 +397,6 @@ if ($#ARGV < 0) { push(@ARGV, '-'); } -if ($color =~ /^[01]$/) { - $color = !$color; -} elsif ($color =~ /^always$/i) { - $color = 1; -} elsif ($color =~ /^never$/i) { - $color = 0; -} elsif ($color =~ /^auto$/i) { - $color = (-t STDOUT); -} else { - die "$P: Invalid color mode: $color\n"; -} - # skip TAB size 1 to avoid additional checks on $tabsize - 1 die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2); @@ -383,6 +499,7 @@ our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeIni # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + volatile| __percpu| __nocast| __safe| @@ -405,7 +522,8 @@ our $Attribute = qr{ ____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__}; @@ -417,7 +535,7 @@ our $Binary = qr{(?i)0b[01]+$Int_type?}; our $Hex = qr{(?i)0x[0-9a-f]+$Int_type?}; our $Int = qr{[0-9]+$Int_type?}; our $Octal = qr{0[0-7]+$Int_type?}; -our $String = qr{"[X\t]*"}; +our $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]?}; @@ -467,10 +585,14 @@ our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeStdioTypedefs = qr{(?x: + FILE +)}; our $typeTypedefs = qr{(?x: $typeC99Typedefs\b| $typeOtherOSTypedefs\b| - $typeKernelTypedefs\b + $typeKernelTypedefs\b| + $typeStdioTypedefs\b )}; our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b}; @@ -487,7 +609,7 @@ our $logFunctions = qr{(?x: our $allocFunctions = qr{(?x: (?:(?:devm_)? - (?:kv|k|v)[czm]alloc(?:_node|_array)? | + (?:kv|k|v)[czm]alloc(?:_array)?(?:_node)? | kstrdup(?:_const)? | kmemdup(?:_nul)?) | (?:\w+)?alloc_skb(?:_ip_align)? | @@ -507,6 +629,115 @@ our $signature_tags = qr{(?xi: Cc: )}; +our @link_tags = qw(Link Closes); + +#Create a search and print patterns for all these strings to be used directly below +our $link_tags_search = ""; +our $link_tags_print = ""; +foreach my $entry (@link_tags) { + if ($link_tags_search ne "") { + $link_tags_search .= '|'; + $link_tags_print .= ' or '; + } + $entry .= ':'; + $link_tags_search .= $entry; + $link_tags_print .= "'$entry'"; +} +$link_tags_search = "(?:${link_tags_search})"; + +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 $obsolete_archives = qr{(?xi: + \Qfreedesktop.org/archives/dri-devel\E | + \Qlists.infradead.org\E | + \Qlkml.org\E | + \Qmail-archive.com\E | + \Qmailman.alsa-project.org/pipermail\E | + \Qmarc.info\E | + \Qozlabs.org/pipermail\E | + \Qspinics.net\E +)}; + our @typeListMisordered = ( qr{char\s+(?:un)?signed}, qr{int\s+(?:(?:un)?signed\s+)?short\s}, @@ -616,6 +847,10 @@ our %deprecated_apis = ( "rcu_barrier_sched" => "rcu_barrier", "get_state_synchronize_sched" => "get_state_synchronize_rcu", "cond_synchronize_sched" => "cond_synchronize_rcu", + "kmap" => "kmap_local_page", + "kunmap" => "kunmap_local", + "kmap_atomic" => "kmap_local_page", + "kunmap_atomic" => "kunmap_local", ); #Create a search pattern for all these strings to speed up a loop below @@ -851,9 +1086,17 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)}; our $declaration_macros = qr{(?x: (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(| (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(| - (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\( + (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\(| + (?:$Storage\s+)?(?:XA_STATE|XA_STATE_ORDER)\s*\( )}; +our %allow_repeated_words = ( + add => '', + added => '', + bad => '', + be => '', +); + sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); @@ -911,10 +1154,10 @@ sub is_maintained_obsolete { sub is_SPDX_License_valid { my ($license) = @_; - return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$root/.git")); + 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" | python scripts/spdxcheck.py -`; + my $status = `cd "$root_path"; echo "$license" | scripts/spdxcheck.py -`; return 0 if ($status ne ""); return 1; } @@ -929,7 +1172,7 @@ sub seed_camelcase_includes { $camelcase_seeded = 1; - if (-e ".git") { + 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"; @@ -957,7 +1200,7 @@ sub seed_camelcase_includes { return; } - if (-e ".git") { + if (-e "$gitroot") { $files = `${git_command} ls-files "include/*.h"`; @include_files = split('\n', $files); } @@ -977,10 +1220,20 @@ sub seed_camelcase_includes { } } +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_command} log --no-color --format='%H %s' -1 $commit 2>&1`; $output =~ s/^\s*//gm; @@ -998,7 +1251,8 @@ sub git_commit_info { # 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); @@ -1019,7 +1273,7 @@ my $fixlinenr = -1; # If input is git commits, extract all commits from the commit expressions. # For example, HEAD-3 means we need check 'HEAD, HEAD~1, HEAD~2'. -die "$P: No git repository found\n" if ($git && !-e ".git"); +die "$P: No git repository found\n" if ($git && !-e "$gitroot"); if ($git) { my @commits = (); @@ -1050,6 +1304,9 @@ my $vname; $allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; + 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"; @@ -1094,6 +1351,7 @@ for my $filename (@ARGV) { @modifierListFile = (); @typeListFile = (); build_types(); + $file = $oldfile if ($is_git_file); } if (!$quiet) { @@ -1139,6 +1397,7 @@ sub parse_email { my ($formatted_email) = @_; my $name = ""; + my $quoted = ""; my $name_comment = ""; my $address = ""; my $comment = ""; @@ -1170,14 +1429,20 @@ sub parse_email { } } - $name = trim($name); - $name =~ s/^\"|\"$//g; - $name =~ s/(\s*\([^\)]+\))\s*//; - if (defined($1)) { - $name_comment = trim($1); + # 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/(?"; + $formatted_email = "$name$name_comment <$address>"; } - + $formatted_email .= "$comment"; return $formatted_email; } @@ -1214,7 +1484,7 @@ sub reformat_email { my ($email) = @_; my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); - return format_email($email_name, $email_address); + return format_email($email_name, $name_comment, $email_address, $comment); } sub same_email_addresses { @@ -1224,7 +1494,9 @@ sub same_email_addresses { my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2); return $email1_name eq $email2_name && - $email1_address eq $email2_address; + $email1_address eq $email2_address && + $name1_comment eq $name2_comment && + $comment1 eq $comment2; } sub which { @@ -2092,7 +2364,16 @@ sub report { 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); @@ -2363,6 +2644,15 @@ sub get_raw_comment { 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; @@ -2383,6 +2673,7 @@ sub process { 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; @@ -2396,6 +2687,8 @@ sub process { 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; @@ -2795,6 +3088,10 @@ sub process { # 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); @@ -2804,9 +3101,37 @@ sub process { if ($line =~ /^\s*signed-off-by:\s*(.*)/i) { $signoff++; $in_commit_log = 0; - if ($author ne '') { + 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; + } + } } } } @@ -2833,8 +3158,17 @@ sub process { 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", @@ -2863,7 +3197,7 @@ sub process { } my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); - my $suggested_email = format_email(($email_name, $email_address)); + 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); @@ -2874,8 +3208,76 @@ sub process { # Don't force email to have quotes # Allow just an angle bracketed address 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/; + } } } @@ -2894,17 +3296,73 @@ sub process { 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); + "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . $herecurr); } 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) { + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr); + } elsif ($rawlines[$linenr] !~ /^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]); + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . $herecurr . $rawlines[$linenr] . "\n"); } 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]); + "Co-developed-by and Signed-off-by: name/email do not match\n" . $herecurr . $rawlines[$linenr] . "\n"); + } + } + +# check if Reported-by: is followed by a Closes: tag + if ($sign_off =~ /^reported(?:|-and-tested)-by:$/i) { + if (!defined $lines[$linenr]) { + WARN("BAD_REPORTED_BY_LINK", + "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . "\n"); + } elsif ($rawlines[$linenr] !~ /^closes:\s*/i) { + WARN("BAD_REPORTED_BY_LINK", + "Reported-by: should be immediately followed by Closes: with a URL to the report\n" . $herecurr . $rawlines[$linenr] . "\n"); + } + } + } + + +# Check Fixes: styles is correct + if (!$in_header_lines && + $line =~ /^\s*fixes:?\s*(?:commit\s*)?[0-9a-f]{5,}\b/i) { + my $orig_commit = ""; + my $id = "0123456789ab"; + my $title = "commit title"; + my $tag_case = 1; + my $tag_space = 1; + my $id_length = 1; + my $id_case = 1; + my $title_has_quotes = 0; + + if ($line =~ /(\s*fixes:?)\s+([0-9a-f]{5,})\s+($balanced_parens)/i) { + my $tag = $1; + $orig_commit = $2; + $title = $3; + + $tag_case = 0 if $tag eq "Fixes:"; + $tag_space = 0 if ($line =~ /^fixes:? [0-9a-f]{5,} ($balanced_parens)/i); + + $id_length = 0 if ($orig_commit =~ /^[0-9a-f]{12}$/i); + $id_case = 0 if ($orig_commit !~ /[A-F]/); + + # Always strip leading/trailing parens then double quotes if existing + $title = substr($title, 1, -1); + if ($title =~ /^".*"$/) { + $title = substr($title, 1, -1); + $title_has_quotes = 1; + } + } + + my ($cid, $ctitle) = git_commit_info($orig_commit, $id, + $title); + + if ($ctitle ne $title || $tag_case || $tag_space || + $id_length || $id_case || !$title_has_quotes) { + if (WARN("BAD_FIXES_TAG", + "Please use correct Fixes: style 'Fixes: <12 chars of sha1> (\"\")' - ie: 'Fixes: $cid (\"$ctitle\")'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] = "Fixes: $cid (\"$ctitle\")"; } } } @@ -2918,8 +3376,11 @@ sub process { # Check for Gerrit Change-Ids not in any patch context if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) { - ERROR("GERRIT_CHANGE_ID", - "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr); + 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 @@ -2939,13 +3400,13 @@ sub process { length($line) > 75 && !($line =~ /^\s*[a-zA-Z0-9_\/\.\-\,]+\s+\|\s+\d+/ || # file delta changes - $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ || + $line =~ /^\s*(?:[\w\.\-\+]*\/)++[\w\.\-\+]+:/ || # filename then : - $line =~ /^\s*(?:Fixes:|Link:)/i || - # A Fixes: or Link: line + $line =~ /^\s*(?:Fixes:|$link_tags_search|$signature_tags)/i || + # A Fixes:, link or signature tag line $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", - "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr); + "Prefer a maximum 75 chars per line (possible unwrapped commit description?)\n" . $herecurr); $commit_log_long_line = 1; } @@ -2955,11 +3416,53 @@ sub process { $commit_log_possible_stack_dump = 0; } +# Check for odd tags before a URI/URL + if ($in_commit_log && + $line =~ /^\s*(\w+:)\s*http/ && $1 !~ /^$link_tags_search$/) { + if ($1 =~ /^v(?:ersion)?\d+/i) { + WARN("COMMIT_LOG_VERSIONING", + "Patch version information should be after the --- line\n" . $herecurr); + } else { + WARN("COMMIT_LOG_USE_LINK", + "Unknown link reference '$1', use $link_tags_print instead\n" . $herecurr); + } + } + +# Check for misuse of the link tags + if ($in_commit_log && + $line =~ /^\s*(\w+:)\s*(\S+)/) { + my $tag = $1; + my $value = $2; + if ($tag =~ /^$link_tags_search$/ && $value !~ m{^https?://}) { + WARN("COMMIT_LOG_WRONG_LINK", + "'$tag' should be followed by a public http(s) link\n" . $herecurr); + } + } + +# 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 && +# 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))) { @@ -2969,49 +3472,62 @@ sub process { 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 mailing list archives other than lore.kernel.org + if ($rawline =~ m{http.*\b$obsolete_archives}) { + WARN("PREFER_LORE_ARCHIVE", + "Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html\n" . $herecurr); } # Check for added, moved or deleted files @@ -3031,7 +3547,7 @@ sub process { ($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/writing-schema.rst\n"); + "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 @@ -3095,15 +3611,18 @@ sub process { # 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/; } @@ -3121,6 +3640,60 @@ sub process { } } +# 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 =~ /^-/); @@ -3163,51 +3736,47 @@ sub process { # Kconfig supports named choices), so use a word boundary # (\b) rather than a whitespace character (\s) $line =~ /^\+\s*(?:config|menuconfig|choice)\b/) { - my $length = 0; - my $cnt = $realcnt; - my $ln = $linenr + 1; - my $f; - my $is_start = 0; - my $is_end = 0; - for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) { - $f = $lines[$ln - 1]; - $cnt-- if ($lines[$ln - 1] !~ /^-/); - $is_end = $lines[$ln - 1] =~ /^\+/; + my $ln = $linenr; + my $needs_help = 0; + my $has_help = 0; + my $help_length = 0; + while (defined $lines[$ln]) { + my $f = $lines[$ln++]; next if ($f =~ /^-/); - last if (!$file && $f =~ /^\@\@/); + last if ($f !~ /^[\+ ]/); # !patch context - 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); - } - $length = -1; + if ($f =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { + $needs_help = 1; + next; + } + if ($f =~ /^\+\s*help\s*$/) { + $has_help = 1; + next; } - $f =~ s/^.//; - $f =~ s/#.*//; - $f =~ s/^\s+//; - next if ($f =~ /^$/); + $f =~ s/^.//; # strip patch context [+ ] + $f =~ s/#.*//; # strip # directives + $f =~ s/^\s+//; # strip leading blanks + next if ($f =~ /^$/); # skip blank lines + # At the end of this Kconfig block: # This only checks context lines in the patch # and so hopefully shouldn't trigger false # positives, even though some of these are # common words in help texts - if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice| - if|endif|menu|endmenu|source)\b/x) { - $is_end = 1; + if ($f =~ /^(?:config|menuconfig|choice|endchoice| + if|endif|menu|endmenu|source)\b/x) { last; } - $length++; + $help_length++ if ($has_help); } - if ($is_start && $is_end && $length < $min_conf_desc_length) { + if ($needs_help && + $help_length < $min_conf_desc_length) { + my $stat_real = get_stat_real($linenr, $ln - 1); WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr); + "please write a help paragraph that fully describes the config symbol\n" . "$here\n$stat_real\n"); } - #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } # check MAINTAINERS entries @@ -3250,13 +3819,6 @@ sub process { } } -# discourage the use of boolean for type definition attributes of Kconfig options - if ($realfile =~ /Kconfig/ && - $line =~ /^\+\s*\bboolean\b/) { - WARN("CONFIG_TYPE_BOOLEAN", - "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); - } - if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { my $flag = $1; @@ -3310,7 +3872,7 @@ sub process { my $comment = ""; if ($realfile =~ /\.(h|s|S)$/) { $comment = '/*'; - } elsif ($realfile =~ /\.(c|dts|dtsi)$/) { + } elsif ($realfile =~ /\.(c|rs|dts|dtsi)$/) { $comment = '//'; } elsif (($checklicenseline == 2) || $realfile =~ /\.(sh|pl|py|awk|tc|yaml)$/) { $comment = '#'; @@ -3337,7 +3899,7 @@ sub process { "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr); } if ($realfile =~ m@^Documentation/devicetree/bindings/@ && - not $spdx_license =~ /GPL-2\.0.*BSD-2-Clause/) { + $spdx_license !~ /GPL-2\.0(?:-only)? OR BSD-2-Clause/) { my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("SPDX_LICENSE_TAG", @@ -3347,12 +3909,23 @@ sub process { $fixed[$fixlinenr] =~ s/SPDX-License-Identifier: .*/SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)/; } } + if ($realfile =~ m@^include/dt-bindings/@ && + $spdx_license !~ /GPL-2\.0(?:-only)? OR \S+/) { + WARN("SPDX_LICENSE_TAG", + "DT binding headers should be licensed (GPL-2.0-only OR .*)\n" . $herecurr); + } } } } +# check for embedded filenames + if ($rawline =~ /^\+.*\b\Q$realfile\E\b/) { + 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)$/); + next if ($realfile !~ /\.(h|c|rs|s|S|sh|dtsi|dts)$/); # check for using SPDX-License-Identifier on the wrong line number if ($realline != $checklicenseline && @@ -3432,8 +4005,18 @@ sub process { # 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/core-api/asm-annotations.rst\n" . $herecurr); } # check we are in a valid source file C or perl if not then ignore this hunk @@ -3452,42 +4035,6 @@ sub process { } } -# check for repeated words separated by a single space - if ($rawline =~ /^\+/) { - while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) { - - my $first = $1; - my $second = $2; - - if ($first =~ /(?:struct|union|enum)/) { - pos($rawline) += length($first) + length($second) + 1; - next; - } - - next if ($first ne $second); - next if ($first eq 'long'); - - 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/; - } - } - } - } - # check for space before tabs. if ($rawline =~ /^\+/ && $rawline =~ / \t/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; @@ -3503,14 +4050,28 @@ sub process { # 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 @@ -3578,7 +4139,7 @@ sub process { 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); } @@ -3631,7 +4192,7 @@ sub process { if ($prevline =~ /^[\+ ]};?\s*$/ && $line =~ /^\+/ && !($line =~ /^\+\s*$/ || - $line =~ /^\+\s*EXPORT_SYMBOL/ || + $line =~ /^\+\s*(?:EXPORT_SYMBOL|early_param)/ || $line =~ /^\+\s*MODULE_/i || $line =~ /^\+\s*\#\s*(?:end|elif|else)/ || $line =~ /^\+[a-z_]*init/ || @@ -3660,43 +4221,48 @@ sub process { } # 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+(?:static\s+)?(?:const\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, "\+"); + } } } @@ -3749,12 +4315,16 @@ sub process { } # 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); + } } } @@ -4037,6 +4607,17 @@ sub process { #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/) { @@ -4152,13 +4733,13 @@ sub process { 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"; @@ -4180,8 +4761,7 @@ sub process { } 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; } @@ -4192,7 +4772,8 @@ sub process { } # 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) { @@ -4271,12 +4852,24 @@ sub process { } } +# 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*\)@) { @@ -4367,12 +4960,12 @@ sub process { } } -# avoid BUG() or BUG_ON() - if ($line =~ /\b(?:BUG|BUG_ON)\b/) { +# do not use BUG() or variants + if ($line =~ /\b(?!AA_|BUILD_|DCCP_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/) { my $msg_level = \&WARN; $msg_level = \&CHK if ($file); &{$msg_level}("AVOID_BUG", - "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr); + "Do not crash the kernel unless it is absolutely unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants\n" . $herecurr); } # avoid LINUX_VERSION_CODE @@ -4393,16 +4986,23 @@ sub process { "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); } +# prefer dev_<level> to dev_printk(KERN_<LEVEL> if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) { my $orig = $1; my $level = lc($orig); @@ -4412,6 +5012,12 @@ sub process { "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. @@ -4580,7 +5186,7 @@ sub process { if|for|while|switch|return|case| volatile|__volatile__| __attribute__|format|__extension__| - asm|__asm__)$/x) + asm|__asm__|scoped_guard)$/x) { # cpp #define statements have non-optional spaces, ie # if there is a space between the name and the open @@ -4853,7 +5459,7 @@ sub process { # 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]); @@ -4937,7 +5543,7 @@ sub process { ## $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) { ## } @@ -5067,9 +5673,13 @@ sub process { } } -#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) { @@ -5078,6 +5688,17 @@ sub process { } } +# 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; @@ -5105,7 +5726,7 @@ sub process { $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 ($perl_version_ok && @@ -5153,7 +5774,7 @@ sub process { # 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); } @@ -5196,6 +5817,7 @@ sub process { defined($stat) && defined($cond) && $line =~ /\b(?:if|while|for)\s*\(/ && $line !~ /^.\s*#/) { my ($s, $c) = ($stat, $cond); + my $fixed_assign_in_if = 0; if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) { if (ERROR("ASSIGN_IN_IF", @@ -5220,6 +5842,7 @@ sub process { $newline .= ')'; $newline .= " {" if (defined($brace)); fix_insert_line($fixlinenr + 1, $newline); + $fixed_assign_in_if = 1; } } } @@ -5243,8 +5866,20 @@ sub process { $stat_real = "[...]\n$stat_real"; } - ERROR("TRAILING_STATEMENTS", - "trailing statements should be on next line\n" . $herecurr . $stat_real); + if (ERROR("TRAILING_STATEMENTS", + "trailing statements should be on next line\n" . $herecurr . $stat_real) && + !$fixed_assign_in_if && + $cond_lines == 0 && + $fix && $perl_version_ok && + $fixed[$fixlinenr] =~ /^\+(\s*)((?:if|while|for)\s*$balanced_parens)\s*(.*)$/) { + my $indent = $1; + my $test = $2; + my $rest = rtrim($4); + if ($rest =~ /;$/) { + $fixed[$fixlinenr] = "\+$indent$test"; + fix_insert_line($fixlinenr + 1, "$indent\t$rest"); + } + } } } @@ -5342,14 +5977,18 @@ sub process { #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 ETHTOOL_LINK_MODE_<foo> variants + $var !~ /^ETHTOOL_LINK_MODE_/ && #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) { + while ($var =~ m{\b($Ident)}g) { my $word = $1; next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/); if ($check) { @@ -5437,9 +6076,9 @@ sub process { $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/) { } @@ -5486,7 +6125,9 @@ sub process { $dstat !~ /$exceptions/ && $dstat !~ /^\.$Ident\s*=/ && # .foo = $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ && # stringification #foo + $dstat !~ /^case\b/ && # case ... $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 {... @@ -5528,7 +6169,7 @@ sub process { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(sizeof|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; @@ -5557,6 +6198,9 @@ sub process { # check for line continuations outside of #defines, preprocessor #, and asm + } elsif ($realfile =~ m@/vmlinux.lds.h$@) { + $line =~ s/(\w+)/$maybe_linker_symbol{$1}++/ge; + #print "REAL: $realfile\nln: $line\nkeys:", sort keys %maybe_linker_symbol; } else { if ($prevline !~ /^..*\\$/ && $line !~ /^\+\s*\#.*\\$/ && # preprocessor @@ -5805,6 +6449,17 @@ sub process { "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", @@ -5816,7 +6471,8 @@ sub process { } # concatenated string without spaces between elements - if ($line =~ /$String[A-Za-z0-9_]/ || $line =~ /[A-Za-z0-9_]$String/) { + 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) { @@ -5829,7 +6485,7 @@ sub process { } # uncoalesced string fragments - if ($line =~ /$String\s*"/) { + if ($line =~ /$String\s*[Lu]?"/) { if (WARN("STRING_FRAGMENTS", "Consecutive strings are generally better as a single string\n" . $herecurr) && $fix) { @@ -5970,6 +6626,28 @@ sub process { "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 ($perl_version_ok && $line =~ /$LvalOrFunc\s*\&\s*($LvalOrFunc)\s*>>/ && @@ -6216,50 +6894,68 @@ sub process { } } -# 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__ section, prefer __section - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) { - my $old = substr($rawline, $-[1], $+[1] - $-[1]); - my $new = substr($old, 1, -1); - if (WARN("PREFER_SECTION", - "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*\)\s*\)\s*\)/__section($new)/; + 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(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; - - } - } - -# 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); } } @@ -6295,18 +6991,18 @@ sub process { 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/; } } @@ -6366,9 +7062,11 @@ sub process { $specifier = $1; $extension = $2; $qualifier = $3; - if ($extension !~ /[SsBKRraEehMmIiUDdgVCbGNOxtf]/ || + if ($extension !~ /[4SsBKRraEehMmIiUDdgVCbGNOxtf]/ || ($extension eq "f" && - defined $qualifier && $qualifier !~ /^w/)) { + defined $qualifier && $qualifier !~ /^w/) || + ($extension eq "4" && + defined $qualifier && $qualifier !~ /^cc/)) { $bad_specifier = $specifier; last; } @@ -6382,15 +7080,19 @@ sub process { } if ($bad_specifier ne "") { my $stat_real = get_stat_real($linenr, $lc); + my $msg_level = \&WARN; my $ext_type = "Invalid"; my $use = ""; if ($bad_specifier =~ /p[Ff]/) { $use = " - use %pS instead"; $use =~ s/pS/ps/ if ($bad_specifier =~ /pf/); + } elsif ($bad_specifier =~ /pA/) { + $use = " - '%pA' is only intended to be used from Rust code"; + $msg_level = \&ERROR; } - WARN("VSPRINTF_POINTER_EXTENSION", - "$ext_type vsprintf pointer extension '$bad_specifier'$use\n" . "$here\n$stat_real\n"); + &{$msg_level}("VSPRINTF_POINTER_EXTENSION", + "$ext_type vsprintf pointer extension '$bad_specifier'$use\n" . "$here\n$stat_real\n"); } } } @@ -6455,6 +7157,24 @@ sub process { # } # } +# strcpy uses that should likely be strscpy + if ($line =~ /\bstrcpy\s*\(/) { + WARN("STRCPY", + "Prefer strscpy over strcpy - see: https://github.com/KSPP/linux/issues/88\n" . $herecurr); + } + +# strlcpy uses that should likely be strscpy + if ($line =~ /\bstrlcpy\s*\(/) { + WARN("STRLCPY", + "Prefer strscpy over strlcpy - see: https://github.com/KSPP/linux/issues/89\n" . $herecurr); + } + +# strncpy uses that should likely be strscpy or strscpy_pad + if ($line =~ /\bstrncpy\s*\(/) { + WARN("STRNCPY", + "Prefer strscpy, strscpy_pad, or __nonstring over strncpy - see: https://github.com/KSPP/linux/issues/90\n" . $herecurr); + } + # typecasts on min/max could be min_t/max_t if ($perl_version_ok && defined $stat && @@ -6559,6 +7279,21 @@ sub process { "arguments for function declarations should follow identifier\n" . $herecurr); } + } elsif ($realfile =~ /\.c$/ && defined $stat && + $stat =~ /^\+extern struct\s+(\w+)\s+(\w+)\[\];/) + { + my ($st_type, $st_name) = ($1, $2); + + for my $s (keys %maybe_linker_symbol) { + #print "Linker symbol? $st_name : $s\n"; + goto LIKELY_LINKER_SYMBOL + if $st_name =~ /$s/; + } + WARN("AVOID_EXTERNS", + "found a file-scoped extern type:$st_type name:$st_name in .c file\n" + . "is this a linker symbol ?\n" . $herecurr); + LIKELY_LINKER_SYMBOL: + } elsif ($realfile =~ /\.c$/ && defined $stat && $stat =~ /^.\s*extern\s+/) { @@ -6627,14 +7362,16 @@ sub process { "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } -# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc +# check for (kv|k)[mz]alloc with multiplies that could be kmalloc_array/kvmalloc_array/kvcalloc/kcalloc if ($perl_version_ok && defined $stat && - $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) { + $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) { my $oldfunc = $3; my $a1 = $4; my $a2 = $10; my $newfunc = "kmalloc_array"; + $newfunc = "kvmalloc_array" if ($oldfunc eq "kvmalloc"); + $newfunc = "kvcalloc" if ($oldfunc eq "kvzalloc"); $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); my $r1 = $a1; my $r2 = $a2; @@ -6651,7 +7388,7 @@ sub process { "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) && $cnt == 1 && $fix) { - $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*((?:kv|k)[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; } } } @@ -6665,7 +7402,7 @@ sub process { } # check for alloc argument mismatch - if ($line =~ /\b(kcalloc|kmalloc_array)\s*\(\s*sizeof\b/) { + if ($line =~ /\b((?:devm_)?((?:k|kv)?(calloc|malloc_array)(?:_node)?))\s*\(\s*sizeof\b/) { WARN("ALLOC_ARRAY_ARGS", "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } @@ -6692,16 +7429,16 @@ sub process { } # check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too) - if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*\)/ && $1 !~ /^CONFIG_/) { + 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); + "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*$/) { + 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) && + "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) && $fix) { $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; } @@ -6837,12 +7574,14 @@ sub process { # 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); @@ -6861,6 +7600,27 @@ sub process { "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'; + } + } + +# check for array definition/declarations that should use flexible arrays instead + if ($sline =~ /^[\+ ]\s*\}(?:\s*__packed)?\s*;\s*$/ && + $prevline =~ /^\+\s*(?:\}(?:\s*__packed\s*)?|$Type)\s*$Ident\s*\[\s*(0|1)\s*\]\s*;\s*$/) { + if (ERROR("FLEXIBLE_ARRAY", + "Use C99 flexible arrays - see https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays\n" . $hereprev) && + $1 == '0' && $fix) { + $fixed[$fixlinenr - 1] =~ s/\[\s*0\s*\]/[]/; + } + } + # nested likely/unlikely calls if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { WARN("LIKELY_MISUSE", @@ -6878,10 +7638,28 @@ sub process { } } -# check for mutex_trylock_recursive usage - if ($line =~ /mutex_trylock_recursive/) { - ERROR("LOCKING", - "recursive locking is bad, do not use this ever.\n" . $herecurr); +# Complain about RCU Tasks Trace used outside of BPF (and of course, RCU). + our $rcu_trace_funcs = qr{(?x: + rcu_read_lock_trace | + rcu_read_lock_trace_held | + rcu_read_unlock_trace | + call_rcu_tasks_trace | + synchronize_rcu_tasks_trace | + rcu_barrier_tasks_trace | + rcu_request_urgent_qs_task + )}; + our $rcu_trace_paths = qr{(?x: + kernel/bpf/ | + include/linux/bpf | + net/bpf/ | + kernel/rcu/ | + include/linux/rcu + )}; + if ($line =~ /\b($rcu_trace_funcs)\s*\(/) { + if ($realfile !~ m{^$rcu_trace_paths}) { + WARN("RCU_TASKS_TRACE", + "use of RCU tasks trace is incorrect outside BPF or core RCU code\n" . $herecurr); + } } # check for lockdep_set_novalidate_class @@ -7025,6 +7803,13 @@ sub process { WARN("MODULE_LICENSE", "unknown module license " . $extracted_string . "\n" . $herecurr); } + if (!$file && $extracted_string eq '"GPL v2"') { + if (WARN("MODULE_LICENSE", + "Prefer \"GPL\" over \"GPL v2\" - see commit bf7fbeeae6db (\"module: Cure the MODULE_LICENSE \"GPL\" vs. \"GPL v2\" bogosity\")\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bMODULE_LICENSE\s*\(\s*"GPL v2"\s*\)/MODULE_LICENSE("GPL")/; + } + } } # check for sysctl duplicate constants @@ -7046,7 +7831,7 @@ sub process { 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); @@ -7060,9 +7845,33 @@ sub process { if ($signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); - } elsif (!$authorsignoff) { - WARN("NO_AUTHOR_SIGN_OFF", - "Missing Signed-off-by: line by nominal patch author '$author'\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"); + } } }