Messages in this thread | | | From | Linus Torvalds <> | Date | Sat, 6 Jan 2024 21:54:05 -0800 | Subject | Re: include/asm-generic/unaligned.h:119:16: sparse: sparse: cast truncates bits from constant value (aa01a0 becomes a0) |
| |
On Sat, 6 Jan 2024 at 16:42, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > This is not really a kernel/driver bug, just sparse being over-eager > with truncation detection. I wonder if we could make sparse skip this > check on forced casts like this:
No, please don't.
Just face the fact that using integer casts to mask bits off is a bad idea.
Yes, we could say "explicit casting is ok", since it's really the hidden implicit casts changing values that sparse complains about, but your solution is really ugly:
> static inline void __put_unaligned_be24(const u32 val, u8 *p) > { > - *p++ = val >> 16; > - *p++ = val >> 8; > - *p++ = val; > + *p++ = (__force u8)(val >> 16); > + *p++ = (__force u8)(val >> 8); > + *p++ = (__force u8)val; > }
That's just disgusting.
The *natural* thing to do is to simply make the masking itself be explicit - not the cast. IOW, just write it as
*p++ = (val >> 16) & 0xff; *p++ = (val >> 8) & 0xff; *p++ = val & 0xff;
and doesn't that look much more natural?
Sure, the compiler will then just notice "you're assigning to a char, to I don't actually need to do any masking at all", but now sparse won't complain because there's no "cast silently drops bits" issue any more.
And while the code is a bit more to read, I think it is actually to some degree more obvious to a human too what is going on.
No?
Linus
| |