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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5276 bytes --]

On Tue, Apr 28, 2026 at 09:17:31AM +0200, Stefano Brivio wrote:
> 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.

Ah, oops.  I misread the -Werror in test/Makefile, thinking it was in
Makefile and applied to everything.  I think the right fix is to put
-Werror in the default CFLAGS and remove it from here.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]