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

5 May 2026

On Tue, 5 May 2026 12:49:11 +0200
Laurent Vivier  wrote:
...
On 5/5/26 12:20, Stefano Brivio wrote:
...
On Tue, 5 May 2026 12:14:02 +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 
---
   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)
      		\
...
+	--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.
Just note that it's not necessarily important to review this series
right now (I'm not sure).
okay, I was reviewing it because "Dynamic configuration" series doesn't apply cleanly 
without it.
Yes, absolutely, and we can't run static checkers on pesto without it,
so we need something like this series.

If we (especially: you ;)) can fix whatever comment you have plus my
concern on 4/13 quickly enough, it makes sense to continue with it. If
it becomes a big effort maybe we should go for a less involved approach
though. I'm not sure.

-- 
Stefano

    