Re: [PATCH 2/3] Fix build with -DNDEBUG

[Stefano Brivio <sbrivio@redhat.com>]

On Sat, 16 May 2026 16:28:52 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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 with
> > > -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.
> > > 
> > > There's also a missing #include that breaks the build with -DNDEBUG on at
> > > least some library versions.
> > > 
> > > Reported-by: Jan Palus <jpalus@fastmail.com>
> > > Fixes: bc872d91765d ("treewide: Spell ASSERT() as assert()")
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > > ---
> > >  util.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > 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
> > >  
> > > +#include <assert.h>
> > >  #include <stdlib.h>
> > >  #include <stdarg.h>
> > >  #include <stdbool.h>
> > > @@ -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__))  
> > 
> > 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?
> > 
> > +	(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?

I think it would be worth changing that given that the only reason why
PLD Linux enables NDEBUG is to avoid evaluating those expressions.

I can also change it on merge though, it's trivial enough. I'll do that
if everybody is fine with it.

-- 
Stefano