From 1db81a682a2f2a664489c4e94f3b945f70a43a13 Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:44:20 -0800 Subject: checkpatch: add new exception to repeated word check Recently, commit 4f6ad8aa1eac ("checkpatch: move repeated word test") moved the repeated word test to check for more file types. But after this, if checkpatch.pl is run on MAINTAINERS, it generates several new warnings of the type: WARNING: Possible repeated word: 'git' For example: WARNING: Possible repeated word: 'git' +T: git git://git.kernel.org/pub/scm/linux/kernel/git/rw/uml.git So, the pattern "git git://..." is a false positive in this case. There are several other combinations which may produce a wrong warning message, such as "@size size", ":Begin begin", etc. Extend repeated word check to compare the characters before and after the word matches. If there is a non whitespace character before the first word or a non whitespace character excluding punctuation characters after the second word, then the check is skipped and the warning is avoided. Also add case insensitive word matching to the repeated word check. Link: https://lore.kernel.org/linux-kernel-mentees/81b6a0bb2c7b9256361573f7a13201ebcd4876f1.camel@perches.com/ Link: https://lkml.kernel.org/r/20201017162732.152351-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Suggested-by: Joe Perches Suggested-by: Lukas Bulwahn Acked-by: Joe Perches Cc: Aditya Srivastava Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fab38b493cef..7e505688257a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3050,19 +3050,30 @@ sub process { # check for repeated words separated by a single space if ($rawline =~ /^\+/ || $in_commit_log) { + 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 ($first ne $second); + 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); + if (WARN("REPEATED_WORD", "Possible repeated word: '$first'\n" . $herecurr) && $fix) { -- cgit From 8d0325cc74a31d517b5b4307c8d895c6e81076b7 Mon Sep 17 00:00:00 2001 From: Aditya Srivastava Date: Tue, 15 Dec 2020 20:44:24 -0800 Subject: checkpatch: fix false positives in REPEATED_WORD warning Presence of hexadecimal address or symbol results in false warning message by checkpatch.pl. For example, running checkpatch on commit b8ad540dd4e4 ("mptcp: fix memory leak in mptcp_subflow_create_socket()") results in warning: WARNING:REPEATED_WORD: Possible repeated word: 'ff' 00 00 00 00 00 00 00 00 00 2f 30 0a 81 88 ff ff ........./0..... Similarly, the presence of list command output in commit results in an unnecessary warning. For example, running checkpatch on commit 899e5ffbf246 ("perf record: Introduce --switch-output-event") gives: WARNING:REPEATED_WORD: Possible repeated word: 'root' dr-xr-x---. 12 root root 4096 Apr 27 17:46 .. Here, it reports 'ff' and 'root' to be repeated, but it is in fact part of some address or code, where it has to be repeated. In these cases, the intent of the warning to find stylistic issues in commit messages is not met and the warning is just completely wrong in this case. To avoid these warnings, add an additional regex check for the directory permission pattern and avoid checking the line for this class of warning. Similarly, to avoid hex pattern, check if the word consists of hex symbols and skip this warning if it is not among the common english words formed using hex letters. A quick evaluation on v5.6..v5.8 showed that this fix reduces REPEATED_WORD warnings by the frequency of 1890. A quick manual check found all cases are related to hex output or list command outputs in commit messages. Link: https://lkml.kernel.org/r/20201024102253.13614-1-yashsri421@gmail.com Signed-off-by: Aditya Srivastava Acked-by: Joe Perches Cc: Dwaipayan Ray Cc: Lukas Bulwahn Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7e505688257a..519da711cb12 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -853,6 +853,13 @@ our $declaration_macros = qr{(?x: (?: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)); @@ -3049,7 +3056,9 @@ sub process { } # check for repeated words separated by a single space - if ($rawline =~ /^\+/ || $in_commit_log) { +# 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) { @@ -3074,6 +3083,11 @@ sub process { 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) { -- cgit From 4104a20646fe20ed5aa9be883eef7340b219f9a8 Mon Sep 17 00:00:00 2001 From: Łukasz Stelmach Date: Tue, 15 Dec 2020 20:44:27 -0800 Subject: checkpatch: ignore generated CamelCase defines and enum values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ignore autogenerated CamelCase-like defines and enum values like DRM_MODE_CONNECTOR_Unknown or ETHTOOL_LINK_MODE_Asym_Pause_BIT. Link: https://lkml.kernel.org/r/20201022184916.7904-1-l.stelmach@samsung.com Signed-off-by: Łukasz Stelmach Suggested-by: Joe Perches Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 ++ 1 file changed, 2 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 519da711cb12..6bbc24e66916 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5320,6 +5320,8 @@ 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 variants $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && #Ignore SI style variants like nS, mV and dB -- cgit From 73169765e6e7ac54528778faa592b15df5c8a93c Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 15 Dec 2020 20:44:30 -0800 Subject: checkpatch: prefer static const declarations There are about 100,000 uses of 'static const ' but about 400 uses of 'static const' in the kernel where type is not a pointer. The kernel almost always uses "static const" over "const static" as there is a compiler warning for that declaration style. But there is no compiler warning for "static const". So add a checkpatch warning for the atypical declaration uses of. const static and static const For example: $ ./scripts/checkpatch.pl -f --emacs --quiet --nosummary -types=static_const arch/arm/crypto/aes-ce-glue.c arch/arm/crypto/aes-ce-glue.c:75: WARNING: Move const after static - use 'static const u8' #75: FILE: arch/arm/crypto/aes-ce-glue.c:75: + static u8 const rcon[] = { Link: https://lkml.kernel.org/r/4b863be68e679546b40d50b97a4a806c03056a1c.camel@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 6bbc24e66916..4018bf89e63a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4232,6 +4232,18 @@ sub process { } } +# check for const static or static const declarations +# prefer 'static const ' over 'const static ' and 'static 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", -- cgit From dc58bc553e7a8e1d6eeaffd92cb8b346e0d62f70 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 15 Dec 2020 20:44:33 -0800 Subject: checkpatch: allow --fix removal of unnecessary break statements switch/case use of break after a return, goto or break is unnecessary. There is an existing warning for the return and goto uses, so add break and a --fix option too. Link: https://lkml.kernel.org/r/d9ea654104d55f590fb97d252d64a66b23c1a096.camel@perches.com Signed-off-by: Joe Perches Cc: Julia Lawall Cc: Tom Rix Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 4018bf89e63a..c128875b6666 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3699,12 +3699,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); + } } } -- cgit From 7ebe1d173cae0778fa748ea3f2ae20dfa0f58e10 Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:44:36 -0800 Subject: checkpatch: extend attributes check to handle more patterns It is generally preferred that the macros from include/linux/compiler_attributes.h are used, unless there is a reason not to. checkpatch currently checks __attribute__ for each of packed, aligned, section, printf, scanf, and weak. Other declarations in compiler_attributes.h are not handled. Add a generic test to check the presence of such attributes. Some attributes require more specific handling and are kept separate. Also add fixes to the generic attributes check to substitute the correct conversions. New attributes which are now handled are: __always_inline__ __assume_aligned__(a, ## __VA_ARGS__) __cold__ __const__ __copy__(symbol) __designated_init__ __externally_visible__ __gnu_inline__ __malloc__ __mode__(x) __no_caller_saved_registers__ __noclone__ __noinline__ __nonstring__ __noreturn__ __pure__ __unused__ __used__ Declarations which contain multiple attributes like __attribute__((__packed__, __cold__)) are also handled except when proper conversions for one or more attributes of the list cannot be determined. Link: https://lore.kernel.org/linux-kernel-mentees/3ec15b41754b01666d94b76ce51b9832c2dd577a.camel@perches.com/ Link: https://lkml.kernel.org/r/20201025193103.23223-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Suggested-by: Joe Perches Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 109 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 38 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c128875b6666..92949b8c5c76 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6187,50 +6187,83 @@ 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); - } - -# 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); - } - -# 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)/; + $rawline =~ /\b__attribute__\s*\(\s*($balanced_parens)\s*\)/) { + my $attr = $1; + $attr =~ s/\s*\(\s*(.*)\)\s*/$1/; + + my %attr_list = ( + "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", + "used" => "__used" + ); + + my @conv_array = (); + my $conv_possible = 1; + + while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { + my $curr_attr = $1; + my $params = ''; + $params = $2 if defined($2); + $curr_attr =~ s/^[\s_]+|[\s_]+$//g; + + if (exists($attr_list{$curr_attr})) { + if ($curr_attr eq "format" && $params) { + $params =~ /^\s*\(\s*(\w+)\s*,\s*(.*)/; + push(@conv_array, "__$1\($2"); + } else { + my $new = $attr_list{$curr_attr}; + push(@conv_array, "$new$params"); + } + } else { + $conv_possible = 0; + last; + } } - } -# 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; + if (scalar @conv_array > 0 && $conv_possible == 1) { + my $replace = join(' ', @conv_array); + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "$replace is preferred over __attribute__(($attr))\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$replace/; + $fixed[$fixlinenr] =~ s/\}\Q$replace\E/} $replace/; + } + } + # Check for __attribute__ section, prefer __section + if ($attr =~ /^_*section_*\s*\(\s*("[^"]*")/) { + my $old = substr($attr, $-[1], $+[1] - $-[1]); + my $new = substr($old, 1, -1); + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__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)/; + } } - } -# 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); } } -- cgit From 47ca69b85821e150cfbbe86a18a038e9488c0090 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Tue, 15 Dec 2020 20:44:40 -0800 Subject: checkpatch: add a fixer for missing newline at eof Remove the trailing error message from the fixed lines. Link: https://lkml.kernel.org/r/20201017142546.28988-1-trix@redhat.com Signed-off-by: Tom Rix Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 92949b8c5c76..7800a090e8fe 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3418,8 +3418,11 @@ 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 we are in a valid source file C or perl if not then ignore this hunk -- cgit From 339f29d91acf3f49bcf919f0e11437438f58559a Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 15 Dec 2020 20:44:43 -0800 Subject: checkpatch: update __attribute__((section("name"))) quote removal commit 33def8498fdd ("treewide: Convert macro and uses of __section(foo) to __section("foo")") removed the stringification of the section name and now requires quotes around the named section. Update checkpatch to not remove any quotes when suggesting conversion of __attribute__((section("name"))) to __section("name") Miscellanea: o Add section to the hash with __section replacement o Remove separate test for __attribute__((section o Remove the limitation on converting attributes containing only known, possible conversions. Any unknown attribute types are now left as-is and known types are converted and moved before __attribute__ and removed from within the __attribute__((list...)). [joe@perches.com: eliminate the separate test below the possible conversions loop] Link: https://lkml.kernel.org/r/58e9d55e933dc8fdc6af489f2ad797fa8eb13e44.camel@perches.com Link: https://lkml.kernel.org/r/c04dd1c810e8d6a68e6a632e3191ae91651c8edf.camel@perches.com Signed-off-by: Joe Perches Cc: Dwaipayan Ray Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 47 +++++++++++++++-------------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7800a090e8fe..58095d9d8f34 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6216,50 +6216,33 @@ sub process { "noreturn" => "__noreturn", "packed" => "__packed", "pure" => "__pure", + "section" => "__section", "used" => "__used" ); - my @conv_array = (); - my $conv_possible = 1; - while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { - my $curr_attr = $1; + 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*(.*)/; - push(@conv_array, "__$1\($2"); + $new = "__$1\($2"; } else { - my $new = $attr_list{$curr_attr}; - push(@conv_array, "$new$params"); + $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*\)//; } - } else { - $conv_possible = 0; - last; - } - } - - if (scalar @conv_array > 0 && $conv_possible == 1) { - my $replace = join(' ', @conv_array); - if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", - "$replace is preferred over __attribute__(($attr))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*\Q$attr\E\s*\)\s*\)/$replace/; - $fixed[$fixlinenr] =~ s/\}\Q$replace\E/} $replace/; - } - } - - # Check for __attribute__ section, prefer __section - if ($attr =~ /^_*section_*\s*\(\s*("[^"]*")/) { - my $old = substr($attr, $-[1], $+[1] - $-[1]); - my $new = substr($old, 1, -1); - if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", - "__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)/; } } -- cgit From 7580c5b9b464f8936be850ef278927671338bbf2 Mon Sep 17 00:00:00 2001 From: Aditya Srivastava Date: Tue, 15 Dec 2020 20:44:47 -0800 Subject: checkpatch: add fix option for GERRIT_CHANGE_ID Currently, whenever a Gerrit Change-Id is present in a commit, checkpatch.pl warns to remove the Change-Id before submitting the patch. E.g., running checkpatch on commit adc311a5bbf6 ("iwlwifi: bump FW API to 53 for 22000 series") reports this error: ERROR: Remove Gerrit Change-Id's before submitting upstream Change-Id: I5725e46394f3f53c3069723fd513cc53c7df383d Provide a simple fix option by simply deleting the indicated line. Link: https://lkml.kernel.org/r/20201030114447.24199-1-yashsri421@gmail.com Signed-off-by: Aditya Srivastava Acked-by: Joe Perches Cc: Lukas Bulwahn Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 58095d9d8f34..3480bcb9bbc6 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2852,8 +2852,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 -- cgit From 0830aab0e1d4d9bd391e5723c39f4b3b002fffb3 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 15 Dec 2020 20:44:50 -0800 Subject: checkpatch: add __alias and __weak to suggested __attribute__ conversions Add __alias and __weak to the suggested __attribute__(()) conversions. Link: https://lkml.kernel.org/r/7b74137743c58ce0633ec4d575b94e2210e4dbe7.camel@perches.com Signed-off-by: Joe Perches Cc: Dwaipayan Ray Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3480bcb9bbc6..5a1096a4e220 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6200,6 +6200,7 @@ sub process { $attr =~ s/\s*\(\s*(.*)\)\s*/$1/; my %attr_list = ( + "alias" => "__alias", "aligned" => "__aligned", "always_inline" => "__always_inline", "assume_aligned" => "__assume_aligned", @@ -6220,7 +6221,8 @@ sub process { "packed" => "__packed", "pure" => "__pure", "section" => "__section", - "used" => "__used" + "used" => "__used", + "weak" => "__weak" ); while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { -- cgit From fccaebf00e603694b892b46722a52db3d4298561 Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:44:53 -0800 Subject: checkpatch: improve email parsing checkpatch doesn't report warnings for many common mistakes in emails. Some of which are trailing commas and incorrect use of email comments. At the same time several false positives are reported due to incorrect handling of mail comments. The most common of which is due to the pattern: # X.X Improve email parsing in checkpatch. Some general email rules are defined: - Multiple name comments should not be allowed. - Comments inside address should not be allowed. - In general comments should be enclosed within parentheses. Relaxation is given to comments beginning with #. - Stable addresses should not begin with a name. - Comments in stable addresses should begin only with a #. Improvements to parsing: - Detect and report unexpected content after email. - Quoted names are excluded from comment parsing. - Trailing dots, commas or quotes in email are removed during formatting. Correspondingly a BAD_SIGN_OFF warning is emitted. - Improperly quoted email like '"name
"' are now warned about. In addition, added fixes for all the possible rules. Link: https://lore.kernel.org/linux-kernel-mentees/6c275d95c3033422addfc256a30e6ae3dd37941d.camel@perches.com/ Link: https://lore.kernel.org/linux-kernel-mentees/20201105200857.GC1333458@kroah.com/ Link: https://lkml.kernel.org/r/20201108100632.75340-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Suggested-by: Joe Perches Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 108 ++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 91 insertions(+), 17 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 5a1096a4e220..2749f32dffe9 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1159,6 +1159,7 @@ sub parse_email { my ($formatted_email) = @_; my $name = ""; + my $quoted = ""; my $name_comment = ""; my $address = ""; my $comment = ""; @@ -1190,14 +1191,20 @@ sub parse_email { } } - $comment = trim($comment); - $name = trim($name); - $name =~ s/^\"|\"$//g; - if ($name =~ s/(\s*\([^\)]+\))\s*//) { - $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/(? 1) { WARN("BAD_SIGN_OFF", - "email address '$email' might be better as '$suggested_email'\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 commments 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/; + } } } -- cgit From e73d27159400f3ed7300387ce5d69707af8bf0fd Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:44:56 -0800 Subject: checkpatch: fix spelling errors and remove repeated word Delete repeated word in scripts/checkpatch.pl: "are are" -> "are" Fix typos: "commments" -> "comments" "falsly" -> "falsely" Link: https://lkml.kernel.org/r/20201113152316.62975-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2749f32dffe9..041b82f6669e 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2841,7 +2841,7 @@ sub process { # stable@vger.kernel.org or stable@kernel.org shouldn't - # have an email name. In addition commments should strictly + # have an email name. In addition comments should strictly # begin with a # if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/i) { if (($comment ne "" && $comment !~ /^#.+/) || @@ -5000,7 +5000,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) { ## } @@ -7109,7 +7109,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); -- cgit From 27b379af61025e32a9baf3a33e939941682693ba Mon Sep 17 00:00:00 2001 From: Aditya Srivastava Date: Tue, 15 Dec 2020 20:44:59 -0800 Subject: checkpatch: avoid COMMIT_LOG_LONG_LINE warning for signature tags Currently checkpatch warns us for long lines in commits even for signature tag lines. Generally these lines exceed the 75-character limit because of: 1) long names and long email address 2) some comments on scoped review and acknowledgement, i.e., for a dedicated pointer on what was reported by the identity in 'Reported-by' 3) some additional comments on CC: stable@vger.org tags Exclude signature tag lines from this class of warning. There were 1896 COMMIT_LOG_LONG_LINE warnings in v5.6..v5.8 before this patch application and 1879 afterwards. A quick manual check found all the dropped warnings related to signature tags. Link: https://lkml.kernel.org/r/20201116083754.10629-1-yashsri421@gmail.com Signed-off-by: Aditya Srivastava Acked-by: Joe Perches Cc: Lukas Bulwahn Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 041b82f6669e..176ea2c189b8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2952,8 +2952,8 @@ sub process { # 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 75 chars per line)\n" . $herecurr); -- cgit From 03f4935135b9efeb780b970ba023c201f81cf4e6 Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:45:02 -0800 Subject: checkpatch: fix unescaped left brace There is an unescaped left brace in a regex in OPEN_BRACE check. This throws a runtime error when checkpatch is run with --fix flag and the OPEN_BRACE check is executed. Fix it by escaping the left brace. Link: https://lkml.kernel.org/r/20201115202928.81955-1-dwaipayanray1@gmail.com Fixes: 8d1824780f2f ("checkpatch: add --fix option for a couple OPEN_BRACE misuses") Signed-off-by: Dwaipayan Ray Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 176ea2c189b8..fdfd5ec09be6 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4505,7 +4505,7 @@ sub process { $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)); -- cgit From da7355ab4e4a0021924e87acce2b9fb7e6f3264e Mon Sep 17 00:00:00 2001 From: Aditya Srivastava Date: Tue, 15 Dec 2020 20:45:06 -0800 Subject: checkpatch: add fix option for ASSIGNMENT_CONTINUATIONS Currently, checkpatch warns us if an assignment operator is placed at the start of a line and not at the end of previous line. E.g., running checkpatch on commit 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug") reports: CHECK: Assignment operator '=' should be on the previous line + struct netvsc_device *nvdev + = container_of(w, struct netvsc_device, subchan_work); Provide a simple fix by appending assignment operator to the previous line and removing from the current line, if both the lines are additions (ie start with '+') Link: https://lkml.kernel.org/r/20201121120407.22942-1-yashsri421@gmail.com Signed-off-by: Aditya Srivastava Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index fdfd5ec09be6..7dc094445d83 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3533,8 +3533,14 @@ 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 -- cgit From 8e08f0765f3744c65e2c8c570004079883b2f546 Mon Sep 17 00:00:00 2001 From: Aditya Srivastava Date: Tue, 15 Dec 2020 20:45:09 -0800 Subject: checkpatch: add fix option for LOGICAL_CONTINUATIONS Currently, checkpatch warns if logical continuations are placed at the start of a line and not at the end of previous line. E.g., running checkpatch on commit 3485507fc272 ("staging: bcm2835-camera: Reduce length of enum names") reports: CHECK:LOGICAL_CONTINUATIONS: Logical continuations should be on the previous line + if (!ret + && camera_port == Provide a simple fix by inserting logical operator at the last non-comment, non-whitespace char of the previous line and removing from current line, if both the lines are additions(ie start with '+') Link: https://lkml.kernel.org/r/20201123102818.24364-1-yashsri421@gmail.com Signed-off-by: Aditya Srivastava Acked-by: Joe Perches Cc: Lukas Bulwahn Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7dc094445d83..241c064f358b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3545,8 +3545,16 @@ sub process { # 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 -- cgit From 831242ab8dffab4cf2f89c597d5902ac86caeefe Mon Sep 17 00:00:00 2001 From: Aditya Srivastava Date: Tue, 15 Dec 2020 20:45:12 -0800 Subject: checkpatch: add fix and improve warning msg for non-standard signature Currently checkpatch warns for BAD_SIGN_OFF on non-standard signature styles. A large number of these warnings occur because of typo mistakes in signature tags. An evaluation over v4.13..v5.8 showed that out of 539 warnings due to non-standard signatures, 87 are due to typo mistakes. Following are the standard signature tags which are often incorrectly used, along with their individual counts of incorrect use (over v4.13..v5.8): Reviewed-by: 42 Signed-off-by: 25 Reported-by: 6 Acked-by: 4 Tested-by: 4 Suggested-by: 4 Provide a fix by calculating levenshtein distance for the signature tag with all the standard signatures and suggest a fix with a signature, whose edit distance is less than or equal to 2 with the misspelled signature. Out of the 86 mispelled signatures fixed with this approach, 85 were found to be good corrections and 1 was bad correction. Following was found to be a bad correction: Tweeted-by (count: 1) => Tested-by Link: https://lkml.kernel.org/r/20201128204333.7054-1-yashsri421@gmail.com Signed-off-by: Aditya Srivastava Acked-by: Joe Perches Cc: Lukas Bulwahn Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 71 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 241c064f358b..d7616a6f1436 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -506,6 +506,64 @@ our $signature_tags = qr{(?xi: Cc: )}; +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}, @@ -2773,8 +2831,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", -- cgit From 70eb2275ff8e0b4cafe67176674d580c987c071d Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:45:15 -0800 Subject: checkpatch: add warning for unnecessary use of %h[xudi] and %hh[xudi] Modifiers %h and %hh should never be used. Commit cbacb5ab0aa0 ("docs: printk-formats: Stop encouraging use of unnecessary %h[xudi] and %hh[xudi]") specifies that: "Standard integer promotion is already done and %hx and %hhx is useless so do not encourage the use of %hh[xudi] or %h[xudi]." "The "h" and "hh" things should never be used. The only reason for them being used if you have an "int", but you want to print it out as a "char" (and honestly, that is a really bad reason, you'd be better off just using a proper cast to make the code more obvious)." Add a new check to emit a warning on finding an unneeded use of %h or %hh modifier. Also add a fix option to the check. Link: https://lore.kernel.org/lkml/4910042649a4f3ab22fac93191b8c1fa0a2e17c3.camel@perches.com/ Link: https://lkml.kernel.org/r/20201128200046.78739-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Suggested-by: Joe Perches Suggested-by: Lukas Bulwahn Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index d7616a6f1436..2716b9bfc0c2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -6102,6 +6102,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*>>/ && -- cgit From 084a617acfa08118eafb51a6ef43e6fa4705853d Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:45:18 -0800 Subject: checkpatch: add warning for lines starting with a '#' in commit log Commit log lines starting with '#' are dropped by git as comments. Add a check to emit a warning for these lines. Also add a --fix option to insert a space before the leading '#' in such lines. Link: https://lkml.kernel.org/r/20201202205740.127986-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Suggested-by: Joe Perches Suggested-by: Peilin Ye Tested-by: Peilin Ye Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2716b9bfc0c2..b7dde7260921 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3033,6 +3033,15 @@ sub process { $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|base-commit):/i && -- cgit From 7da07c31b1df1fa973d184378862443302fd1129 Mon Sep 17 00:00:00 2001 From: Dwaipayan Ray Date: Tue, 15 Dec 2020 20:45:21 -0800 Subject: checkpatch: fix TYPO_SPELLING check for words with apostrophe checkpatch reports a false TYPO_SPELLING warning for some words containing an apostrophe when run with --codespell option. A false positive is "doesn't". Occurrence of the word causes checkpatch to emit the following warning: "WARNING: 'doesn'' may be misspelled - perhaps 'doesn't'?" Modify the regex pattern to be more in line with the codespell default word matching regex. This fixes the word capture and avoids the false warning. In addition, highlight the misspelled word location by adding a caret below the word. [akpm@linux-foundation.org: make matched misspelling more obvious, per Joe] Link: https://lkml.kernel.org/r/09c24ef1aa2f1c4fe909d76f5426f08780b9d81c.camel@perches.com Link: https://lkml.kernel.org/r/20201201190729.169733-1-dwaipayanray1@gmail.com Signed-off-by: Dwaipayan Ray Suggested-by: Joe Perches Reported-by: Peilin Ye Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b7dde7260921..7b086d1cd6c2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3182,15 +3182,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/; } -- cgit From f5eea3b0442da801404859a780c02721d649f02f Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Tue, 15 Dec 2020 20:45:24 -0800 Subject: checkpatch: add printk_once and printk_ratelimit to prefer pr_ warning Add the _once and _ratelimited variants to the test for printk(KERN_ that should prefer pr_. Miscellanea: o Add comment description for the conversions [joe@perches.com: fixlet] Link: https://lkml.kernel.org/r/32260871d4718ba7f48a8e9e07452bb76de300db.camel@perches.comLink: https://lkml.kernel.org/r/993b72b2ef91a57c5e725b52971ce3fd31375061.camel@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'scripts/checkpatch.pl') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 7b086d1cd6c2..00085308ed9d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4543,16 +4543,23 @@ sub process { "printk() should include KERN_ facility level\n" . $herecurr); } - if ($line =~ /\bprintk\s*\(\s*KERN_([A-Z]+)/) { - my $orig = $1; +# prefer variants of (subsystem|netdev|dev|pr)_ to printk(KERN_ + 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_ to dev_printk(KERN_ if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) { my $orig = $1; my $level = lc($orig); -- cgit