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

[Stefano Brivio <sbrivio@redhat.com>]

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
*not* build without -std=c11, FORTIFY_SOURCE, -pie, -fPIE, or
DUAL_STACK_SOCKETS anyway) 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.

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