Re: [PATCH v2 01/13] Makefile: Use make variables for static checker configuration

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

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

On Tue, May 05, 2026 at 12:14:02PM +0200, Laurent Vivier wrote:
> On 4/21/26 05:23, David Gibson wrote:
> > Our cppcheck and clang-tidy rules don't really follow normal Makefile
> > conventions.  Usually any commands other than the very basics have their
> > binary specified in a variable so it can be overridden on the command line
> > if they're in an unusual location.  Implement that for $(CPPCHECK) and
> > $(CLANG_TIDY)
> > 
> > Likewise flags to tools usually have their own Make variable.  Do the same
> > with $(CLANG_TIDY_FLAGS) and $(CPPCHECK_FLAGS).  Note that these only have
> > the options specifically for the static checker, not compiler flags which
> > we are also supplying to the static checker - those are derived from
> > FLAGS / CFLAGS / CPPFLAGS as before.
> > 
> > As part of that we change the probing for --check-level=exhaustive from
> > being run as part of the cppcheck target, to being run when we build the
> > CPPCHECK_FLAGS variable.  That doesn't make any real difference now, but
> > will make things nicer if we need multiple cppcheck targets in future (e.g.
> > for passt-repair).
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >   Makefile | 32 +++++++++++++++++++-------------
> >   1 file changed, 19 insertions(+), 13 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index f697c12b..17e70d22 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -174,21 +174,27 @@ docs: README.md
> >   		done < README.md;					\
> >   	) > README.plain.md
> > +CLANG_TIDY = clang-tidy
> > +CLANG_TIDY_FLAGS = -DCLANG_TIDY_58992
> > +
> >   clang-tidy: $(PASST_SRCS)
> > -	clang-tidy $^ -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) \
> > -	           -DCLANG_TIDY_58992
> > +	$(CLANG_TIDY) $^ -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) \
> > +		$(CLANG_TIDY_FLAGS)
> > -cppcheck: $(PASST_SRCS) $(HEADERS)
> > -	if cppcheck --check-level=exhaustive /dev/null > /dev/null 2>&1; then \
> > -		CPPCHECK_EXHAUSTIVE="--check-level=exhaustive";		\
> > -	else								\
> > -		CPPCHECK_EXHAUSTIVE=;					\
> > -	fi;								\
> > -	cppcheck --std=c11 --error-exitcode=1 --enable=all --force	\
> > +CPPCHECK = cppcheck
> > +CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force	\
> >   	--inconclusive --library=posix --quiet				\
> > -	$${CPPCHECK_EXHAUSTIVE}						\
> >   	--inline-suppr							\
> > -	--suppress=missingIncludeSystem \
> > +	$(shell if $(CPPCHECK) --quiet --check-level=exhaustive /dev/null; then \
> > +		echo "--check-level=exhaustive";			\
> > +	else								\
> > +		echo "";						\
> > +	fi)			
> 
> Perhaps we can add a reusable function to check flags:
> 
> check_flag = $(shell $(1) $(2) >/dev/null 2>&1 && echo $(2))
> 
> Then
> 	$(call check_flag, $(CPPCHECK) /dev/null, --check-level=exhaustive)

We could, but that seems orthogonal to what I'm trying to accomplish
here.

> > +	--suppress=missingIncludeSystem					\
> >   	--suppress=unusedStructMember					\
> > -	$(filter -D%,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) -D CPPCHECK_6936	\
> > -	$^
> > +	 -D CPPCHECK_6936
> > +
> > +cppcheck: $(PASST_SRCS) $(HEADERS)
> > +	$(CPPCHECK) $(CPPCHECK_FLAGS) 					\
> > +		$(filter -D%,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) $^		\
> > +		$^
> 
> You have duplicate '$^' here.

Oops, that's a bug.

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