From: Willy Tarreau <w@1wt.eu>
Date: Tue, 17 Dec 2024 13:54:20 +0000 (+0100)
Subject: CLEANUP: quic: replace ALREADY_CHECKED() with ASSUME_NONNULL() at a few places
X-Git-Tag: v3.2-dev2~50
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7760e3a374d9eb1a4adae8209c07517252b6a4c4;p=thirdparty%2Fhaproxy.git

CLEANUP: quic: replace ALREADY_CHECKED() with ASSUME_NONNULL() at a few places

There were 4 instances of ALREADY_CHECKED() used to tell the compiler that
the argument couldn't be NULL by design. Let's change them to the cleaner
ASSUME_NONNULL(). Functions like qc_snd_buf() were slightly reduced in
size (-24 bytes).

Apparently gcc-13 sees a potential case that others don't see, and it's
likely a bug since depending what is masked, it will completely change
the output warnings to the point of contradicting itself. After many
attempts, it appears that just checking that CMSG_FIRSTHDR(msg) is not
null suffices to calm it down, so the strange warnings might have been
the result of an overoptimization based on a supposed UB in the first
place. At least now all versions up to 13.2 as well as clang are happy.
---

diff --git a/src/quic_loss.c b/src/quic_loss.c
index affa6f2064..1c32d92c4a 100644
--- a/src/quic_loss.c
+++ b/src/quic_loss.c
@@ -317,7 +317,7 @@ int qc_release_lost_pkts(struct quic_conn *qc, struct quic_pktns *pktns,
 	 * that <pkts> list is not empty. Without this, GCC 12.2.0 reports a
 	 * possible overflow on a 0 byte region with O2 optimization.
 	 */
-	ALREADY_CHECKED(oldest_lost);
+	ASSUME_NONNULL(oldest_lost);
 	quic_tx_packet_refdec(oldest_lost);
 	if (newest_lost != oldest_lost)
 		quic_tx_packet_refdec(newest_lost);
diff --git a/src/quic_sock.c b/src/quic_sock.c
index 327deb492f..b7ee8f8f48 100644
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -613,8 +613,11 @@ static void cmsg_set_saddr(struct msghdr *msg, struct cmsghdr **cmsg,
 	/* Set first msg_controllen to be able to use CMSG_* macros. */
 	msg->msg_controllen += CMSG_SPACE(sz);
 
+	/* seems necessary to please gcc-13 */
+	ASSUME_NONNULL(CMSG_FIRSTHDR(msg));
+
 	*cmsg = !(*cmsg) ? CMSG_FIRSTHDR(msg) : CMSG_NXTHDR(msg, *cmsg);
-	ALREADY_CHECKED(*cmsg);
+	ASSUME_NONNULL(*cmsg);
 	c = *cmsg;
 	c->cmsg_len = CMSG_LEN(sz);
 
@@ -666,8 +669,11 @@ static void cmsg_set_gso(struct msghdr *msg, struct cmsghdr **cmsg,
 	/* Set first msg_controllen to be able to use CMSG_* macros. */
 	msg->msg_controllen += CMSG_SPACE(sz);
 
+	/* seems necessary to please gcc-13 */
+	ASSUME_NONNULL(CMSG_FIRSTHDR(msg));
+
 	*cmsg = !(*cmsg) ? CMSG_FIRSTHDR(msg) : CMSG_NXTHDR(msg, *cmsg);
-	ALREADY_CHECKED(*cmsg);
+	ASSUME_NONNULL(*cmsg);
 	c = *cmsg;
 	c->cmsg_len = CMSG_LEN(sz);
 
@@ -884,7 +890,7 @@ int qc_rcv_buf(struct quic_conn *qc)
 			TRACE_STATE("datagram for other connection on quic-conn socket, requeue it", QUIC_EV_CONN_RCV, qc);
 
 			rxbuf = MT_LIST_POP(&l->rx.rxbuf_list, typeof(rxbuf), rxbuf_el);
-			ALREADY_CHECKED(rxbuf);
+			ASSUME_NONNULL(rxbuf);
 			cspace = b_contig_space(&rxbuf->buf);
 
 			tmp_dgram = quic_rxbuf_purge_dgrams(rxbuf);