Re: [PATCH v2 04/13] Makefile: Remove non-standard $(FLAGS) variable

[Stefano Brivio <sbrivio@redhat.com>]

On Tue, 21 Apr 2026 13:23:29 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> FLAGS was introduced over the more standard CFLAGS, because there are some
> options we can't compile without, so overriding CFLAGS from the command
> line wasn't practical.  We've now better dealt with that using
> BASE_CPPFLAGS, so there's no real need for FLAGS any more.  Replace it
> with the more conventional CFLAGS, which now *can* be reasonable overridden
> from the command line.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  Makefile            | 21 ++++++++++-----------
>  test/build/build.py |  4 ++--
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index e89e5556..1e5f0282 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -36,8 +36,8 @@ BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \
>  	-DVERSION=\"$(VERSION)\"
>  CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
>  
> -FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security
> -FLAGS += -pedantic -std=c11 -O2 -pie -fPIE
> +WARNINGS = -Wall -Wextra -Wno-format-zero-length -Wformat-security
> +CFLAGS = -pedantic -std=c11 -O2 -pie -fPIE $(WARNINGS)
>  
>  PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \
>  	epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \
> @@ -66,7 +66,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
>  endif
>  
>  ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
> -	FLAGS += -fstack-protector-strong
> +	CFLAGS += -fstack-protector-strong
>  endif
>  
>  prefix		?= /usr/local
> @@ -85,7 +85,7 @@ endif
>  
>  all: $(BIN) $(MANPAGES) docs
>  
> -static: FLAGS += -static
> +static: CFLAGS += -static
>  static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS
>  static: clean all
>  
> @@ -96,12 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS)
>  	@ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)
>  
>  passt: $(PASST_SRCS) $(HEADERS)
> -	$(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
> +	$(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)
>  
> -passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops
> +passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops
>  passt.avx2: $(PASST_SRCS) $(HEADERS)
> -	$(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \
> -		$(PASST_SRCS) -o passt.avx2 $(LDFLAGS)
> +	$(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS)
>  
>  passt.avx2: passt
>  
> @@ -109,16 +108,16 @@ pasta.avx2 pasta.1 pasta: pasta%: passt%
>  	ln -sf $< $@
>  
>  qrap: $(QRAP_SRCS) passt.h
> -	$(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS)
> +	$(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS)
>  
>  passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h
> -	$(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)
> +	$(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)
>  
>  valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction	\
>  			    rt_sigreturn getpid gettid kill clock_gettime \
>  			    mmap|mmap2 munmap open unlink gettimeofday futex \
>  			    statx readlink
> -valgrind: FLAGS += -g
> +valgrind: CFLAGS += -g
>  valgrind: CPPFLAGS += -DVALGRIND
>  valgrind: all
>  
> diff --git a/test/build/build.py b/test/build/build.py
> index e3de8305..7c9cbb44 100755
> --- a/test/build/build.py
> +++ b/test/build/build.py
> @@ -60,7 +60,7 @@ def test_make(target: str, expected_files: list[str]) -> None:
>      with clone_sources():
>          for p in ex_paths:
>              assert not p.exists(), f"{p} existed before make"
> -        sh(f'make {target} CFLAGS="-Werror"')
> +        sh(f'make {target}')
>          for p in ex_paths:
>              assert p.exists(), f"{p} wasn't made"
>          sh('make clean')
> @@ -90,7 +90,7 @@ def test_install_uninstall() -> None:
>              progs = ['passt', 'pasta', 'qrap']
>  
>              # Install
> -            sh(f'make install CFLAGS="-Werror" prefix={prefix}')
> +            sh(f'make install prefix={prefix}')

Here, and above: I don't understand what (if anything) implies -Werror
now.

-- 
Stefano