From 63b7c73ec86b8c73560d22c7552bc6f47f0b8da9 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 8 Sep 2017 16:16:01 -0700 Subject: checkpatch: add --strict check for ifs with unnecessary parentheses An if statement test like if ((foo == bar) && (baz != qux)) can arguably be better written without the parentheses as if (foo == bar && baz != qux) Add a test to find these cases. Link: http://lkml.kernel.org/r/dcd0561ddd0fa43c51a420d53b550d738bf42001.1502734458.git.joe@perches.com Signed-off-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2287a0bca863b..143ab5ca2c412 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4496,6 +4496,30 @@ sub process { } } +# check for unnecessary parentheses around comparisons in if uses + if ($^V && $^V ge 5.10.0 && defined($stat) && + $stat =~ /(^.\s*if\s*($balanced_parens))/) { + my $if_stat = $1; + my $test = substr($2, 1, -1); + my $herectx; + while ($test =~ /(?:^|[^\w\&\!\~])+\s*\(\s*([\&\!\~]?\s*$Lval\s*(?:$Compare\s*$FuncArg)?)\s*\)/g) { + my $match = $1; + # avoid parentheses around potential macro args + next if ($match =~ /^\s*\w+\s*$/); + if (!defined($herectx)) { + $herectx = $here . "\n"; + my $cnt = statement_rawlines($if_stat); + for (my $n = 0; $n < $cnt; $n++) { + my $rl = raw_line($linenr, $n); + $herectx .= $rl . "\n"; + last if $rl =~ /^[ \+].*\{/; + } + } + CHK("UNNECESSARY_PARENTHESES", + "Unnecessary parentheses around '$match'\n" . $herectx); + } + } + #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:/)) { -- cgit v1.2.3 From ab1ecabf4fdbd80b9d4ab7eacfd318c517b3f8f4 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Fri, 8 Sep 2017 16:16:04 -0700 Subject: checkpatch: fix typo in comment Link: http://lkml.kernel.org/r/20170902175249.15bb77f2@endymion Signed-off-by: Jean Delvare 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') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 143ab5ca2c412..3e66fbfd6234a 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -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 # -- cgit v1.2.3 From 0675a8fbd71c1a0b605ce82adea37131452f1070 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Fri, 8 Sep 2017 16:16:07 -0700 Subject: checkpatch: rename variables to avoid confusion The 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. Link: http://lkml.kernel.org/r/20170902175345.175db33a@endymion Signed-off-by: Jean Delvare Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3e66fbfd6234a..b3a1bc70d9ee6 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -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+//; -- cgit v1.2.3 From 0547fa5851c921eb1f64286f6b2ae7ab5df24e6e Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Fri, 8 Sep 2017 16:16:11 -0700 Subject: checkpatch: add 6 missing types to --list-types Unlike 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. Link: http://lkml.kernel.org/r/20170902175610.7e4a7c9d@endymion Fixes: 3beb42eced39 ("checkpatch: add --list-types to show message types to show or ignore") Signed-off-by: Jean Delvare Acked-by: Joe Perches Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts') diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index b3a1bc70d9ee6..dd2c262aebbf2 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -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)); -- cgit v1.2.3