Re: [PATCH 4/5] treewide: Spell ASSERT() as assert()

On Tue, 17 Mar 2026 11:39:42 +1100 David Gibson wrote:

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:

...

+++ 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 force 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))

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:

https://pubs.opengroup.org/onlinepubs/9799919799/functions/assert.html

Forcing a definition of the name NDEBUG, either from the compiler command line or with the preprocessor control statement #define NDEBUG ahead of the #include statement, shall stop assertions from being compiled into the program.

...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)?

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).

A project for some other time, I think.

Well but it's just (probably harmless) warnings right? I tried with this on top of your patch: --- 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) +#else #define assert(expr) \ assert_with_msg((expr), "ASSERTION FAILED in %s (%s:%d): %s", \ __func__, __FILE__, __LINE__, STRINGIFY(expr)) +#endif #ifdef P_tmpdir #define TMPDIR P_tmpdir --- and 'CFLAGS="-DNDEBUG" make' gives me some stuff such as, for example: util.c: In function ‘sock_l4_’: util.c:90:14: warning: ‘proto’ may be used uninitialized [-Wmaybe-uninitialized] 90 | fd = socket(af, socktype, proto); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ 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. 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. Either way, should I go ahead and merge this (in the original version or amended for NDEBUG, I'm fine with both)? -- Stefano