[PATCH 00/13] Improvements to static checker invocation
While working on pesto, I ran into a number of awkward errors with the static checkers. This series reworks the invocation of the checkers in a way that will let us deal with that. As a bonus, it also gives us static checking for passt-repair and qrap. It also makes a few other cleanups to the Makefile that seemed natural along the way. David Gibson (13): Makefile: Use make variables for static checker configuration cppcheck: Split out essential defines into a BASE_CPPFLAGS variable Makefile: Remove preprocessor flags from $(FLAGS) Makefile: Remove non-standard $(FLAGS) variable Makefile: Make conditional definition of $(BIN) clearer Makefile: Use common binary compilation rule Makefile: Remove unhelpful $(HEADERS) variable Makefile: Add header dependencies for secondary binaries Makefile: Split static checker targets passt-repair: Split out inotify handling to its own function passt-repair: Simplify construction of Unix path from inotify passt-repair: Run static checkers qrap: Run static checkers Makefile | 106 ++++++++++++++++++------------ linux_dep.h | 2 +- passt-repair.c | 171 +++++++++++++++++++++++++++---------------------- qrap.c | 42 +++++++----- 4 files changed, 188 insertions(+), 133 deletions(-) -- 2.53.0
FLAGS contains both compiler and preprocessor flags, which isn't great
make practice. Move preprocessor flags out of that variable and into the
more conventionally named $(CPPFLAGS). Use both $(BASE_CPPFLAGS) and
$(CPPFLAGS) in most places, so we get both the essential flags and the
conditional ones (which could be overridden on the command line).
Signed-off-by: David Gibson
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
The list of binaries is dependent on the target architecture, because
x86_64 addes the passt.avx2 and pasta.avx2 binaries. Make this more clear
by defining BIN in common, then augmenting it in the x86_64 case.
Signed-off-by: David Gibson
Each of our binaries (passt, passt.avx2, qrap and passt-repair) has a
separate Make rule instructing how to compile it, but they're all basically
identical. Combine these all into a single pattern rule, just using
different dependencies and variable overrides where necessary.
Signed-off-by: David Gibson
Confusingly HEADERS is not headers that are shared between our various
binaries. Rather it's just the (non generated) headers for passt, plus
seccomp.h. This isn't particularly useful, just open code it in the
handful of places we need it.
Signed-off-by: David Gibson
PASST_HEADERS contains all the headers used by passt, which we use in
various dependencies. However, qrap and passt-repair each use several
headers which we don't have dependencies for. Add handling for this to the
Makefile.
Signed-off-by: David Gibson
passt-repair can operate two ways: either it can be given an explicit
socket path to connect to, or it can be given a directory. In the second
mode, it will wait for a socket to appear in that directory before
connecting to it.
That waiting involves some inotify logic that is essentially unrelated to
the rest of the code. However, it's currently inline in main() making that
very long. Moreover, the block handling inotify shadows several variables
used in the rest of main() which will make static checkers complain once
we get them running on passt-repair.
Address this by moving the inotify handling into its own function.
Signed-off-by: David Gibson
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
Currently we have a single 'cppcheck' and 'clang-tidy' target which checks
passt. However, it doesn't check the additional binaries, qrap and
passt-repair. In preparation for running the static checkers on those as
well, split the targets into a top-level rule and a pattern rule which we
will be able to reuse.
Signed-off-by: David Gibson
Run the static checkers, cppcheck and clang-tidy on passt-repair as well
as on passt proper. This shows up handful of remaining minor warnings,
which we correct.
Signed-off-by: David Gibson
Run the static checkers on qrap as well as on passt-repair and passt. This
shows a number of minor warnings, which we fix.
Signed-off-by: David Gibson
When passt-repair is invoked with a directory name, it waits for a Unix
socket to appear in that directory. We need to build the Unix path name
from the given directory, plus the stem file name from the inotify event.
Currently, we build that path into a temporary buffer of size PATH_MAX,
then move it into the smaller buffer inside the Unix sockaddr. There's no
particular reason for this two step process, we can build the address
directly within the sockaddr_un. This will give a slightly different error
if the constructed path exceeds the maximum length of a Unix address, but
it will fail either way so it doesn't really matter.
Signed-off-by: David Gibson
On Tue, Apr 21, 2026 at 12:43:31PM +1000, David Gibson wrote:
While working on pesto, I ran into a number of awkward errors with the static checkers. This series reworks the invocation of the checkers in a way that will let us deal with that. As a bonus, it also gives us static checking for passt-repair and qrap. It also makes a few other cleanups to the Makefile that seemed natural along the way.
Sorry, realised 4/13 introduced a test failure due to a bad interaction with test/build/build.py. New spin coming shortly.
David Gibson (13): Makefile: Use make variables for static checker configuration cppcheck: Split out essential defines into a BASE_CPPFLAGS variable Makefile: Remove preprocessor flags from $(FLAGS) Makefile: Remove non-standard $(FLAGS) variable Makefile: Make conditional definition of $(BIN) clearer Makefile: Use common binary compilation rule Makefile: Remove unhelpful $(HEADERS) variable Makefile: Add header dependencies for secondary binaries Makefile: Split static checker targets passt-repair: Split out inotify handling to its own function passt-repair: Simplify construction of Unix path from inotify passt-repair: Run static checkers qrap: Run static checkers
Makefile | 106 ++++++++++++++++++------------ linux_dep.h | 2 +- passt-repair.c | 171 +++++++++++++++++++++++++++---------------------- qrap.c | 42 +++++++----- 4 files changed, 188 insertions(+), 133 deletions(-)
-- 2.53.0
-- 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
participants (1)
-
David Gibson