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=QzlwRhsr; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id ADB4C5A0265 for ; Thu, 05 Mar 2026 03:32:17 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1772677934; bh=Dm7vVNGInpFCV1rZUiGZEkULYw7I7SM6JbWuRUZ2Ze0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=QzlwRhsrjxx9N115Q9jcHqs70bmxyHOITHX5smD4UPdXlz+YxXD6DcRldQ3ZES/3y dJiAtQHFQV6oK7qC7JGOO6L/4I01vWJzEN8YuBx5N6e1B7VBnxHH2uema4V7ulnxta 6P6gw5L9m6Jgt3mUuZfkc51m7PiXC3u9CrlhpnS13KBqXBOeIBE1ZT9t864BsCzy5m 2t1RukJ6+BI60s27Ogj2oDzTe21UOSbjhHWYMeLOaVWl3idP3Ep8BusCPPggQ/y+Ol LqIn0HAybrkNzStrigwDRdBYJGyRlaXt4hhwVmcTXoAbs+TyvgASLqBRgYdsTzpmdC xHBaaPSwGPL3A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fRD8G48Gkz4wDN; Thu, 05 Mar 2026 13:32:14 +1100 (AEDT) Date: Thu, 5 Mar 2026 13:32:08 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] tcp: Avoid comparison of expressions with different signedness in RTT_SET() Message-ID: References: <20260304163232.2440892-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="pi+Q53S7ROvmwAC9" Content-Disposition: inline In-Reply-To: <20260304163232.2440892-1-sbrivio@redhat.com> Message-ID-Hash: 7J2TLNCIWCB3D7W3AUMX2AGSZSRJMFX5 X-Message-ID-Hash: 7J2TLNCIWCB3D7W3AUMX2AGSZSRJMFX5 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: --pi+Q53S7ROvmwAC9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 04, 2026 at 05:32:32PM +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, as it's always > positive the way we're using it. Should this ever break this for > whatever unlikely reason, RTT_EXP_MAX is the fallback value we want to > use anyway. >=20 > Fixes: 000601ba86da ("tcp: Adaptive interval based on RTT for socket-side= acknowledgement checks") > Signed-off-by: Stefano Brivio This is correct, so Reviewed-by: David Gibson It's kind of inelegant, though - ilog2() only returns a signed to report an error case, which we avoid with the MAX inside the argument. That's not a super obvious connection. I guess we could do: (unsigned)MAX(0, ilog2(rtt / RTT_STORE_MIN)) which should be equivalent and makes it slightly more obvious that the cast is safe. Not sure if it's really an improvement, though. This is the only user of ilog2(), so we could also consider replacing it with a version that explicitly clamps its argument to >=3D 1 and returns unsigned. --=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 --pi+Q53S7ROvmwAC9 Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmo6ygACgkQzQJF27ox 2Gem2Q/+NRoD3eZ7ZL2zXQkbvGDBwyse1PaJCVVXUb8XcGHUlDfJBr3akbvwobUV sPyq2CBFjBhVKWxJ3sgFpPL/M5hK3ZPofAGCZDK/cwPjuZ4s+G/Kkfky3Zk0C889 GSnSi6NRBpJRKqLAaAt4L7BQknxiGzQ2uyuixd+AJWXp7mnMzZ9gaAylRvLjNXlW xwBLts/nTcgEr8F5OQ65FdrEGJCcmSm++Xcb+aUoOH1EH/72dciwdl9WyVo5OFci tfb6Q8to1O5e6+7VqxbCmL8o83yt0d+1mHmNcUmneG+ipwJnpBCkGIcCyoR3eGRT J2hfjWeHcpEXZVIS4a5dHetaGQ9HIiudb2qg/Dolxz9zEoIpxH7C4a/5D7ZMqsXo 6QooCOspC/QxBcb8qRKrNDimUOBiZ6dfoYGRkZj2ldPgEnwDWRuIqHjgNdIC6zUz LCy5M9krSteAcmfUeg4oe5P8C4PYF1ea/M6bQCkdw3vycsvMIQkl461rTV6Moq2V Vtlb21DIA2fp4bMwFchBoAdZvL1MacF+MnYitDUp6U118Vsjf3Q2jIGvwkq+3hTx EMU/lMP3gRPtF4DRcp1k3ocqma+6TZO1MLbhiIMPy7oHpHCmwB+JemVf83R35WU5 J1nbl4Y7YfY2FxPmVwAgKtaNzQFZFGb0oFo3ptDA4sud/Zs2mqQ= =PbGa -----END PGP SIGNATURE----- --pi+Q53S7ROvmwAC9--