Re: [PATCH v2 02/13] cppcheck: Split out essential defines into a BASE_CPPFLAGS variable

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

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

On Wed, May 06, 2026 at 09:46:51AM +0200, Stefano Brivio wrote:
> On Tue, 21 Apr 2026 13:23:27 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Our cppcheck target need certain flags from the compiler so that they it
> > can analyse the code correctly.  Currently we extract these rather
> > awkwardly from FLAGS / CFLAGS / CPPFLAGS.  But this means we inhibit one
> > of cppcheck's features: by default it will attempt to analyse paths for all
> > combinations of compile time options, not just a single one.
> > 
> > Analysing *all* paths doesn't work for us because many of the -D options we
> > use are essential to compile at all, so unless we supply those to cppcheck,
> > overriding the default behaviour we get many spurious errors.  At the
> > moment, however, we give cppcheck *all* our -D options, including
> > conditional / configurable ones, not just the essential ones.
> > 
> > All cppcheck really needs here is those essential -D options.  Split those
> > into a separate variable, and use that directly rather than the clunky
> > $(filter) expression.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  Makefile | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 17e70d22..0de98375 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -30,11 +30,15 @@ ifeq ($(shell $(CC) -O2 -dM -E - < /dev/null 2>&1 | grep ' _FORTIFY_SOURCE ' > /
> >  FORTIFY_FLAG := -D_FORTIFY_SOURCE=2
> >  endif
> >  
> > +# Require preprocessor flags we can't build without
> > +BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \
> > +	-DPAGE_SIZE=$(shell getconf PAGE_SIZE) \
> > +	-DVERSION=\"$(VERSION)\"
> > +
> >  FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security
> > -FLAGS += -pedantic -std=c11 -D_XOPEN_SOURCE=700 -D_GNU_SOURCE
> > +FLAGS += -pedantic -std=c11
> 
> I tried a bit harder but this distinction looks bogus to me (we must

I'm not sure which distinction you're referring to.  There are two
options AFAICT: the distinction between "required" and "default only"
flags.  That one I'll grant is a bit tenuous.  Mind you, that's kind
of already the split between FLAGS and CFLAGS.

The other is the distinction between flags for the preprocessor versus
for the compiler proper.  That's the one I actually need to do what
this series is aiming to do.

> *not* build without -std=c11, FORTIFY_SOURCE, -pie, -fPIE, or
> DUAL_STACK_SOCKETS anyway)

Agreed for -pie and -fPIE.  Maybe for -std=c11.  FORTIFY_SOURCE seems
like something we want to encourage, but by no means essential to the
build.  DUAL_STACK_SOCKETS seems like it should be configurable.
Omitting when possible will waste memory, but should not result in a
failed or incorrect build.

> and adapting the whole series to a
> BASE_CPPFLAGS / CPPFLAGS / CFLAGS split is rather time consuming, even
> if I drop unrelated patches such as 5/13 to 8/13 and 10/13 to 13/13, so
> I would drop this series for now.
> 
> I'm running static checkers on pesto manually for the moment.

Eh, ok.  I'll rework on top of whatever once I'm back.

> Note that the rationale given for 3/13 and 4/13 ignores documented
> reasons behind the current sets of flags. It can be changed indeed but
> functionality needs to be maintained, as I already mentioned in the
> discussion about 4/13.
> 
> >  FLAGS +=  $(FORTIFY_FLAG) -O2 -pie -fPIE
> > -FLAGS += -DPAGE_SIZE=$(shell getconf PAGE_SIZE)
> > -FLAGS += -DVERSION=\"$(VERSION)\"
> > +FLAGS += $(BASE_CPPFLAGS)
> >  FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)
> >  
> >  PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \
> > @@ -195,6 +199,4 @@ CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force	\
> >  	 -D CPPCHECK_6936
> >  
> >  cppcheck: $(PASST_SRCS) $(HEADERS)
> > -	$(CPPCHECK) $(CPPCHECK_FLAGS) 					\
> > -		$(filter -D%,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) $^		\
> > -		$^
> > +	$(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^
> 
> -- 
> 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

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