Re: [PATCH v3 06/12] Makefile: Split $(FLAGS) into cpp and cc components

On Tue, 12 May 2026 15:52:50 +1000 David Gibson wrote:

The $(FLAGS) variable contains mandatory compiler flags that should not be overridden. However, it contains a mixture of flags for the preprocessor and for the compiler proper. That's causing some inconvenience for other Makefile cleanups, so split it into $(BASE_CPPFLAGS) and $(BASE_CFLAGS) variables.

Signed-off-by: David Gibson --- Makefile | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/Makefile b/Makefile index a8f8d06e..697f229f 100644 --- a/Makefile +++ b/Makefile @@ -30,12 +30,17 @@ ifeq ($(shell $(CC) -O2 -dM -E - < /dev/null 2>&1 | grep ' _FORTIFY_SOURCE ' > / FORTIFY_FLAG := -D_FORTIFY_SOURCE=2 endif

-FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE -FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE -FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) -FLAGS += -DVERSION=\"$(VERSION)\" -FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) +# Mandatory preprocessor flags that won't be overridden with $(CPPFLAGS) +# FIXME: Could some of these be default, rather than required? +BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE $(FORTIFY_FLAG) +BASE_CPPFLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE) +BASE_CPPFLAGS += -DVERSION=\"$(VERSION)\" +BASE_CPPFLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) + +# Mandatory compiler flags that won't be overridden with $(CFLAGS) +# FIXME: Could some of these be default, rather than required? +BASE_CFLAGS := -std=c11 -pie -fPIE -O2 +BASE_CFLAGS += -pedantic -Wall -Wextra -Wno-format-zero-length -Wformat-security

This new version of the series looks good to me in general (minus potential concern reported below), and everything seems to work on Debian and Fedora, but I would still like to try things out on Alpine or Void Linux because musl might cause surprises. I haven't got to it yet. Meanwhile, regarding these FIXME comments: I think it *is* currently possible to override those flags (with different values for the same options), and overriding -D_FORTIFY_SOURCE on openSUSE (I haven't tried right now) was the initial motivation behind FLAGS. That is, the overriding role of CFLAGS seems to be preserved for these BASE_* flags as well, because $CFLAGS is given to the compiler after $BASE_CPPFLAGS, $CPPFLAGS, and $BASE_CFLAGS. So, in this sense, I would already call them "default" flags. If that's the case, I think it's fine. Otherwise we need to find another solution at least for the short term. By the way, if it helps addressing those comments at some point (I would apply anyway this series meanwhile if I don't find breakages, because not being able to run static checkers automatically on pesto is pretty nasty), out of those flags: * -D_XOPEN_SOURCE, -D_GNU_SOURCE, and -DPAGE_SIZE are strictly required to build (at least in some environments) * -D_FORTIFY_SOURCE, -pie, -fPIE are not required to build but they are critical for security * -DVERSION is not required to build but makes things confusing and issues hard to debug because the version (usually supplied by the distribution) isn't reported in logs and logs of other tools - -DDUAL_STACK_SOCKETS doesn't seem to be used anymore starting from commit b8d4fac6a2e7 ("util, pif: Replace sock_l4() with pif_sock_l4()")... was it intended, actually? - -std=c11 is strictly required to ensure we build things correctly - -O2 is optional, but dropping it (by default) might require annoying adjustments in distributions - -pedantic, -Wall, -Wextra, -Wno-format-zero-length, -Wformat-security are all optional and useful for development (including distribution development), and might be security relevant in some cases -- Stefano