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=khxfL4Nd; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 1D15C5A0262 for ; Sat, 16 May 2026 10:18:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1778919476; bh=VT3XC8Hfi92VL6bESHhkjgw8k8Pu6KANQbO/50zgMM8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=khxfL4NddvqIdpAIi5nA8FpjTHsffMr9TeHKAZHAOYnSGZEX6Kuu4t1T5yffzm7sS 7U4ky8OV13pu1wIE5r9WNhwRDwaPtTO27/skscIha8iR6xLojx3nJprTMEi3x9AXzr lcIcFOGmWCC8Vpgy9ZhaeyQNmAz/JTDCpQjcyJSP/048f/0dia8E6uMiLjYJpU20qq Qb+usfIn6iobWomLGQx2Kz+9tRgzmQGPZgfBlrdr3UYvJ4bA6L4thmQIWSDjs9BSJ1 /EJOOu8UVwpCNcXIlLKTaIhS1wpa9m41exxJOxFZBUDVpbAkWuYaF42rjK/yCJMTkA TfgiaJ2Lh/xUA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gHcPw6pPPz4x2H; Sat, 16 May 2026 18:17:56 +1000 (AEST) Date: Sat, 16 May 2026 16:28:52 +1000 From: David Gibson To: Jan Palus Subject: Re: [PATCH 2/3] Fix build with -DNDEBUG Message-ID: References: <20260515041312.317644-1-david@gibson.dropbear.id.au> <20260515041312.317644-3-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="+BIoDv/mSMzoyJnT" Content-Disposition: inline In-Reply-To: Message-ID-Hash: 7EEWJSO2GG4JJAYUUZBSSFPRX4L2F6EN X-Message-ID-Hash: 7EEWJSO2GG4JJAYUUZBSSFPRX4L2F6EN 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, Stefano Brivio 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: --+BIoDv/mSMzoyJnT Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 15, 2026 at 01:05:50PM +0200, Jan Palus wrote: > On 15.05.2026 14:13, David Gibson wrote: > > Since bc872d91765d, our assert() statements are omitted if we compile w= ith > > -DNDEBUG, like the standard library assert(3). Unfortunately a trivial= but > > embarrassing mistake in that patch means that instead of never aborting= in > > this case, assert_with_msg() *always* aborts, breaking pretty much > > everything. > >=20 > > There's also a missing #include that breaks the build with -DNDEBUG on = at > > least some library versions. > >=20 > > Reported-by: Jan Palus > > Fixes: bc872d91765d ("treewide: Spell ASSERT() as assert()") > > Signed-off-by: David Gibson > > --- > > util.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > >=20 > > diff --git a/util.h b/util.h > > index 70aadeba..11f71d45 100644 > > --- a/util.h > > +++ b/util.h > > @@ -6,6 +6,7 @@ > > #ifndef UTIL_H > > #define UTIL_H > > =20 > > +#include > > #include > > #include > > #include > > @@ -60,7 +61,7 @@ void abort_with_msg(const char *fmt, ...) > > __func__, __FILE__, __LINE__, STRINGIFY(expr)) > > #else > > #define assert_with_msg(expr, ...) \ > > - ((void)(expr), 0 ? (void)0 : abort_with_msg(__VA_ARGS__)) > > + ((void)(expr), 1 ? (void)0 : abort_with_msg(__VA_ARGS__)) >=20 > There is a slight semantic difference between assert() and > assert_with_msg() when building with -DNDEBUG -- `expr` is still being > evaluated in abort_with_msg() although likely optimized out in most > builds. Right, I'm expecting the compiler to optimise this away. That won't be the case if the expression has side effects, but side effects in an assert() expression is already a bug, precisely because of the possibility NDEBUG. > I'm assuming you'd prefer to avoid cppcheck suppressions. cppcheck suppressions would be acceptable if we could put them inside the macro. But these are unused variable warnings, which show up at the site of variable declaration not (obviously) at the place the variable... isn't used. Putting suppressions on variables because they might become unused depending on a macro definition certainly isn't acceptable. Both for aesthetics, and because it means if we removed the assert we'd no longer get a warning that the variable now really isn't used. > How > about moving `expr` into branch which is never evaluated then? Would it > keep cppcheck happy? >=20 > + (1 ? (void)0 : ((void)(expr), abort_with_msg(__VA_ARGS__))) That does work. Not sure if it's worth the bother of a respin. Stefano, any opinion? --=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 --+BIoDv/mSMzoyJnT Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmoIDpYACgkQzQJF27ox 2GcZ4g//c+3a1Aitt+WZY1MxK8ph9XmluevyP7fwqPuU1TMltC9yl1MwdaCRCYBS EoJS1XwwgfiwGBxMY4SW+Khq8PbrYXXuf9n6nN+7fEYXTsVTwPaRuqCfDZlggjEW 1mnjsZhIGcNy2ut5lv2PPjpmlSDs2GUWeHDrKsyb5NB3gmVHXNSKlztr8vCQPUnS 5y5A32D3dEydVTVYqMQuFF8xoxwSWQ5ihJuC/5hDr75JHsnxMCYnjCrp8iGZxQww Ziz0avnBbuBx02nVd/9Ra8Ko8q/Yf/D82aq+IMKVA2zbYLg1OYP+Xbz0szqn3Daj ffCMXB0X2bBAFKvSiU9fHsatjIxeGnfTOGGJXZxmXVWl6ObGuwAHIZEX1/MgoOLz 9TU3nqOo6ohzWeTS1i/vxvx3Qlw93DoDQDB85ZT7Pmbnjql/5ejHTZoRoFfiANBk hHHiN9HxqhqhMobiJ5fkSytK07h4DJrG70lYm6F44wCS2lKWp4LNKc1CpYbluAYj 9UTDUAPmjciiTQY/N2LjkBOFFVl5uexc80usVCdWxo4MsuMNo+R6qohhE9wMPhXc eITu82ZWVQbFrVzFPysQzFnmHx4Iie4PrgviFhd8QUtoGAVecd3QPTDi7rLqoA8W /espySsXYk/HXrkvpVjY3PFnekUeZPtuA34Iyr+lPw6oCcwqyuM= =PPSj -----END PGP SIGNATURE----- --+BIoDv/mSMzoyJnT--