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.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 include/linux/bitmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 595217b7a6e7..4395e0a618f4 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -442,7 +442,7 @@ static __always_inline
 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);
 }
 
-- 
2.49.0.504.g3bcea36a83-goog

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'.

      Arnd

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