Skip to content

Commit

Permalink
checkpatch: add constant comparison on left side test
Browse files Browse the repository at this point in the history
"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 <[email protected]>
Cc: Nicolas Morey Chaisemartin <[email protected]>
Cc: Viresh Kumar <[email protected]>
Cc: Dan Carpenter <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
JoePerches authored and torvalds committed Sep 10, 2015
1 parent 54507b5 commit c5595fa
Showing 1 changed file with 29 additions and 0 deletions.
29 changes: 29 additions & 0 deletions scripts/checkpatch.pl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit c5595fa

Please sign in to comment.