Re: [PATCH 4/6] util: Define small and big thresholds for socket buffers as unsigned long long

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5049 bytes --]

On Thu, Nov 07, 2024 at 07:43:29PM +0100, Stefano Brivio wrote:
> On 32-bit architectures, clang-tidy reports:
> 
> /home/pi/passt/tcp.c:728:11: error: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long long') of a multiplication performed in type 'unsigned long' [bugprone-implicit-widening-of-multiplication-result,-warnings-as-errors]
>   728 |         if (v >= SNDBUF_BIG)
>       |                  ^
> /home/pi/passt/util.h:158:22: note: expanded from macro 'SNDBUF_BIG'
>   158 | #define SNDBUF_BIG              (4UL * 1024 * 1024)
>       |                                  ^
> /home/pi/passt/tcp.c:728:11: note: make conversion explicit to silence this warning
>   728 |         if (v >= SNDBUF_BIG)
>       |                  ^
> /home/pi/passt/util.h:158:22: note: expanded from macro 'SNDBUF_BIG'
>   158 | #define SNDBUF_BIG              (4UL * 1024 * 1024)
>       |                                  ^~~~~~~~~~~~~~~~~
> /home/pi/passt/tcp.c:728:11: note: perform multiplication in a wider type
>   728 |         if (v >= SNDBUF_BIG)
>       |                  ^
> /home/pi/passt/util.h:158:22: note: expanded from macro 'SNDBUF_BIG'
>   158 | #define SNDBUF_BIG              (4UL * 1024 * 1024)
>       |                                  ^~~~~~~~~~
> /home/pi/passt/tcp.c:730:15: error: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long long') of a multiplication performed in type 'unsigned long' [bugprone-implicit-widening-of-multiplication-result,-warnings-as-errors]
>   730 |         else if (v > SNDBUF_SMALL)
>       |                      ^
> /home/pi/passt/util.h:159:24: note: expanded from macro 'SNDBUF_SMALL'
>   159 | #define SNDBUF_SMALL            (128UL * 1024)
>       |                                  ^
> /home/pi/passt/tcp.c:730:15: note: make conversion explicit to silence this warning
>   730 |         else if (v > SNDBUF_SMALL)
>       |                      ^
> /home/pi/passt/util.h:159:24: note: expanded from macro 'SNDBUF_SMALL'
>   159 | #define SNDBUF_SMALL            (128UL * 1024)
>       |                                  ^~~~~~~~~~~~
> /home/pi/passt/tcp.c:730:15: note: perform multiplication in a wider type
>   730 |         else if (v > SNDBUF_SMALL)
>       |                      ^
> /home/pi/passt/util.h:159:24: note: expanded from macro 'SNDBUF_SMALL'
>   159 | #define SNDBUF_SMALL            (128UL * 1024)
>       |                                  ^~~~~
> /home/pi/passt/tcp.c:731:17: error: performing an implicit widening conversion to type 'uint64_t' (aka 'unsigned long long') of a multiplication performed in type 'unsigned long' [bugprone-implicit-widening-of-multiplication-result,-warnings-as-errors]
>   731 |                 v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2;
>       |                               ^
> /home/pi/passt/util.h:159:24: note: expanded from macro 'SNDBUF_SMALL'
>   159 | #define SNDBUF_SMALL            (128UL * 1024)
>       |                                  ^
> /home/pi/passt/tcp.c:731:17: note: make conversion explicit to silence this warning
>   731 |                 v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2;
>       |                               ^
> /home/pi/passt/util.h:159:24: note: expanded from macro 'SNDBUF_SMALL'
>   159 | #define SNDBUF_SMALL            (128UL * 1024)
>       |                                  ^~~~~~~~~~~~
> /home/pi/passt/tcp.c:731:17: note: perform multiplication in a wider type
>   731 |                 v -= v * (v - SNDBUF_SMALL) / (SNDBUF_BIG - SNDBUF_SMALL) / 2;
>       |                               ^
> /home/pi/passt/util.h:159:24: note: expanded from macro 'SNDBUF_SMALL'
>   159 | #define SNDBUF_SMALL            (128UL * 1024)
>       |                                  ^~~~~
> 
> because, wherever we use those thresholds, we define the other term
> of comparison as uint64_t. Define the thresholds as unsigned long long
> as well, to make sure we match types.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  util.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/util.h b/util.h
> index 582ef57..963f57b 100644
> --- a/util.h
> +++ b/util.h
> @@ -158,9 +158,9 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags,
>  			 (void *)(arg));				\
>  	} while (0)
>  
> -#define RCVBUF_BIG		(2UL * 1024 * 1024)
> -#define SNDBUF_BIG		(4UL * 1024 * 1024)
> -#define SNDBUF_SMALL		(128UL * 1024)
> +#define RCVBUF_BIG		(2ULL * 1024 * 1024)
> +#define SNDBUF_BIG		(4ULL * 1024 * 1024)
> +#define SNDBUF_SMALL		(128ULL * 1024)
>  
>  #include <net/if.h>
>  #include <limits.h>

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]