Re: [PATCH v4 6/8] tcp: Cast operands of sequence comparison macros to uint32_t before using them

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

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

On Tue, Sep 09, 2025 at 08:16:53PM +0200, Stefano Brivio wrote:
> Otherwise, passing signed types causes automatic promotion of the
> result of the subtractions as well, which is not what we want, as
> these macros rely on unsigned 32-bit arithmetic.
> 
> The next patch introduces a ssize_t operand for SEQ_LE, illustrating
> the issue.
> 
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

C macros and promotion rules strike again :/.  Personally I'd probably
switch to inline functions, but this is fine too.

> ---
>  tcp_internal.h | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tcp_internal.h b/tcp_internal.h
> index d0009f8..ce6fee2 100644
> --- a/tcp_internal.h
> +++ b/tcp_internal.h
> @@ -21,10 +21,14 @@
>  						   sizeof(struct ipv6hdr), \
>  						   sizeof(uint32_t))
>  
> -#define SEQ_LE(a, b)			((b) - (a) < MAX_WINDOW)
> -#define SEQ_LT(a, b)			((b) - (a) - 1 < MAX_WINDOW)
> -#define SEQ_GE(a, b)			((a) - (b) < MAX_WINDOW)
> -#define SEQ_GT(a, b)			((a) - (b) - 1 < MAX_WINDOW)
> +#define SEQ_LE(a, b)			\
> +	((uint32_t)(b) - (uint32_t)(a) < MAX_WINDOW)
> +#define SEQ_LT(a, b)			\
> +	((uint32_t)(b) - (uint32_t)(a) - 1 < MAX_WINDOW)
> +#define SEQ_GE(a, b)			\
> +	((uint32_t)(a) - (uint32_t)(b) < MAX_WINDOW)
> +#define SEQ_GT(a, b)			\
> +	((uint32_t)(a) - (uint32_t)(b) - 1 < MAX_WINDOW)
>  
>  #define FIN		(1 << 0)
>  #define SYN		(1 << 1)
> -- 
> 2.43.0
> 

-- 
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 --]