Messages in this thread Patch in this message | | | Date | Thu, 27 Aug 2015 09:09:07 +0200 (CEST) | From | Nicolas Morey Chaisemartin <> | Subject | Re: [PATCH] checkpatch: add --strict "pointer comparison to NULL" test |
| |
----- Original Message ----- > From: "Joe Perches" <joe@perches.com> > To: "Viresh Kumar" <viresh.kumar@linaro.org> > Cc: "Andrew Morton" <akpm@linux-foundation.org>, "Dan Carpenter" <dan.carpenter@oracle.com>, "Greg KH" > <gregkh@linuxfoundation.org>, "LKML" <linux-kernel@vger.kernel.org>, "Mike Holmes" <mike.holmes@linaro.org>, > nmorey@kalray.eu > Sent: Thursday, 27 August, 2015 5:05:37 AM > Subject: Re: [PATCH] checkpatch: add --strict "pointer comparison to NULL" test > > On Thu, 2015-08-27 at 07:49 +0530, Viresh Kumar wrote: > > Few colleagues asked me why isn't checkpatch warning for (NULL == ptr) > > or (NULL != ptr) checks, as it warns for (ptr == NULL) and (ptr != NULL). > > > > Did you miss it? or was it intentional ? > > I didn't miss it. > > NULL == foo is relatively unusual and not really worth the > bother. > > And because most likely, "CONST test variable" checks like > NULL != foo > and > 0 < bar > > should probably be a separate test. > > Something like: > --- > scripts/checkpatch.pl | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index e14dcdb..457ddef 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -4231,6 +4231,29 @@ sub process { > } > } > > +# comparisons with a constant on the left > + if ($^V && $^V ge 5.10.0 && > + $line =~ /\b($Constant|[A-Z_]+)\s*($Compare)\s*($LvalOrFunc)/) { > + my $const = $1; > + my $comp = $2; > + my $to = $3; > + my $newcomp = $comp; > + if (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 = "<"; > + }
I like the concept but are you sure about this? I think the "=" should be added or removed. If a < b, b > a, not b >= a.
Nicolas
| |