[PATCH 0/3] Fix broken build with -DNDEBUG

newer
[PATCH v2] util: Fix changes to...

David Gibson

15 May 2026 15 May '26

6:13 a.m.

Fix the trivial but nasty with -DNDEBUG builds recently reported by Jan Palus. While we're there, add a couple of extra build tests. These wouldn't actually catch this problem, but they're better than nothing and do catch the #include problem I also spotted. David Gibson (3): test: Extend exeter build tests to cover more recent binaries Fix build with -DNDEBUG test: Add test for builds with -DNDEBUG test/build/build.py | 25 +++++++++++++++++++++---- util.h | 3 ++- 2 files changed, 23 insertions(+), 5 deletions(-) -- 2.54.0

Show replies by date

David Gibson

15 May 15 May

6:13 a.m.

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

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 Fixes: bc872d91765d ("treewide: Spell ASSERT() as assert()") Signed-off-by: David Gibson --- 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 #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__)) #endif #ifdef P_tmpdir -- 2.54.0

David Gibson

6:13 a.m.

New subject: [PATCH 1/3] test: Extend exeter build tests to cover more recent binaries

test/build/build.py tests that the Makefile works to build our binaries. However, it hasn't been updated for a while, and doesn't cover passt-repair or pesto. Extend it to cover those too. Signed-off-by: David Gibson --- test/build/build.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test/build/build.py b/test/build/build.py index e3de8305..eeee5acc 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -68,10 +68,13 @@ def test_make(target: str, expected_files: list[str]) -> None: assert not p.exists(), f"{p} existed after make clean" -exeter.register('make_passt', test_make, 'passt', ['passt']) -exeter.register('make_pasta', test_make, 'pasta', ['pasta']) -exeter.register('make_qrap', test_make, 'qrap', ['qrap']) -exeter.register('make_all', test_make, 'all', ['passt', 'pasta', 'qrap']) +BINARIES = ['passt', 'pasta', 'qrap', 'passt-repair', 'pesto'] + + +for bin in BINARIES: + exeter.register(f'make_{bin.replace('-', '_')}', test_make, bin, [bin]) + +exeter.register('make_all', test_make, 'all', BINARIES) @exeter.test -- 2.54.0

Jan Palus

1:05 p.m.

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

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 Fixes: bc872d91765d ("treewide: Spell ASSERT() as assert()") Signed-off-by: David Gibson --- 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 #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__))

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. I'm assuming you'd prefer to avoid cppcheck suppressions. 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__)))

...

#endif

#ifdef P_tmpdir -- 2.54.0

Stefano Brivio

16 May 16 May

5:46 p.m.

On Fri, 15 May 2026 14:13:09 +1000 David Gibson wrote:

...

Fix the trivial but nasty with -DNDEBUG builds recently reported by Jan Palus. While we're there, add a couple of extra build tests. These wouldn't actually catch this problem, but they're better than nothing and do catch the #include problem I also spotted.

David Gibson (3): test: Extend exeter build tests to cover more recent binaries Fix build with -DNDEBUG test: Add test for builds with -DNDEBUG

Applied, with 2/3 amended as suggested by Jan. Jan, I'll let you know once I tag a new release so that you can drop your no-assert.patch. I'm trying to figure out, first, a couple of potential regressions (https://bugs.passt.top/show_bug.cgi?id=202, https://bugs.passt.top/show_bug.cgi?id=203) so that those can be fixed in the new release as well, but it might be a while before I even get to look into them, and I won't wait more than a few days with a new release in any case. -- Stefano

David Gibson

17 May 17 May

3:08 a.m.

On Sat, May 16, 2026 at 05:46:44PM +0200, Stefano Brivio wrote:

...

On Fri, 15 May 2026 14:13:09 +1000 David Gibson wrote:

...
Fix the trivial but nasty with -DNDEBUG builds recently reported by Jan Palus. While we're there, add a couple of extra build tests. These wouldn't actually catch this problem, but they're better than nothing and do catch the #include problem I also spotted.

David Gibson (3): test: Extend exeter build tests to cover more recent binaries Fix build with -DNDEBUG test: Add test for builds with -DNDEBUG

Applied, with 2/3 amended as suggested by Jan.

Uh.. I'm afraid you modified the wrong copy of assert_with_msg(), effectively nixing asserts in the !NDEBUG case while leaving my implementation in the NDEBUG case.

...

Jan, I'll let you know once I tag a new release so that you can drop your no-assert.patch.

I'm trying to figure out, first, a couple of potential regressions (https://bugs.passt.top/show_bug.cgi?id=202, https://bugs.passt.top/show_bug.cgi?id=203) so that those can be fixed in the new release as well, but it might be a while before I even get to look into them, and I won't wait more than a few days with a new release in any case.

-- Stefano

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

Age (days ago)

Last active (days ago)

List overview

Download

5 comments

3 participants

participants (3)

David Gibson
Jan Palus
Stefano Brivio