All of lore.kernel.org
 help / color / mirror / Atom feed
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;



      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.