Use cpu_to_le32 to convert the constants to __le32 type
before comparing them with p->des0 and p->des1 (they are __le32 type)
and to fix following sparse warnings:
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:110:23: sparse: warning: restricted __le32 degrades to integer
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:110:50: sparse: warning: restricted __le32 degrades to integer
Reviewed-by: Simon Horman <[email protected]>
Signed-off-by: Min-Hua Chen <[email protected]>
---
Change since v1:
use cpu_to_le32 to the constants
Change since v2:
remove unnecessary parentheses
---
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
index 13c347ee8be9..ffe4a41ffcde 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c
@@ -107,7 +107,8 @@ static int dwxgmac2_rx_check_timestamp(void *desc)
ts_valid = !(rdes3 & XGMAC_RDES3_TSD) && (rdes3 & XGMAC_RDES3_TSA);
if (likely(desc_valid && ts_valid)) {
- if ((p->des0 == 0xffffffff) && (p->des1 == 0xffffffff))
+ if (p->des0 == cpu_to_le32(0xffffffff) &&
+ p->des1 == cpu_to_le32(0xffffffff))
return -EINVAL;
return 0;
}
--
2.34.1
On Fri, May 19, 2023 at 07:50:28PM +0800, Min-Hua Chen wrote:
> [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Use cpu_to_le32 to convert the constants to __le32 type
> before comparing them with p->des0 and p->des1 (they are __le32 type)
> and to fix following sparse warnings:
>
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:110:23: sparse: warning: restricted __le32 degrades to integer
> drivers/net/ethernet/stmicro/stmmac/dwxgmac2_descs.c:110:50: sparse: warning: restricted __le32 degrades to integer
>
> Reviewed-by: Simon Horman <[email protected]>
> Signed-off-by: Min-Hua Chen <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
On Fri, 19 May 2023 19:50:28 +0800 Min-Hua Chen wrote:
> - if ((p->des0 == 0xffffffff) && (p->des1 == 0xffffffff))
> + if (p->des0 == cpu_to_le32(0xffffffff) &&
> + p->des1 == cpu_to_le32(0xffffffff))
Can you try to fix the sparse tool instead? I believe it already
ignores such errors for the constant of 0, maybe it can be taught
to ignore all "isomorphic" values?
By "isomorphic" I mean that 0xffffffff == cpu_to_le32(0xffffffff)
so there's no point complaining.
--
pw-bot: reject
Hi Jakub,
>On Fri, 19 May 2023 19:50:28 +0800 Min-Hua Chen wrote:
>> - if ((p->des0 == 0xffffffff) && (p->des1 == 0xffffffff))
>> + if (p->des0 == cpu_to_le32(0xffffffff) &&
>> + p->des1 == cpu_to_le32(0xffffffff))
>
>Can you try to fix the sparse tool instead? I believe it already
>ignores such errors for the constant of 0, maybe it can be taught
>to ignore all "isomorphic" values?
>
I downloaded the source code of sparse and I'm afraid that I cannot make
0xFFFFFFFF ignored easily. I've tried ~0 instead of 0xFFFFFF,
but it did not work with current sparse.
0 is a special case mentioned in [1].
"""
One small note: the constant integer “0” is special.
You can use a constant zero as a bitwise integer type without
sparse ever complaining. This is because “bitwise” (as the name
implies) was designed for making sure that bitwise types don’t
get mixed up (little-endian vs big-endian vs cpu-endian vs whatever),
and there the constant “0” really _is_ special.
"""
For 0xFFFFFFFF, it may look like a false alarm, but we can silence the
sparse warning by taking a fix like mine and people can keep working on
other sparse warnings easier.
(There are around 7000 sparse warning in ARCH=arm64 defconfig build and
sometimes it is hard to remember all the false alarm cases)
Could you consider taking this patch, please?
>
>By "isomorphic" I mean that 0xffffffff == cpu_to_le32(0xffffffff)
>so there's no point complaining.
thanks,
Min-Hua
[1] https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html
On Sat, 20 May 2023 09:55:27 +0800 Min-Hua Chen wrote:
> >Can you try to fix the sparse tool instead? I believe it already
> >ignores such errors for the constant of 0, maybe it can be taught
> >to ignore all "isomorphic" values?
> >
>
> I downloaded the source code of sparse and I'm afraid that I cannot make
> 0xFFFFFFFF ignored easily. I've tried ~0 instead of 0xFFFFFF,
> but it did not work with current sparse.
>
> 0 is a special case mentioned in [1].
>
> """
> One small note: the constant integer “0” is special.
> You can use a constant zero as a bitwise integer type without
> sparse ever complaining. This is because “bitwise” (as the name
> implies) was designed for making sure that bitwise types don’t
> get mixed up (little-endian vs big-endian vs cpu-endian vs whatever),
> and there the constant “0” really _is_ special.
> """
>
> For 0xFFFFFFFF, it may look like a false alarm, but we can silence the
> sparse warning by taking a fix like mine and people can keep working on
> other sparse warnings easier.
We can make working with sparse easier by making sure it doesn't
generate false positive warnings :\
> (There are around 7000 sparse warning in ARCH=arm64 defconfig build and
> sometimes it is hard to remember all the false alarm cases)
>
> Could you consider taking this patch, please?
No. We don't take patches to address false positive static
checker warnings.
Hi Jakub,
>We can make working with sparse easier by making sure it doesn't
>generate false positive warnings :\
It will be good if sparse can handle this case correctly.
>
>> (There are around 7000 sparse warning in ARCH=arm64 defconfig build and
>> sometimes it is hard to remember all the false alarm cases)
>>
>> Could you consider taking this patch, please?
>
>No. We don't take patches to address false positive static
>checker warnings.
No problem.
thanks,
Min-Hua
On 20/05/2023 02:55, Min-Hua Chen wrote:
>> On Fri, 19 May 2023 19:50:28 +0800 Min-Hua Chen wrote:
>>> - if ((p->des0 == 0xffffffff) && (p->des1 == 0xffffffff))
>>> + if (p->des0 == cpu_to_le32(0xffffffff) &&
>>> + p->des1 == cpu_to_le32(0xffffffff))
>>
>> Can you try to fix the sparse tool instead? I believe it already
>> ignores such errors for the constant of 0, maybe it can be taught
>> to ignore all "isomorphic" values?
>>
>
> I downloaded the source code of sparse and I'm afraid that I cannot make
> 0xFFFFFFFF ignored easily. I've tried ~0 instead of 0xFFFFFF,
> but it did not work with current sparse.
>
> 0 is a special case mentioned in [1].
I believe you can do something like
if ((p->des0 == ~(__le32)0) && (p->des1 == ~(__le32)0))
and sparse will accept that, because the cast is allowed under the
special case.
HTH,
-ed
hi Edward,
>>> On Fri, 19 May 2023 19:50:28 +0800 Min-Hua Chen wrote:
>>>> - if ((p->des0 == 0xffffffff) && (p->des1 == 0xffffffff))
>>>> + if (p->des0 == cpu_to_le32(0xffffffff) &&
>>>> + p->des1 == cpu_to_le32(0xffffffff))
>>>
>>> Can you try to fix the sparse tool instead? I believe it already
>>> ignores such errors for the constant of 0, maybe it can be taught
>>> to ignore all "isomorphic" values?
>>>
>>
>> I downloaded the source code of sparse and I'm afraid that I cannot make
>> 0xFFFFFFFF ignored easily. I've tried ~0 instead of 0xFFFFFF,
>> but it did not work with current sparse.
>>
>> 0 is a special case mentioned in [1].
>
>I believe you can do something like
> if ((p->des0 == ~(__le32)0) && (p->des1 == ~(__le32)0))
> and sparse will accept that, because the cast is allowed under the
> special case.
>HTH,
>-ed
I tested ~(__le32)0 and it worked: sparse accpets this.
Thanks for sharing this.
cheers,
Min-Hua