Re: [PATCH v1 3/5] bitops: Silence a clang -Wshorten-64-to-32 warning

[Ian Rogers]

On Thu, Apr 3, 2025 at 10:43 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Thu, Apr 3, 2025, at 18:57, Ian Rogers wrote:
> > The clang warning -Wshorten-64-to-32 can be useful to catch
> > inadvertent truncation. In some instances this truncation can lead to
> > changing the sign of a result, for example, truncation to return an
> > int to fit a sort routine. Silence the warning by making the implicit
> > truncation explicit.
>
> The fls64() function only seems to deal with unsigned values, so
> I don't see how it would change the sign.

You are right. I was trying to motivate in the message why building
with -Wshorten-64-to-32 is a good thing, and in this case we're making
an implicit cast explicit.

> > diff --git a/include/asm-generic/bitops/fls64.h
> > b/include/asm-generic/bitops/fls64.h
> > index 866f2b2304ff..9ad3ff12f454 100644
> > --- a/include/asm-generic/bitops/fls64.h
> > +++ b/include/asm-generic/bitops/fls64.h
> > @@ -21,7 +21,7 @@ static __always_inline int fls64(__u64 x)
> >       __u32 h = x >> 32;
> >       if (h)
> >               return fls(h) + 32;
> > -     return fls(x);
> > +     return fls((__u32)x);
> >  }
>
> Maybe this would be clearer with an explicit upper_32_bits()/
> lower_32_bits() instead of the cast and the shift?

It feels a little overkill to me, but if others prefer it then it is a
minor change.

Thanks,
Ian

>       Arnd