* checkpatch: comparisons with a constant on the left
@ 2019-10-11 1:52 Sergey Senozhatsky
2019-10-11 3:23 ` Joe Perches
0 siblings, 1 reply; 3+ messages in thread
From: Sergey Senozhatsky @ 2019-10-11 1:52 UTC (permalink / raw)
To: Joe Perches; +Cc: Sergey Senozhatsky, linux-kernel
Hi Joe,
I noticed that this code
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 18, 0)
triggers checkpatch's warning:
"WARNING: Comparisons should place the constant on
the right side of the test"
Both LINUX_VERSION_CODE and KERNEL_VERSION are constants, so
I'm wondering if it's worth it to improve that check a tiny
bit.
E.g. something utterly trivial:
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cf7543a9d1b2..8a1964c3d9a5 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -4855,6 +4855,7 @@ sub process {
my $to = $4;
my $newcomp = $comp;
if ($lead !~ /(?:$Operators|\.)\s*$/ &&
+ $to !~ /^KERNEL_VERSION.*/ &&
$to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/ &&
WARN("CONSTANT_COMPARISON",
"Comparisons should place the constant on the right side of the test\n" . $herecurr) &&
---
I'm sure you'll have a better idea.
-ss
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: checkpatch: comparisons with a constant on the left
2019-10-11 1:52 checkpatch: comparisons with a constant on the left Sergey Senozhatsky
@ 2019-10-11 3:23 ` Joe Perches
2019-10-20 7:30 ` Sergey Senozhatsky
0 siblings, 1 reply; 3+ messages in thread
From: Joe Perches @ 2019-10-11 3:23 UTC (permalink / raw)
To: Sergey Senozhatsky; +Cc: Sergey Senozhatsky, linux-kernel
On Fri, 2019-10-11 at 10:52 +0900, Sergey Senozhatsky wrote:
> Hi Joe,
Hi Sergey.
> I noticed that this code
>
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 18, 0)
>
> triggers checkpatch's warning:
>
> "WARNING: Comparisons should place the constant on
> the right side of the test"
>
> Both LINUX_VERSION_CODE and KERNEL_VERSION are constants, so
> I'm wondering if it's worth it to improve that check a tiny
> bit.
Probably not.
My preference is for people to ignore checkpatch
message bleats when they don't make overall sense.
checkpatch thinks anything that uses a form like
"name(<args...>)" is a function.
> I'm sure you'll have a better idea.
I suggest reversing the test if it really bothers you.
# if KERNEL_VERSION(4.18.0) < LINUX_VERSION_CODE
but then again just using LINUX_VERSION_CODE emits a
warning message, so it's better to remove whatever is
in the block anyway... <smile>
cheers, Joe
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: checkpatch: comparisons with a constant on the left
2019-10-11 3:23 ` Joe Perches
@ 2019-10-20 7:30 ` Sergey Senozhatsky
0 siblings, 0 replies; 3+ messages in thread
From: Sergey Senozhatsky @ 2019-10-20 7:30 UTC (permalink / raw)
To: Joe Perches; +Cc: Sergey Senozhatsky, Sergey Senozhatsky, linux-kernel
On (10/10/19 20:23), Joe Perches wrote:
> On Fri, 2019-10-11 at 10:52 +0900, Sergey Senozhatsky wrote:
> > Hi Joe,
>
> Hi Sergey.
Hi Joe,
For some reason your reply triggered gmail spam filter; took
me a while to notice and "recover" it from spam folder.
[..]
> > Both LINUX_VERSION_CODE and KERNEL_VERSION are constants, so
> > I'm wondering if it's worth it to improve that check a tiny
> > bit.
>
> Probably not.
>
> My preference is for people to ignore checkpatch
> message bleats when they don't make overall sense.
>
> checkpatch thinks anything that uses a form like
> "name(<args...>)" is a function.
For example, DMA_BIT_MASK(xxx) can look like a function
call yet still be a compile time constant.
Another example could be ARRAY_SIZE(xxx), I guess.
[..]
> but then again just using LINUX_VERSION_CODE emits a
> warning message, so it's better to remove whatever is
> in the block anyway... <smile>
That's certainly right. LINUX_VERSION_CODE should not be in the
code, I agree. I was thinking more about 'const vs const' comparison
in general, less about particular LINUX_VERSION_CODE case.
-ss
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-20 7:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 1:52 checkpatch: comparisons with a constant on the left Sergey Senozhatsky
2019-10-11 3:23 ` Joe Perches
2019-10-20 7:30 ` Sergey Senozhatsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).