From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=hiR5PZya; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 6FB755A0269 for ; Fri, 06 Mar 2026 13:08:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1772798882; bh=G6gvuiyKOKMAf/pHgKXRardLMpiQYWIHzIGijhEP+jw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=hiR5PZyaW60ifNdZOJUf1kZ0oewDSvj09Zqd0RKE6eAi+tW0g6XCY05oLmK1tmBQF 22lLyYPM3kH1DJvz5yoW5IY5tOEUjoEZrtbLkbBAyZLXSJm/lbj6S+OAs+zinBIRmR nhqcS91zfcyEVTMTv2Xt40QKkJ0SgSJASnepX/Di33tsgAhwaPxLaUeCXRbOtG4JuY Vg9nF3A86xyn3eaKjiLkP5dUA7tC+V1O7EaTA/bXE28DMlS3HQE5WFGK0DwgCpzqxo rewDBnrFoWeSeV2tfKYq91yubuY0IxfrKO+rpdyzUuyHipLBnvNXeYNMcuqduCm6XH AKtfL9aNXMKsw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fS4tB2JRkz4wCp; Fri, 06 Mar 2026 23:08:02 +1100 (AEDT) Date: Fri, 6 Mar 2026 23:02:50 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2] tcp: Avoid comparison of expressions with different signedness in RTT_SET() Message-ID: References: <20260306070235.2687041-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="zX7wE3rXBNCMPQK4" Content-Disposition: inline In-Reply-To: <20260306070235.2687041-1-sbrivio@redhat.com> Message-ID-Hash: MZLIMF65OXIWQ25WJCDV2Y7S4ZQIZE25 X-Message-ID-Hash: MZLIMF65OXIWQ25WJCDV2Y7S4ZQIZE25 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --zX7wE3rXBNCMPQK4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 06, 2026 at 08:02:35AM +0100, Stefano Brivio wrote: > With gcc 14.2, building against musl 1.2.5 (slightly outdated Alpine > on x86_64): >=20 > tcp.c: In function 'tcp_update_seqack_wnd': > util.h:40:39: warning: comparison of integer expressions of different sig= nedness: 'unsigned int' and 'int' [-Wsign-compare] > 40 | #define MIN(x, y) (((x) < (y)) ? (x) : (y)) > | ^ > tcp_conn.h:63:26: note: in expansion of macro 'MIN' > 63 | (conn->rtt_exp =3D MIN(RTT_EXP_MAX, ilog2(MAX(1, rtt / RT= T_STORE_MIN)))) > | ^~~ > tcp.c:1234:17: note: in expansion of macro 'RTT_SET' > 1234 | RTT_SET(conn, tinfo->tcpi_rtt); > | ^~~~~~~ > 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_conn.h:63:26: note: in expansion of macro 'MIN' > 63 | (conn->rtt_exp =3D MIN(RTT_EXP_MAX, ilog2(MAX(1, rtt / RT= T_STORE_MIN)))) > | ^~~ > tcp.c:1234:17: note: in expansion of macro 'RTT_SET' > 1234 | RTT_SET(conn, tinfo->tcpi_rtt); > | ^~~~~~~ >=20 > for some reason, that's not reported by gcc with glibc. >=20 > Cast the result of ilog2() to unsigned before using it, and introduce > 0 as lower bound, to make it obvious that we expect the argument to be > always valid, the way we're using it. >=20 > Suggested-by: David Gibson > Fixes: 000601ba86da ("tcp: Adaptive interval based on RTT for socket-side= acknowledgement checks") > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > v2: Apply lower bound to the result of ilog2() instead of its argument, > as suggested by David >=20 > tcp_conn.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) >=20 > diff --git a/tcp_conn.h b/tcp_conn.h > index b7b85c1..6985426 100644 > --- a/tcp_conn.h > +++ b/tcp_conn.h > @@ -60,7 +60,8 @@ struct tcp_tap_conn { > #define RTT_STORE_MIN 100 /* us, minimum representable */ > #define RTT_STORE_MAX ((long)(RTT_STORE_MIN << RTT_EXP_MAX)) > #define RTT_SET(conn, rtt) \ > - (conn->rtt_exp =3D MIN(RTT_EXP_MAX, ilog2(MAX(1, rtt / RTT_STORE_MIN)))) > + (conn->rtt_exp =3D MIN(RTT_EXP_MAX, \ > + (unsigned)MAX(0, ilog2(rtt / RTT_STORE_MIN)))) > #define RTT_GET(conn) (RTT_STORE_MIN << conn->rtt_exp) > =20 > bool tap_inactive :1; > --=20 > 2.43.0 >=20 --=20 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 --zX7wE3rXBNCMPQK4 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmqwlQACgkQzQJF27ox 2GdiOg/6AmkGQRXhJo6zsOqDKmzxo0MUXWDnjP0kgpvI+oXkYZ1YS0PJc3gBD8ZO MvZf5GdLCcUqDp7dtJo2XcIKySnD046dM6gv/BFyS71E/n9yr3HUzHNarfRMuumG vQdTftUBvjmtoJYOt83gLskqdRpuGTpHXjlqWB2CL4R17rsiuanCCmuqZm1s0Uet ekokkB3e+HyVuEd/hylnrrn8C6K5Vz+OSKRND5PFfZLMGT4KauZGVHnslmCZ6MXn oqVC/1ViNo5AX6K3p4FvJKnu/O0tYINnh5PkcR8qxEWMlem2/nzvVxsdeZGkXzi9 mnVNtqe6tDKy6YgGinPllq5SSyBVNSokgCsJZ8pkEZoSXTfl7RtAJmMvSq3PROo7 ZPQrg+4g25izRSlEJ8OTfm3y7u0+duMVMPmsIFcWvRBfM3q8nwwOhCfjoGZSZD4y UakaAc7YjUOjJqjx3fE/F7rSlG/+FM+9iPBcI7teov1FkZQv7FXbxMpTHw8QMojP zOUX12ta3+P20WDc8gEG57qrx5YoREdZwQrqbvE4wHoNPIHksAmiIhsHm5h43XhE fTvRh11wBxK4OFgyrtpxmObAeOkVpOSCMu8TcJB159iegWykT52DV6SwMJCOma3O LZR9ZJd8i+eJY0sGW0LydSRCzfbnCg59xRpufmXJ62uWV2eG90A= =niLW -----END PGP SIGNATURE----- --zX7wE3rXBNCMPQK4--