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=PSCQvHUu; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id CCC0C5A0265 for ; Wed, 18 Mar 2026 02:08:42 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1773796118; bh=hngce48DbKRH6mZTMsxVWFkZDrUN96wPgPTPHPaxLTo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=PSCQvHUuS0KylxiIyaIvapc3y1jY+e9xvfx1zJ39Aujtf9MmPs6mbccwMuyJv+BDQ rFU3TimdV3vC5wK9KleVSzITDSVT/eyZ2Gk/iLVHQm8zR810VHBRzIuToSNKTEldzS peWHojzPo539BW3tpB3NVYBR2anBpO1NA7A4R1A6elIG3i6dKFomLhH09MVOVnYGtt lwVfw4lqS+ZBj2sbGyY6PVyWDaf77syD4cAmLRP6fvTlbOGnYXSARZF58JuTljKbFq +U4vmuDj63bjIN9mKMmKbhh90ui5xLN6HLOYBQrJZOoKOpeBbx1h9pUSaIpSfpvrop sT8TA8+gl5Esw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fb9gp6SJ6z4wCJ; Wed, 18 Mar 2026 12:08:38 +1100 (AEDT) Date: Wed, 18 Mar 2026 11:52:51 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 4/5] treewide: Spell ASSERT() as assert() Message-ID: References: <20260316054629.239002-1-david@gibson.dropbear.id.au> <20260316054629.239002-5-david@gibson.dropbear.id.au> <20260317010233.0723ea6d@elisabeth> <20260317103624.7b547b48@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="WLhbjERx408h9bip" Content-Disposition: inline In-Reply-To: <20260317103624.7b547b48@elisabeth> Message-ID-Hash: ZOPKYRBX3UCPS23QHSY6QV6DFK6JAVJU X-Message-ID-Hash: ZOPKYRBX3UCPS23QHSY6QV6DFK6JAVJU 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: --WLhbjERx408h9bip Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 17, 2026 at 10:36:25AM +0100, Stefano Brivio wrote: > On Tue, 17 Mar 2026 11:39:42 +1100 > David Gibson wrote: >=20 > > On Tue, Mar 17, 2026 at 01:02:34AM +0100, Stefano Brivio wrote: > > > On Mon, 16 Mar 2026 16:46:28 +1100 > > > David Gibson wrote: > > > =20 > > > > +++ b/util.h > > > > @@ -73,10 +73,14 @@ void abort_with_msg(const char *fmt, ...) > > > > * Therefore, avoid using the usual do while wrapper we use to for= ce the macro > > > > * to act like a single statement requiring a ';'. > > > > */ > > > > -#define ASSERT_WITH_MSG(expr, ...) \ > > > > +#define assert_with_msg(expr, ...) \ > > > > ((expr) ? (void)0 : abort_with_msg(__VA_ARGS__)) > > > > -#define ASSERT(expr) \ > > > > - ASSERT_WITH_MSG((expr), "ASSERTION FAILED in %s (%s:%d): %s", \ > > > > +/* The standard library assert() hits our seccomp filter and dies = before it can > > > > + * actually print a message. So, replace it with our own version. > > > > + */ > > > > +#undef assert > > > > +#define assert(expr) \ > > > > + assert_with_msg((expr), "ASSERTION FAILED in %s (%s:%d): %s", \ > > > > __func__, __FILE__, __LINE__, STRINGIFY(expr)) =20 > > >=20 > > > While looking this up to make sure it's specified as a macro (it is, > > > and this builds against musl as well), I realised that POSIX.1-2024 > > > says: > > >=20 > > > https://pubs.opengroup.org/onlinepubs/9799919799/functions/assert.h= tml > > >=20 > > > Forcing a definition of the name NDEBUG, either from the compiler > > > command line or with the preprocessor control statement #define NDE= BUG > > > ahead of the #include statement, shall stop assertions f= rom > > > being compiled into the program. > > >=20 > > > ...so, I wonder, now that it's called assert(), should we define it as > > > "do { } while(0)" #ifdef NDEBUG, for correctness (and maybe somebody > > > has obscure usages for NDEBUG which we shouldn't sabotage)? =20 > >=20 > > I like the idea in principle. Actually implementing it turns out to > > be kind of a pain in the arse, because if we actually try to compile > > with -DNDEBUG then we get a much of warnings due to reaching the end > > of functions (assert(0) stopped us otherwise) or unused variables > > (they're only used in the assert expression or message). > >=20 > > A project for some other time, I think. >=20 > Well but it's just (probably harmless) warnings right? I tried with > this on top of your patch: >=20 > --- > diff --git a/util.h b/util.h > index dcb79af..77b59bc 100644 > --- a/util.h > +++ b/util.h > @@ -75,13 +75,18 @@ void abort_with_msg(const char *fmt, ...) > */ > #define assert_with_msg(expr, ...) \ > ((expr) ? (void)0 : abort_with_msg(__VA_ARGS__)) > + > /* The standard library assert() hits our seccomp filter and dies before= it can > * actually print a message. So, replace it with our own version. > */ > #undef assert > +#ifdef NDEBUG > +#define assert(expr) do { } while(0) In fact we don't need to explicitly do this. In the NDEBUG case, assert() is already a no-op, so we don't need to redefine it. > +#else > #define assert(expr) \ > assert_with_msg((expr), "ASSERTION FAILED in %s (%s:%d): %s", \ > __func__, __FILE__, __LINE__, STRINGIFY(expr)) =2E.. and principle of least surprise suggests to me that assert_with_msg() should also become a no-op with -DNDEBUG. > +#endif > =20 > #ifdef P_tmpdir > #define TMPDIR P_tmpdir > --- >=20 > and 'CFLAGS=3D"-DNDEBUG" make' gives me some stuff such as, for example: >=20 > util.c: In function =E2=80=98sock_l4_=E2=80=99: > util.c:90:14: warning: =E2=80=98proto=E2=80=99 may be used uninitialized = [-Wmaybe-uninitialized] > 90 | fd =3D socket(af, socktype, proto); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >=20 > because an invalid value for it isn't caught by assert(0) anymore, but > from a quick review I'd say it's all intended and implied because of the > missing assert() calls. >=20 > Sure, the output with NDEBUG is not pretty, but we won't be able to "fix" > most of that anyway. If somebody passes it as extra CFLAGS, I would expect > they know what they're doing. Nobody will build distribution packages or > anything "official" with it, I think. Good point. I was thinking that compiling clean with -DNDEBUG was a goal, but it really isn't. > Either way, should I go ahead and merge this (in the original version or > amended for NDEBUG, I'm fine with both)? Not quite - I'll put a version with the tweaks described above applied in the next spin. --=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 --WLhbjERx408h9bip Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmm591IACgkQzQJF27ox 2GewQA/8CllHAssOCoiIIU0eYcDKBPk0eAgugCgkwXYdPucN9ZlHieyQgAhEdw6A b9RRhUNThkQcerhhnMPuJOxQv7Xi9g5OvwT9amfI2NvNKQYOiXBvUnBxhemkNW7E ij7sBi23ZcfRXA4OKMqJsBWSngi4j5nKesp1rGQG3x1P7keQktj/EYsoJh7pcr6E k/nMaZk93uEOXvTlusNvYrLxyvIl/ATffNaGX2wz4BhkDMai/hIvHqpz4yUff9xH MXsXD5J4QoBTLJMUGvLLzroO4dk5CXV281mKLAOyvTG43v3qJOZMJNb/h59E3Rzg vrpuo9fLq75FvLGSlrMNlQEKtR1hpoMPtXopYGYv23GX+Ns1EEtF0iXAff7pSUjY Vfr9/sx5dBixQMlK2yQHpWC8TDfTWz8/0kAzHezMp1lBTymALqFpbFe9ClzcfcZ5 EsM8wON6AqWPaklQS5lN0ByNLz03F1czI5RbtxkHG7kGuPWY00+IzbQhjOWO4ci1 JxI5eeDc9+yrfgeu5NhGk3iutaPneGagsKW1UW0UHBVJPEKGeg4t7wN+3OgeFEnG jtP0emjwReW4uo3KsjaOMsZPyjmWilDt38CiWmy1zUgM1R1OkaLDU1Iw63dKGIMU 9xMgdsf1jab3CFK6glri3RvkS2icJI6E4kkaGTLGLN3kWGp44Zc= =Aumf -----END PGP SIGNATURE----- --WLhbjERx408h9bip--