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

Ian Rogers
2025-04-04T11:32:06Z

On Thu, Apr 3, 2025 at 10:49 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Thu, Apr 3, 2025, at 18:56, 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.
>
>
> > unsigned int bitmap_weight(const unsigned long *src, unsigned int nbits)
> > {
> > if (small_const_nbits(nbits))
> > - return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> > + return (int)hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
> > return __bitmap_weight(src, nbits);
> > }
>
> I don't understand this one. hweight_long() and bitmap_weight()
> both return unsigned value, so why do you need to cast this to
> a signed value to avoid a signedness problem?
>
> hweight_long() should never return anything larger than 64ul
> anyway, which is way outside of the range where it would get
> sign-extended.
>
> A more logical change to me would be to make hweight_long()
> and bitmap_weight() have the same return type, either
> 'unsigned long' or 'unsigned int'.

I don't disagree but the scope of that change would be much larger.
Yury has expressed concern over needing to update printf modifiers. I
was aiming for the minimal change that silences clang's
-Wshorten-64-to-32 warning.

Thanks,
Ian

> Arnd