Re: [PATCH] tcp: Avoid comparison of expressions with different signedness in tcp_timer_handler()

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

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

On Wed, Mar 04, 2026 at 05:32:03PM +0100, Stefano Brivio wrote:
> With gcc 14.2, building against musl 1.2.5 (slightly outdated Alpine
> on x86_64):
> 
> tcp.c: In function 'tcp_timer_handler':
> util.h:40:39: warning: comparison of integer expressions of different signedness: 'unsigned int' and 'int' [-Wsign-compare]
>    40 | #define MIN(x, y)               (((x) < (y)) ? (x) : (y))
>       |                                       ^
> tcp.c:2593:31: note: in expansion of macro 'MIN'
>  2593 |                         max = MIN(TCP_MAX_RETRIES, max);
>       |                               ^~~
> util.h:40:54: warning: operand of '?:' changes signedness from 'int' to 'unsigned int' due to unsignedness of other operand [-Wsign-compare]
>    40 | #define MIN(x, y)               (((x) < (y)) ? (x) : (y))
>       |                                                      ^~~
> tcp.c:2593:31: note: in expansion of macro 'MIN'
>  2593 |                         max = MIN(TCP_MAX_RETRIES, max);
>       |                               ^~~
> 
> for some reason, that's not reported by gcc with glibc.
> 
> Make the temporary 'max' variable unsigned, as we know it can't be
> negative anyway.
> 
> While at it, add the customary blank line between variable
> declarations and code.
> 
> Fixes: 3dde0e07804e ("tcp: Update data retransmission timeout")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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

> ---
>  tcp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tcp.c b/tcp.c
> index 88ad886..68c92da 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -2588,7 +2588,8 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
>  		tcp_timer_ctl(c, conn);
>  	} else if (conn->flags & ACK_FROM_TAP_DUE) {
>  		if (!(conn->events & ESTABLISHED)) {
> -			int max;
> +			unsigned int max;
> +
>  			max = c->tcp.syn_retries + c->tcp.syn_linear_timeouts;
>  			max = MIN(TCP_MAX_RETRIES, max);
>  			if (conn->retries >= max) {
> -- 
> 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 --]