Add missing types to checkpatch.pl --list-types
cover Add missing types to checkpatch.pl --list-types
[PATCH 1/3] checkpatch: fix typo in comment
[PATCH 2/3] checkpatch: rename variables to avoid confusion [PATCH 3/3] checkpatch: add 6 missing types to --list-types
--- scripts/checkpatch.pl | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-)
Signature · 2 lines
Jean Delvare SUSE L3 Support
PATCH 1/3 checkpatch: fix typo in comment
--- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
scripts/checkpatch.pl +1 -1
@@ -2875,7 +2875,7 @@ sub process { # #defines that are a single string # # There are 3 different line length message types: -# LONG_LINE_COMMENT a comment starts before but extends beyond $max_linelength +# LONG_LINE_COMMENT a comment starts before but extends beyond $max_line_length # LONG_LINE_STRING a string starts before but extends beyond $max_line_length # LONG_LINE all other lines longer than $max_line_length #
Signature · 2 lines
Jean Delvare SUSE L3 Support
PATCH 2/3 checkpatch: rename variables to avoid confusion
In reply to Jean DelvareThe variable name "$msg_type" is sometimes used to set the message type, and sometimes used to set the message level. This works but is kind of confusing. Use "$msg_level" in the latter case instead, to make the code clearer.
--- scripts/checkpatch.pl | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)
scripts/checkpatch.pl +16 -16
@@ -2715,10 +2715,10 @@ sub process { my $typo_fix = $spelling_fix{lc($typo)}; $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/); $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/); - my $msg_type = \&WARN; - $msg_type = \&CHK if ($file); - if (&{$msg_type}("TYPO_SPELLING", - "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && + my $msg_level = \&WARN; + $msg_level = \&CHK if ($file); + if (&{$msg_level}("TYPO_SPELLING", + "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/; }
@@ -2753,10 +2753,10 @@ sub process { $rawline =~ /\b59\s+Temple\s+Pl/i || $rawline =~ /\b51\s+Franklin\s+St/i) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; - my $msg_type = \&ERROR; - $msg_type = \&CHK if ($file); - &{$msg_type}("FSF_MAILING_ADDRESS", - "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet) + my $msg_level = \&ERROR; + $msg_level = \&CHK if ($file); + &{$msg_level}("FSF_MAILING_ADDRESS", + "Do not include the paragraph about writing to the Free Software Foundation's mailing address from the sample GPL notice. The FSF has changed addresses in the past, and may do so again. Linux already includes a copy of the GPL.\n" . $herevet) } # check for Kconfig help text having a real description
@@ -3810,10 +3810,10 @@ sub process { # avoid BUG() or BUG_ON() if ($line =~ /\b(?:BUG|BUG_ON)\b/) { - my $msg_type = \&WARN; - $msg_type = \&CHK if ($file); - &{$msg_type}("AVOID_BUG", - "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr); + 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); } # avoid LINUX_VERSION_CODE
@@ -4339,11 +4339,11 @@ sub process { # messages are ERROR, but ?: are CHK if ($ok == 0) { - my $msg_type = \&ERROR; - $msg_type = \&CHK if (($op eq '?:' || $op eq '?' || $op eq ':') && $ctx =~ /VxV/); + my $msg_level = \&ERROR; + $msg_level = \&CHK if (($op eq '?:' || $op eq '?' || $op eq ':') && $ctx =~ /VxV/); - if (&{$msg_type}("SPACING", - "spaces required around that '$op' $at\n" . $hereptr)) { + if (&{$msg_level}("SPACING", + "spaces required around that '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; if (defined $fix_elements[$n + 2]) { $fix_elements[$n + 2] =~ s/^\s+//;
Signature · 2 lines
Jean Delvare SUSE L3 Support
PATCH 3/3 checkpatch: add 6 missing types to --list-types
In reply to Jean DelvareUnlike all other types, LONG_LINE, LONG_LINE_COMMENT and LONG_LINE_STRING are passed to WARN() through a variable. This causes the parser in list_types() to miss them and consequently they are not present in the output of --list-types.
Additionally, types TYPO_SPELLING, FSF_MAILING_ADDRESS and AVOID_BUG are passed with a variable level, causing the parser to miss them too.
So modify the regex to also catch these special cases.
--- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
scripts/checkpatch.pl +2 -1
@@ -145,7 +145,8 @@ sub list_types { close($script); my @types = (); - for ($text =~ /\b(?:(?:CHK|WARN|ERROR)\s*\(\s*"([^"]+)")/g) { + # 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, $_); } @types = sort(uniq(@types));
Signature · 2 lines
Jean Delvare SUSE L3 Support
Quoted 3 lines · Jean Delvare
[PATCH 1/3] checkpatch: fix typo in comment
[PATCH 2/3] checkpatch: rename variables to avoid confusion [PATCH 3/3] checkpatch: add 6 missing types to --list-types
Andrew, can you please pick these up? Thanks.
Jean, that's a pretty obscure defect.
Thanks for handling it.
btw: how or why did you find it?
Hi Joe,
Quoted 9 lines · Joe Perches
Quoted 3 lines · Jean Delvare
[PATCH 1/3] checkpatch: fix typo in comment
[PATCH 2/3] checkpatch: rename variables to avoid confusion [PATCH 3/3] checkpatch: add 6 missing types to --list-typesAndrew, can you please pick these up? Thanks.
Jean, that's a pretty obscure defect.
Thanks for handling it.
You're welcome.
Quoted 1 line
btw: how or why did you find it?
I wrote a patch which introduces a lot of long lines, and I am fine with that in this specific case, so I wanted to ask checkpatch to print everything BUT line-over-80-columns warnings. I used --list-types to find out the internal name for it... and couldn't see anything matching in the list. But I knew it had to exist, so looked into the code.
Signature · 2 lines
Jean Delvare SUSE L3 Support
Oh, btw, is there any purpose in listing a number in front of each type? It makes it look like one can pass that number instead of the type name, however it doesn't work, and I don't think it should as the numbering isn't stable and could change with any update of the script.
Can't we simplify the output and simply print the list of type names?
Thanks,
Signature · 2 lines
Jean Delvare SUSE L3 Support
Joe Perches
maintainerQuoted 2 lines · Jean Delvare
Oh, btw, is there any purpose in listing a number in front of each type?
Not really. I did that because I wanted a header
and because I had no idea how many of those types
existed and I was too lazy to count.
Quoted 3 lines
It makes it look like one can pass that number instead of the type name, however it doesn't work, and I don't think it should as the numbering isn't stable and could change with any update of the script.
True.
Quoted 1 line
Can't we simplify the output and simply print the list of type names?
<shrug> If you want.
Quoted 7 lines · Joe Perches
Quoted 2 lines · Jean Delvare
Oh, btw, is there any purpose in listing a number in front of each type?
Not really. I did that because I wanted a header
and because I had no idea how many of those types
existed and I was too lazy to count.
Given the intended audience of the script, I think "| wc -l" is the way to answer that question.
Quoted 9 lines
Quoted 3 lines
It makes it look like one can pass that number instead of the type name, however it doesn't work, and I don't think it should as the numbering isn't stable and could change with any update of the script.
True.
Quoted 1 line
Can't we simplify the output and simply print the list of type names?
<shrug> If you want.
Patch coming.
Signature · 2 lines
Jean Delvare SUSE L3 Support