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

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 12 May 2026 15:52:50 +1000
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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