[PATCH v3 0/6] Test and linter fixups

David Gibson

2 Oct 2025 2 Oct '25

7:04 a.m.

Before starting to convert testcases to use tunbridge, I wanted to add linting for the Python test scripts. While doing that I discovered a new crop of cppcheck and clang-tidy false positives, and a Makefile bug. Here's a batch of fixes. I didn't manage to get through the whole testsuite with this. I keep getting hangs on rampstream_out, which I *think* are unrelated. v3: * Delete mypy's cache on make clean v2: * Actually understood why exetool was being deleted, and fixed it properly. David Gibson (6): clang-tidy: Suppress redundant expression warning cppcheck: Suppress the suppression of a suppression cppcheck: Suppress a buggy cppcheck warning cppcheck: Suppress variable scope warnings in dhcpv6() test: Don't delete exetool on make clean test: Add linting of Python test scripts dhcpv6.c | 8 ++++++++ linux_dep.h | 2 +- tcp.c | 5 +++++ test/Makefile | 20 ++++++++++++++++---- test/build/build.py | 5 +++-- test/build/static_checkers.sh | 6 +++++- vhost_user.c | 1 + 7 files changed, 39 insertions(+), 8 deletions(-) -- 2.51.0

Show replies by date

David Gibson

2 Oct 2 Oct

7:04 a.m.

New subject: [PATCH v3 3/6] cppcheck: Suppress a buggy cppcheck warning

Another cppcheck package in Fedora, another bogus false positive. This one seems to not realise that a variable has been initialised by getsockopt() under a complicated set of circumstances. Signed-off-by: David Gibson --- tcp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tcp.c b/tcp.c index 48b1ef29..7da41797 100644 --- a/tcp.c +++ b/tcp.c @@ -1030,6 +1030,11 @@ int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, return 0; } + /* This trips a cppcheck bug in some versions, including + * cppcheck 2.18.3. + * https://sourceforge.net/p/cppcheck/discussion/general/thread/fecde59085/ + */ + /* cppcheck-suppress [uninitvar,unmatchedSuppression] */ conn->seq_ack_to_tap = tinfo->tcpi_bytes_acked + conn->seq_init_from_tap; -- 2.51.0

David Gibson

7:04 a.m.

New subject: [PATCH v3 5/6] test: Don't delete exetool on make clean

exetool is listed in $(LOCAL_ASSETS), which seems like it makes sense, until you realise that $(LOCAL_ASSETS) are deleted on make clean, and we can't rebuild exetool after deleting it (since it's just part of the exeter tree we download). Move it to $(DOWNLOAD_ASSETS) instead. Signed-off-by: David Gibson --- test/Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/Makefile b/test/Makefile index 49388276..3800a0a9 100644 --- a/test/Makefile +++ b/test/Makefile @@ -52,15 +52,14 @@ UBUNTU_NEW_IMGS = xenial-server-cloudimg-powerpc-disk1.img \ jammy-server-cloudimg-s390x.img UBUNTU_IMGS = $(UBUNTU_OLD_IMGS) $(UBUNTU_NEW_IMGS) -DOWNLOAD_ASSETS = exeter mbuto podman \ +DOWNLOAD_ASSETS = $(EXETOOL) mbuto podman \ $(DEBIAN_IMGS) $(FEDORA_IMGS) $(OPENSUSE_IMGS) $(UBUNTU_IMGS) TESTDATA_ASSETS = small.bin big.bin medium.bin \ rampstream LOCAL_ASSETS = mbuto.img mbuto.mem.img podman/bin/podman QEMU_EFI.fd \ $(DEBIAN_IMGS:%=prepared-%) $(FEDORA_IMGS:%=prepared-%) \ $(UBUNTU_NEW_IMGS:%=prepared-%) \ - nstool guest-key guest-key.pub $(EXETOOL) \ - $(TESTDATA_ASSETS) + nstool guest-key guest-key.pub $(TESTDATA_ASSETS) ASSETS = $(DOWNLOAD_ASSETS) $(LOCAL_ASSETS) -- 2.51.0

David Gibson

7:04 a.m.

New subject: [PATCH v3 6/6] test: Add linting of Python test scripts

We currently have one test moved to the new exeter based framwork written in Python. We plan to add many more, so add linting (flake8) and type checking (mypy) of those scripts. This can be invoked manually with "make flake8" or "make mypy" in test/, and is also added to the static checkers test set. Signed-off-by: David Gibson --- test/Makefile | 15 ++++++++++++++- test/build/build.py | 5 +++-- test/build/static_checkers.sh | 6 +++++- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/test/Makefile b/test/Makefile index 3800a0a9..5b5f0fc1 100644 --- a/test/Makefile +++ b/test/Makefile @@ -8,6 +8,8 @@ BATS = bats -j $(shell nproc) EXETOOL = exeter/exetool/exetool WGET = wget -c +FLAKE8 = flake8 +MYPY = mypy --strict DEBIAN_IMGS = debian-8.11.0-openstack-amd64.qcow2 \ debian-10-nocloud-amd64.qcow2 \ @@ -64,11 +66,15 @@ LOCAL_ASSETS = mbuto.img mbuto.mem.img podman/bin/podman QEMU_EFI.fd \ ASSETS = $(DOWNLOAD_ASSETS) $(LOCAL_ASSETS) EXETER_PYPATH = exeter/py3 +EXETER_PYTHON = build/build.py EXETER_BATS = smoke/smoke.sh.bats \ - build/build.py.bats build/static_checkers.sh.bats + $(EXETER_PYTHON:%=%.bats) build/static_checkers.sh.bats BATS_FILES = $(EXETER_BATS) \ podman/test/system/505-networking-pasta.bats +# Python test code (for linters) +PYPKGS = $(EXETER_PYTHON) + CFLAGS = -Wall -Werror -Wextra -pedantic -std=c99 assets: $(ASSETS) @@ -127,6 +133,12 @@ medium.bin: big.bin: dd if=/dev/urandom bs=1M count=10 of=$@ +flake8: pull-exeter + PYTHONPATH=$(EXETER_PYPATH) $(FLAKE8) $(PYPKGS) + +mypy: pull-exeter + PYTHONPATH=$(EXETER_PYPATH) $(MYPY) $(PYPKGS) + $(EXETER_BATS): %.bats: % $(EXETOOL) PYTHONPATH=$(EXETER_PYPATH) $(EXETOOL) bats -- $< > $@ @@ -141,6 +153,7 @@ debug: assets clean: rm -f perf.js *~ + rm -rf .mypy_cache rm -f $(LOCAL_ASSETS) rm -f $(EXETER_BATS) rm -rf test_logs diff --git a/test/build/build.py b/test/build/build.py index e49287c9..e3de8305 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -18,11 +18,12 @@ import os from pathlib import Path import subprocess import tempfile -from typing import Iterable, Iterator +from typing import Iterator import exeter -def sh(cmd): + +def sh(cmd: str) -> None: """Run given command in a shell""" subprocess.run(cmd, shell=True) diff --git a/test/build/static_checkers.sh b/test/build/static_checkers.sh index 42806e79..228b99ae 100755 --- a/test/build/static_checkers.sh +++ b/test/build/static_checkers.sh @@ -21,6 +21,10 @@ exeter_set_description cppcheck "passt sources pass cppcheck" exeter_register clang_tidy make -C .. clang-tidy exeter_set_description clang_tidy "passt sources pass clang-tidy" -exeter_main "$@" +exeter_register flake8 make flake8 +exeter_set_description flake8 "passt tests in Python pass flake8" +exeter_register mypy make mypy +exeter_set_description mypy "passt tests in Python pass mypy --strict" +exeter_main "$@" -- 2.51.0

David Gibson

7:04 a.m.

New subject: [PATCH v3 1/6] clang-tidy: Suppress redundant expression warning

clang-tidy 20.1.8 doesn't like (VHOST_USER_MAX_VQS / 2), because it expands to (2 / 2). But in the context of the #define, this makes logical sense, so suppress the warning. I'm not sure why it isn't firing on the debug() line just below. Possibly it only complains once per expression per function, so we only have to suppress it once? Signed-off-by: David Gibson --- vhost_user.c | 1 + 1 file changed, 1 insertion(+) diff --git a/vhost_user.c b/vhost_user.c index fa343a86..223332d5 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -939,6 +939,7 @@ static bool vu_get_queue_num_exec(struct vu_dev *vdev, { (void)vdev; + /* NOLINTNEXTLINE(misc-redundant-expression) */ vmsg_set_reply_u64(vmsg, VHOST_USER_MAX_VQS / 2); debug("VHOST_USER_MAX_VQS %u", VHOST_USER_MAX_VQS / 2); -- 2.51.0

Stefano Brivio

7 Oct 7 Oct

4:13 p.m.

New subject: [PATCH v3 1/6] clang-tidy: Suppress redundant expression warning

On Thu, 2 Oct 2025 15:04:32 +1000 David Gibson wrote:

...

clang-tidy 20.1.8 doesn't like (VHOST_USER_MAX_VQS / 2), because it expands to (2 / 2). But in the context of the #define, this makes logical sense, so suppress the warning.

I'm not sure why it isn't firing on the debug() line just below. Possibly it only complains once per expression per function, so we only have to suppress it once?

I have the feeling it's not really "exploring" debug() calls, rather, but I didn't really investigate. Whatever, it doesn't really matter.

...

Signed-off-by: David Gibson --- vhost_user.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/vhost_user.c b/vhost_user.c index fa343a86..223332d5 100644 --- a/vhost_user.c +++ b/vhost_user.c @@ -939,6 +939,7 @@ static bool vu_get_queue_num_exec(struct vu_dev *vdev, { (void)vdev;

+ /* NOLINTNEXTLINE(misc-redundant-expression) */ vmsg_set_reply_u64(vmsg, VHOST_USER_MAX_VQS / 2);

debug("VHOST_USER_MAX_VQS %u", VHOST_USER_MAX_VQS / 2);

-- Stefano

Stefano Brivio

4:13 p.m.

On Thu, 2 Oct 2025 15:04:31 +1000 David Gibson wrote:

...

Before starting to convert testcases to use tunbridge, I wanted to add linting for the Python test scripts. While doing that I discovered a new crop of cppcheck and clang-tidy false positives, and a Makefile bug.

Here's a batch of fixes.

I didn't manage to get through the whole testsuite with this. I keep getting hangs on rampstream_out, which I *think* are unrelated.

I saw this once too, a couple of weeks ago. Maybe it's the same issue I've been trying to find the time to work on for a while now, that is, the kernel not freezing queues of the sockets from the source instance once they switch to repair mode. But it happened just once so I didn't try re-running with PCAP=1. I might, if it happens again.

...

v3: * Delete mypy's cache on make clean v2: * Actually understood why exetool was being deleted, and fixed it properly.

David Gibson (6): clang-tidy: Suppress redundant expression warning cppcheck: Suppress the suppression of a suppression cppcheck: Suppress a buggy cppcheck warning cppcheck: Suppress variable scope warnings in dhcpv6() test: Don't delete exetool on make clean test: Add linting of Python test scripts

I'm applying this now, even though, strictly speaking, this breaks tests because test/README.md doesn't mention one needs to install flake8 and mypy, and I don't think they're very commonly installed, so I think you should update it. -- Stefano

David Gibson

8 Oct 8 Oct

2:22 a.m.

On Tue, Oct 07, 2025 at 04:13:46PM +0200, Stefano Brivio wrote:

...

On Thu, 2 Oct 2025 15:04:31 +1000 David Gibson wrote:

...
Before starting to convert testcases to use tunbridge, I wanted to add linting for the Python test scripts. While doing that I discovered a new crop of cppcheck and clang-tidy false positives, and a Makefile bug.

Here's a batch of fixes.

I didn't manage to get through the whole testsuite with this. I keep getting hangs on rampstream_out, which I *think* are unrelated.

I saw this once too, a couple of weeks ago.

Maybe it's the same issue I've been trying to find the time to work on for a while now, that is, the kernel not freezing queues of the sockets from the source instance once they switch to repair mode.

Maybe.

...

But it happened just once so I didn't try re-running with PCAP=1. I might, if it happens again.

...
v3: * Delete mypy's cache on make clean v2: * Actually understood why exetool was being deleted, and fixed it properly.

David Gibson (6): clang-tidy: Suppress redundant expression warning cppcheck: Suppress the suppression of a suppression cppcheck: Suppress a buggy cppcheck warning cppcheck: Suppress variable scope warnings in dhcpv6() test: Don't delete exetool on make clean test: Add linting of Python test scripts

I'm applying this now, even though, strictly speaking, this breaks tests because test/README.md doesn't mention one needs to install flake8 and mypy, and I don't think they're very commonly installed, so I think you should update it.

Ah, good point. Beter yet, I should skip rather than failing if the linters aren't available. I'll send a follow up for this. -- 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

Age (days ago)

Last active (days ago)

List overview

Download

7 comments

2 participants

participants (2)

David Gibson
Stefano Brivio

[PATCH v3 0/6] Test and linter fixups

tags

participants (2)