From c5595fa2f1ce3c1a74dbd2ddc949257e80d81dc0 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Wed, 9 Sep 2015 15:37:58 -0700 Subject: [PATCH] checkpatch: add constant comparison on left side test "CONST variable" checks like: if (NULL != foo) and while (0 < bar(...)) where a constant (or what appears to be a constant like an upper case identifier) is on the left of a comparison are generally preferred to be written using the constant on the right side like: if (foo != NULL) and while (bar(...) > 0) Add a test for this. Add a --fix option too, but only do it when the code is immediately surrounded by parentheses to avoid misfixing things like "(0 < bar() + constant)" Signed-off-by: Joe Perches Cc: Nicolas Morey Chaisemartin Cc: Viresh Kumar Cc: Dan Carpenter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- scripts/checkpatch.pl | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 97ee377c4f51..f2a1131b2f8b 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -4231,6 +4231,35 @@ sub process { } } +# comparisons with a constant or upper case identifier on the left +# avoid cases like "foo + BAR < baz" +# only fix matches surrounded by parentheses to avoid incorrect +# conversions like "FOO < baz() + 5" being "misfixed" to "baz() > FOO + 5" + if ($^V && $^V ge 5.10.0 && + $line =~ /^\+(.*)\b($Constant|[A-Z_][A-Z0-9_]*)\s*($Compare)\s*($LvalOrFunc)/) { + my $lead = $1; + my $const = $2; + my $comp = $3; + my $to = $4; + my $newcomp = $comp; + if ($lead !~ /$Operators\s*$/ && + $to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/ && + WARN("CONSTANT_COMPARISON", + "Comparisons should place the constant on the right side of the test\n" . $herecurr) && + $fix) { + if ($comp eq "<") { + $newcomp = ">"; + } elsif ($comp eq "<=") { + $newcomp = ">="; + } elsif ($comp eq ">") { + $newcomp = "<"; + } elsif ($comp eq ">=") { + $newcomp = "<="; + } + $fixed[$fixlinenr] =~ s/\(\s*\Q$const\E\s*$Compare\s*\Q$to\E\s*\)/($to $newcomp $const)/; + } + } + # 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;