From: Joe Perches <joe@perches.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Nicolas Morey Chaisemartin <nmorey@kalray.eu>,
Viresh Kumar <viresh.kumar@linaro.org>,
Dan Carpenter <dan.carpenter@oracle.com>,
LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH] checkpatch: add constant comparison on left side test
Date: Thu, 27 Aug 2015 10:33:22 -0700 [thread overview]
Message-ID: <1440696802.11525.96.camel@perches.com> (raw)
In-Reply-To: <1440660159.11525.79.camel@perches.com>
"CONST <comparison> 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 <joe@perches.com>
---
scripts/checkpatch.pl | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index e14dcdb..5fca1da 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;
prev parent reply other threads:[~2015-08-27 17:33 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-13 18:57 [PATCH] checkpatch: add --strict "pointer comparison to NULL" test Joe Perches
2015-08-27 2:19 ` Viresh Kumar
2015-08-27 3:05 ` Joe Perches
2015-08-27 7:09 ` Nicolas Morey Chaisemartin
2015-08-27 7:22 ` Joe Perches
2015-08-27 17:33 ` Joe Perches [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1440696802.11525.96.camel@perches.com \
--to=joe@perches.com \
--cc=akpm@linux-foundation.org \
--cc=dan.carpenter@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=nmorey@kalray.eu \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.