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) \ + --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)) $^ \ + $^ -- 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 --- Makefile | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index 0de98375..e89e5556 100644 --- a/Makefile +++ b/Makefile @@ -34,12 +34,10 @@ endif BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ -DPAGE_SIZE=$(shell getconf PAGE_SIZE) \ -DVERSION=\"$(VERSION)\" +CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -FLAGS += $(FORTIFY_FLAG) -O2 -pie -fPIE -FLAGS += $(BASE_CPPFLAGS) -FLAGS += -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) +FLAGS += -pedantic -std=c11 -O2 -pie -fPIE PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ @@ -64,7 +62,7 @@ HEADERS = $(PASST_HEADERS) seccomp.h C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) - FLAGS += -DHAS_GETRANDOM + CPPFLAGS += -DHAS_GETRANDOM endif ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -87,7 +85,8 @@ endif all: $(BIN) $(MANPAGES) docs -static: FLAGS += -static -DGLIBC_NO_STATIC_NSS +static: FLAGS += -static +static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS static: clean all seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) @@ -97,11 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS) passt: $(PASST_SRCS) $(HEADERS) - $(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) + $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops passt.avx2: $(PASST_SRCS) $(HEADERS) - $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(CPPFLAGS) \ + $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \ $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) passt.avx2: passt @@ -110,16 +109,17 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@ qrap: $(QRAP_SRCS) passt.h - $(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) + $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h - $(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) + $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ statx readlink -valgrind: FLAGS += -g -DVALGRIND +valgrind: FLAGS += -g +valgrind: CPPFLAGS += -DVALGRIND valgrind: all .PHONY: clean @@ -182,8 +182,7 @@ CLANG_TIDY = clang-tidy CLANG_TIDY_FLAGS = -DCLANG_TIDY_58992 clang-tidy: $(PASST_SRCS) - $(CLANG_TIDY) $^ -- $(filter-out -pie,$(FLAGS) $(CFLAGS) $(CPPFLAGS)) \ - $(CLANG_TIDY_FLAGS) + $(CLANG_TIDY) $^ -- $(BASE_CPPFLAGS) $(CPPFLAGS) $(CLANG_TIDY_FLAGS) CPPCHECK = cppcheck CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force \ -- 2.53.0

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 --- Makefile | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index a2576771..18343491 100644 --- a/Makefile +++ b/Makefile @@ -77,9 +77,14 @@ docdir ?= $(datarootdir)/doc/passt mandir ?= $(datarootdir)/man man1dir ?= $(mandir)/man1 -BIN = passt pasta qrap passt-repair +BASEBIN = passt qrap passt-repair ifeq ($(TARGET_ARCH),x86_64) -BIN += passt.avx2 pasta.avx2 +BASEBIN += passt.avx2 +endif + +BIN = $(BASEBIN) pasta +ifeq ($(TARGET_ARCH),x86_64) +BIN += pasta.avx2 endif all: $(BIN) $(MANPAGES) docs @@ -94,23 +99,21 @@ seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS) +$(BASEBIN): %: + $(CC) $(BASE_CPPFLAGS) $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) $(filter %.c,$^) -o $@ + passt: $(PASST_SRCS) $(HEADERS) - $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops passt.avx2: $(PASST_SRCS) $(HEADERS) - $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) - -passt.avx2: passt pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@ +qrap: BASE_CPPFLAGS += -DARCH=\"$(TARGET_ARCH)\" qrap: $(QRAP_SRCS) passt.h - $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h - $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ -- 2.53.0

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 --- passt-repair.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/passt-repair.c b/passt-repair.c index 980b0b09..d4c8ce9a 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -64,10 +64,9 @@ static int wait_for_socket(struct sockaddr_un *a, const char *dir, char buf[sizeof(struct inotify_event) + NAME_MAX + 1] __attribute__ ((aligned(__alignof__(struct inotify_event)))); const struct inotify_event *ev = NULL; - char path[PATH_MAX + 1]; bool found = false; + int fd, ret; ssize_t n; - int fd; if ((fd = inotify_init1(IN_CLOEXEC)) < 0) { fprintf(stderr, "inotify_init1: %i\n", errno); @@ -113,13 +112,15 @@ static int wait_for_socket(struct sockaddr_un *a, const char *dir, _exit(1); } - snprintf(path, sizeof(path), "%s/%s", dir, ev->name); - if ((stat(path, sb))) { - fprintf(stderr, "Can't stat() %s: %i\n", path, errno); + ret = snprintf(a->sun_path, sizeof(a->sun_path), "%s/%s", + dir, ev->name); + + if ((stat(a->sun_path, sb))) { + fprintf(stderr, "Can't stat() %s: %i\n", a->sun_path, errno); _exit(1); } - return snprintf(a->sun_path, sizeof(a->sun_path), "%s", path); + return ret; } /** -- 2.53.0

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 --- Makefile | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 03113803..9cb5e1ba 100644 --- a/Makefile +++ b/Makefile @@ -58,6 +58,8 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h conf.h dhcp.h dhcpv6.h \ siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h tcp_splice.h \ tcp_vu.h udp.h udp_flow.h udp_internal.h udp_vu.h util.h vhost_user.h \ virtio.h vu_common.h +QRAP_HEADERS = arp.h ip.h passt.h util.h +PASST_REPAIR_HEADERS = linux_dep.h C := \#include \nint main(){int a=getrandom(0, 0, 0);} ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) @@ -95,7 +97,7 @@ static: clean all seccomp.h: seccomp.sh $(PASST_SRCS) $(PASST_HEADERS) @ EXTRA_SYSCALLS="$(EXTRA_SYSCALLS)" ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp.h $(PASST_SRCS) $(PASST_HEADERS) -seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) +seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS) $(BASEBIN): %: @@ -110,9 +112,9 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@ qrap: BASE_CPPFLAGS += -DARCH=\"$(TARGET_ARCH)\" -qrap: $(QRAP_SRCS) passt.h +qrap: $(QRAP_SRCS) $(QRAP_HEADERS) -passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h +passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ -- 2.53.0

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 --- Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 1e5f0282..a2576771 100644 --- a/Makefile +++ b/Makefile @@ -77,10 +77,9 @@ docdir ?= $(datarootdir)/doc/passt mandir ?= $(datarootdir)/man man1dir ?= $(mandir)/man1 +BIN = passt pasta qrap passt-repair ifeq ($(TARGET_ARCH),x86_64) -BIN := passt passt.avx2 pasta pasta.avx2 qrap passt-repair -else -BIN := passt pasta qrap passt-repair +BIN += passt.avx2 pasta.avx2 endif all: $(BIN) $(MANPAGES) docs -- 2.53.0

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 --- Makefile | 6 ++++-- linux_dep.h | 2 +- passt-repair.c | 49 +++++++++++++++++++++++++------------------------ 3 files changed, 30 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index b8a3cf9e..d29f46d2 100644 --- a/Makefile +++ b/Makefile @@ -183,13 +183,14 @@ docs: README.md CLANG_TIDY = clang-tidy CLANG_TIDY_FLAGS = -DCLANG_TIDY_58992 -clang-tidy: passt.clang-tidy +clang-tidy: passt.clang-tidy passt-repair.clang-tidy .PHONY: %.clang-tidy %.clang-tidy: $(CLANG_TIDY) $(filter %.c,$^) -- $(BASE_CPPFLAGS) $(CPPFLAGS) $(CLANG_TIDY_FLAGS) passt.clang-tidy: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h +passt-repair.clang-tidy: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h CPPCHECK = cppcheck CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force \ @@ -204,10 +205,11 @@ CPPCHECK_FLAGS = --std=c11 --error-exitcode=1 --enable=all --force \ --suppress=unusedStructMember \ -D CPPCHECK_6936 -cppcheck: passt.cppcheck +cppcheck: passt.cppcheck passt-repair.cppcheck .PHONY: %.cppcheck %.cppcheck: $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^ passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h +passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h diff --git a/linux_dep.h b/linux_dep.h index 3f8184bd..fa539ce4 100644 --- a/linux_dep.h +++ b/linux_dep.h @@ -145,7 +145,7 @@ struct tcp_info_linux { #endif __attribute__ ((weak)) -/* cppcheck-suppress [funcArgNamesDifferent,unmatchedSuppression] */ +/* cppcheck-suppress [funcArgNamesDifferent,unusedFunction,unmatchedSuppression] */ int close_range(unsigned int first, unsigned int last, int flags) { return syscall(SYS_close_range, first, last, flags); } diff --git a/passt-repair.c b/passt-repair.c index d4c8ce9a..80a39086 100644 --- a/passt-repair.c +++ b/passt-repair.c @@ -46,6 +46,9 @@ #define REPAIR_EXT ".repair" #define REPAIR_EXT_LEN strlen(REPAIR_EXT) +/* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */ +#define FPRINTF(f, ...) (void)fprintf(f, __VA_ARGS__) + /** * wait_for_socket() - Wait for a Unix socket to appear in a directory * @a: Unix domain address to update with socket's path @@ -66,33 +69,33 @@ static int wait_for_socket(struct sockaddr_un *a, const char *dir, const struct inotify_event *ev = NULL; bool found = false; int fd, ret; - ssize_t n; if ((fd = inotify_init1(IN_CLOEXEC)) < 0) { - fprintf(stderr, "inotify_init1: %i\n", errno); + FPRINTF(stderr, "inotify_init1: %i\n", errno); _exit(1); } if (inotify_add_watch(fd, dir, IN_CREATE) < 0) { - fprintf(stderr, "inotify_add_watch: %i\n", errno); + FPRINTF(stderr, "inotify_add_watch: %i\n", errno); _exit(1); } do { + ssize_t n; char *p; n = read(fd, buf, sizeof(buf)); if (n < 0) { - fprintf(stderr, "inotify read: %i\n", errno); + FPRINTF(stderr, "inotify read: %i\n", errno); _exit(1); } - buf[n - 1] = '\0'; if (n < (ssize_t)sizeof(*ev)) { - fprintf(stderr, "Short inotify read: %zi\n", n); + FPRINTF(stderr, "Short inotify read: %zi\n", n); continue; } + buf[n - 1] = '\0'; for (p = buf; p < buf + n; p += sizeof(*ev) + ev->len) { ev = (const struct inotify_event *)p; @@ -108,7 +111,7 @@ static int wait_for_socket(struct sockaddr_un *a, const char *dir, } while (!found); if (ev->len > NAME_MAX + 1 || ev->name[ev->len - 1] != '\0') { - fprintf(stderr, "Invalid filename from inotify\n"); + FPRINTF(stderr, "Invalid filename from inotify\n"); _exit(1); } @@ -116,7 +119,7 @@ static int wait_for_socket(struct sockaddr_un *a, const char *dir, dir, ev->name); if ((stat(a->sun_path, sb))) { - fprintf(stderr, "Can't stat() %s: %i\n", a->sun_path, errno); + FPRINTF(stderr, "Can't stat() %s: %i\n", a->sun_path, errno); _exit(1); } @@ -160,7 +163,7 @@ int main(int argc, char **argv) prog.filter = filter_repair; if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) || prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) { - fprintf(stderr, "Failed to apply seccomp filter\n"); + FPRINTF(stderr, "Failed to apply seccomp filter\n"); _exit(1); } @@ -173,17 +176,17 @@ int main(int argc, char **argv) cmsg = CMSG_FIRSTHDR(&msg); if (argc != 2) { - fprintf(stderr, "Usage: %s PATH\n", argv[0]); + FPRINTF(stderr, "Usage: %s PATH\n", argv[0]); _exit(2); } if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) { - fprintf(stderr, "Failed to create AF_UNIX socket: %i\n", errno); + FPRINTF(stderr, "Failed to create AF_UNIX socket: %i\n", errno); _exit(1); } if ((stat(argv[1], &sb))) { - fprintf(stderr, "Can't stat() %s: %i\n", argv[1], errno); + FPRINTF(stderr, "Can't stat() %s: %i\n", argv[1], errno); _exit(1); } @@ -195,12 +198,12 @@ int main(int argc, char **argv) } if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) { - fprintf(stderr, "Invalid socket path\n"); + FPRINTF(stderr, "Invalid socket path\n"); _exit(2); } if ((sb.st_mode & S_IFMT) != S_IFSOCK) { - fprintf(stderr, "%s is not a socket\n", a.sun_path); + FPRINTF(stderr, "%s is not a socket\n", a.sun_path); _exit(2); } @@ -208,7 +211,7 @@ int main(int argc, char **argv) if (inotify_dir && errno == ECONNREFUSED) continue; - fprintf(stderr, "Failed to connect to %s: %s\n", a.sun_path, + FPRINTF(stderr, "Failed to connect to %s: %s\n", a.sun_path, strerror(errno)); _exit(1); } @@ -219,7 +222,7 @@ loop: if (errno == ECONNRESET) { ret = 0; } else { - fprintf(stderr, "Failed to read message: %i\n", errno); + FPRINTF(stderr, "Failed to read message: %i\n", errno); _exit(1); } } @@ -231,7 +234,7 @@ loop: cmsg->cmsg_len < CMSG_LEN(sizeof(int)) || cmsg->cmsg_len > CMSG_LEN(sizeof(int) * SCM_MAX_FD) || cmsg->cmsg_type != SCM_RIGHTS) { - fprintf(stderr, "No/bad ancillary data from peer\n"); + FPRINTF(stderr, "No/bad ancillary data from peer\n"); _exit(1); } @@ -246,7 +249,7 @@ loop: } if (!n) { cmsg_len = cmsg->cmsg_len; /* socklen_t is 'unsigned' on musl */ - fprintf(stderr, "Invalid ancillary data length %zu from peer\n", + FPRINTF(stderr, "Invalid ancillary data length %zu from peer\n", cmsg_len); _exit(1); } @@ -255,15 +258,15 @@ loop: if (cmd != TCP_REPAIR_ON && cmd != TCP_REPAIR_OFF && cmd != TCP_REPAIR_OFF_NO_WP) { - fprintf(stderr, "Unsupported command 0x%04x\n", cmd); + FPRINTF(stderr, "Unsupported command 0x%04x\n", cmd); _exit(1); } - op = cmd; + op = (int)cmd; for (i = 0; i < n; i++) { if (setsockopt(fds[i], SOL_TCP, TCP_REPAIR, &op, sizeof(op))) { - fprintf(stderr, + FPRINTF(stderr, "Setting TCP_REPAIR to %i on socket %i: %s\n", op, fds[i], strerror(errno)); _exit(1); @@ -275,11 +278,9 @@ loop: /* Confirm setting by echoing the command back */ if (send(s, &cmd, sizeof(cmd), 0) < 0) { - fprintf(stderr, "Reply to %i: %s\n", op, strerror(errno)); + FPRINTF(stderr, "Reply to %i: %s\n", op, strerror(errno)); _exit(1); } goto loop; - - return 0; } -- 2.53.0

On Tue, Apr 28, 2026 at 09:17:31AM +0200, Stefano Brivio wrote:

On Tue, 21 Apr 2026 13:23:29 +1000 David Gibson wrote:

...

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 --- Makefile | 21 ++++++++++----------- test/build/build.py | 4 ++-- 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile index e89e5556..1e5f0282 100644 --- a/Makefile +++ b/Makefile @@ -36,8 +36,8 @@ BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ -DVERSION=\"$(VERSION)\" CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)

-FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -O2 -pie -fPIE +WARNINGS = -Wall -Wextra -Wno-format-zero-length -Wformat-security +CFLAGS = -pedantic -std=c11 -O2 -pie -fPIE $(WARNINGS)

PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ @@ -66,7 +66,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) endif

ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0) - FLAGS += -fstack-protector-strong + CFLAGS += -fstack-protector-strong endif

prefix ?= /usr/local @@ -85,7 +85,7 @@ endif

all: $(BIN) $(MANPAGES) docs

-static: FLAGS += -static +static: CFLAGS += -static static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS static: clean all

@@ -96,12 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)

passt: $(PASST_SRCS) $(HEADERS) - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)

-passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops +passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops passt.avx2: $(PASST_SRCS) $(HEADERS) - $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \ - $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS)

passt.avx2: passt

@@ -109,16 +108,16 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@

qrap: $(QRAP_SRCS) passt.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS)

passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ statx readlink -valgrind: FLAGS += -g +valgrind: CFLAGS += -g valgrind: CPPFLAGS += -DVALGRIND valgrind: all

diff --git a/test/build/build.py b/test/build/build.py index e3de8305..7c9cbb44 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -60,7 +60,7 @@ def test_make(target: str, expected_files: list[str]) -> None: with clone_sources(): for p in ex_paths: assert not p.exists(), f"{p} existed before make" - sh(f'make {target} CFLAGS="-Werror"') + sh(f'make {target}') for p in ex_paths: assert p.exists(), f"{p} wasn't made" sh('make clean') @@ -90,7 +90,7 @@ def test_install_uninstall() -> None: progs = ['passt', 'pasta', 'qrap']

# Install - sh(f'make install CFLAGS="-Werror" prefix={prefix}') + sh(f'make install prefix={prefix}')

Here, and above: I don't understand what (if anything) implies -Werror now.

Ah, oops. I misread the -Werror in test/Makefile, thinking it was in Makefile and applied to everything. I think the right fix is to put -Werror in the default CFLAGS and remove it from here. -- 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

On Wed, 29 Apr 2026 13:47:13 +1000 David Gibson wrote:

On Tue, Apr 28, 2026 at 09:17:31AM +0200, Stefano Brivio wrote:

...

On Tue, 21 Apr 2026 13:23:29 +1000 David Gibson wrote:

...

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.

I was about to add CFLAGS back to test/build/build.py (see below), but then I realised that this patch actually defeats the purpose of FLAGS, see commit 512f5b1aab2a ("Makefile: Allow define overrides by prepending, not appending, CFLAGS") for the actual reason behind it. That is, with this patch:

...

...

Replace it with the more conventional CFLAGS, which now *can* be reasonable overridden from the command line.

...passing CFLAGS completely overrides the default CFLAGS (instead of prepending them, that is, overriding single existing options). So you can't practically pass (add) "-Werror" anymore, as that will drop stuff like -std=c11, which means one can't use -Werror in build tests. It also drops bits like -pie -fPIE, so distributions (e.g. openSUSE) can't use that to override FORTIFY_SOURCE. When I committed 512f5b1aab2a, I realised that, with the "prepending" semantics, there's no way to completely override / clear CFLAGS, but: - it doesn't seem like a common use case anyway - what passing CFLAGS from the command line does isn't really standardised, and it seems to be common to use it as "extra" CFLAGS Now, it would be possible to introduce something like EXTRA_CFLAGS and ask users and distributions to switch to it, but it needs to happen in two steps: - add EXTRA_CFLAGS and ask distribution maintainers to switch to that - after a reasonable amount of time (I would say six months or so) make command-line CFLAGS replace the default CFLAGS I don't really see a strong motivation to do this though, at the moment.

...

...

Signed-off-by: David Gibson --- Makefile | 21 ++++++++++----------- test/build/build.py | 4 ++-- 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile index e89e5556..1e5f0282 100644 --- a/Makefile +++ b/Makefile @@ -36,8 +36,8 @@ BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ -DVERSION=\"$(VERSION)\" CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)

-FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -O2 -pie -fPIE +WARNINGS = -Wall -Wextra -Wno-format-zero-length -Wformat-security +CFLAGS = -pedantic -std=c11 -O2 -pie -fPIE $(WARNINGS)

PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ @@ -66,7 +66,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) endif

ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0) - FLAGS += -fstack-protector-strong + CFLAGS += -fstack-protector-strong endif

prefix ?= /usr/local @@ -85,7 +85,7 @@ endif

all: $(BIN) $(MANPAGES) docs

-static: FLAGS += -static +static: CFLAGS += -static static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS static: clean all

@@ -96,12 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)

passt: $(PASST_SRCS) $(HEADERS) - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)

-passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops +passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops passt.avx2: $(PASST_SRCS) $(HEADERS) - $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \ - $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS)

passt.avx2: passt

@@ -109,16 +108,16 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@

qrap: $(QRAP_SRCS) passt.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS)

passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ statx readlink -valgrind: FLAGS += -g +valgrind: CFLAGS += -g valgrind: CPPFLAGS += -DVALGRIND valgrind: all

diff --git a/test/build/build.py b/test/build/build.py index e3de8305..7c9cbb44 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -60,7 +60,7 @@ def test_make(target: str, expected_files: list[str]) -> None: with clone_sources(): for p in ex_paths: assert not p.exists(), f"{p} existed before make" - sh(f'make {target} CFLAGS="-Werror"') + sh(f'make {target}') for p in ex_paths: assert p.exists(), f"{p} wasn't made" sh('make clean') @@ -90,7 +90,7 @@ def test_install_uninstall() -> None: progs = ['passt', 'pasta', 'qrap']

# Install - sh(f'make install CFLAGS="-Werror" prefix={prefix}') + sh(f'make install prefix={prefix}')

Here, and above: I don't understand what (if anything) implies -Werror now.

Ah, oops. I misread the -Werror in test/Makefile, thinking it was in Makefile and applied to everything. I think the right fix is to put -Werror in the default CFLAGS and remove it from here.

That's not really doable as it would occasionally break distributions, where specific toolchain or architecture combinations lead quite often to harmless warnings. I can remember dozens of those, and not a single one that was actually a critical problem that made it preferable to have missing packages. It's also very annoying for developers, especially for myself as I often have to run quick tests with different compilers. I would rather add -Werror back to test/build/build.py for the moment being by dropping this patch, and then drop patches that non-trivially depend on this one (due to lack of time, not because I have anything against the other patches, which look good to me except for 13/13). That is, I would reduce this series to the bare minimum that's needed for the "RFC: Dynamic configuration update implementation" series, to avoid blocking progress there. I haven't quite figured out how to do that yet, but that's next on my list unless you get to that first. -- Stefano

On Mon, 4 May 2026 14:47:05 +1000 David Gibson wrote:

On Sun, May 03, 2026 at 11:56:47PM +0200, Stefano Brivio wrote:

...

On Wed, 29 Apr 2026 13:47:13 +1000 David Gibson wrote:

...

On Tue, Apr 28, 2026 at 09:17:31AM +0200, Stefano Brivio wrote:

...

On Tue, 21 Apr 2026 13:23:29 +1000 David Gibson wrote:

...

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.

I was about to add CFLAGS back to test/build/build.py (see below), but then I realised that this patch actually defeats the purpose of FLAGS, see commit 512f5b1aab2a ("Makefile: Allow define overrides by prepending, not appending, CFLAGS") for the actual reason behind it.

That is, with this patch:

...

...

...

Replace it with the more conventional CFLAGS, which now *can* be reasonable overridden from the command line.

...passing CFLAGS completely overrides the default CFLAGS (instead of prepending them, that is, overriding single existing options).

Yes, that was intentional. Fully overriding CFLAGS isn't always the most convenient, but it is how CFLAGS conventionally works in Makefiles, so least surprise and all that.

...

So you can't practically pass (add) "-Werror" anymore, as that will drop stuff like -std=c11, which means one can't use -Werror in build tests.

Right, that's the most awkward consequence that I've spotted. For this case we can neutralize that by including -Werror by default, which I think is worthwhile anyway.

...except we can't do that because of the other reasons I mentioned.

...

It also drops bits like -pie -fPIE, so distributions (e.g. openSUSE) can't use that to override FORTIFY_SOURCE.

Hm, ok. I guess I'm not really sure what the purpose of the -pie -fPIE is, so I'm not clear whether it's essential for the build, or just a sensible default that users/distros could have reason to override.

It's essential for security as it enables usage of ASLR (address space layout randomization) and most distributions turn it on by default (or require it in guidelines these days). We want to avoid that some distributions might miss that by mistake.

I didn't realise it was connected to FORTIFY_SOURCE.

It's not directly connected, but if openSUSE just needs to redefine FORTIFY_SOURCE, then they can't just add it by passing CFLAGS, because that would also disable -pie -fPIE, plus whatever flag we want to have on for security reasons that are independent from a specific distribution.

...

When I committed 512f5b1aab2a, I realised that, with the "prepending" semantics, there's no way to completely override / clear CFLAGS, but:

Right. I think gcc is "last option wins".

Yes, as described in that commit (same for Clang).

...

- it doesn't seem like a common use case anyway

Maybe?

...

- what passing CFLAGS from the command line does isn't really standardised, and it seems to be common to use it as "extra" CFLAGS

Huh.. ok. That's the opposite of the impression I've generally had. Although that said I guess "essential" compiler flags aren't usually in CFLAGS, which could be interpreted as extra flags.

Maybe EXTRA_CFLAGS is more common, but we don't have it...

...

Now, it would be possible to introduce something like EXTRA_CFLAGS and ask users and distributions to switch to it, but it needs to happen in two steps:

Actually, I'd suggest inverting the pattern. Put vital flags in another variable (say BASE_CFLAGS) and have the default CFLAGS contain only flags that are a suggested default build, but can be safely overridden. This is the same pattern I've used for BASE_CPPFLAGS/CPPFLAGS in the earlier patches.

Ah, good idea, that would avoid bothering users and distributions. By the way, I think all current FLAGS are vital flags that should be in BASE_CFLAGS, and one can still override -O or similar by passing CFLAGS if CFLAGS is appended.

I didn't implement that in this patch originally, because I hadn't recognized that anything in FLAGS (other than the -D options I already moved to BASE_CPPFLAGS) was virtal in that sense. But it sounds like I was incorrect about that, so we can change accordingly.

...

- add EXTRA_CFLAGS and ask distribution maintainers to switch to that

- after a reasonable amount of time (I would say six months or so) make command-line CFLAGS replace the default CFLAGS

Splitting BASE_CPPFLAGS/CFLAGS instead of CFLAGS/EXTRA_CFLAGS should make that transitions significantly less awkward.

Right.

...

I don't really see a strong motivation to do this though, at the moment.

...

...

...

Signed-off-by: David Gibson --- Makefile | 21 ++++++++++----------- test/build/build.py | 4 ++-- 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile index e89e5556..1e5f0282 100644 --- a/Makefile +++ b/Makefile @@ -36,8 +36,8 @@ BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ -DVERSION=\"$(VERSION)\" CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)

-FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -O2 -pie -fPIE +WARNINGS = -Wall -Wextra -Wno-format-zero-length -Wformat-security +CFLAGS = -pedantic -std=c11 -O2 -pie -fPIE $(WARNINGS)

PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ @@ -66,7 +66,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) endif

ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0) - FLAGS += -fstack-protector-strong + CFLAGS += -fstack-protector-strong endif

prefix ?= /usr/local @@ -85,7 +85,7 @@ endif

all: $(BIN) $(MANPAGES) docs

-static: FLAGS += -static +static: CFLAGS += -static static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS static: clean all

@@ -96,12 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)

passt: $(PASST_SRCS) $(HEADERS) - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)

-passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops +passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops passt.avx2: $(PASST_SRCS) $(HEADERS) - $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \ - $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS)

passt.avx2: passt

@@ -109,16 +108,16 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@

qrap: $(QRAP_SRCS) passt.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS)

passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ statx readlink -valgrind: FLAGS += -g +valgrind: CFLAGS += -g valgrind: CPPFLAGS += -DVALGRIND valgrind: all

diff --git a/test/build/build.py b/test/build/build.py index e3de8305..7c9cbb44 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -60,7 +60,7 @@ def test_make(target: str, expected_files: list[str]) -> None: with clone_sources(): for p in ex_paths: assert not p.exists(), f"{p} existed before make" - sh(f'make {target} CFLAGS="-Werror"') + sh(f'make {target}') for p in ex_paths: assert p.exists(), f"{p} wasn't made" sh('make clean') @@ -90,7 +90,7 @@ def test_install_uninstall() -> None: progs = ['passt', 'pasta', 'qrap']

# Install - sh(f'make install CFLAGS="-Werror" prefix={prefix}') + sh(f'make install prefix={prefix}')

Here, and above: I don't understand what (if anything) implies -Werror now.

Ah, oops. I misread the -Werror in test/Makefile, thinking it was in Makefile and applied to everything. I think the right fix is to put -Werror in the default CFLAGS and remove it from here.

That's not really doable as it would occasionally break distributions, where specific toolchain or architecture combinations lead quite often to harmless warnings. I can remember dozens of those, and not a single one that was actually a critical problem that made it preferable to have missing packages.

Ah, right. And those distributions don't already override CFLAGS with something that doesn't include -Werror?

We don't have -Werror on. That's just for tests.

...

It's also very annoying for developers, especially for myself as I often have to run quick tests with different compilers.

Ok, fair point.

...

I would rather add -Werror back to test/build/build.py for the moment being by dropping this patch, and then drop patches that non-trivially depend on this one (due to lack of time, not because I have anything against the other patches, which look good to me except for 13/13).

That is, I would reduce this series to the bare minimum that's needed for the "RFC: Dynamic configuration update implementation" series, to avoid blocking progress there. I haven't quite figured out how to do that yet, but that's next on my list unless you get to that first.

I'm still unwell, so unlikely.

Maybe it can all be made simpler by keeping this series substantially as it is with the change you suggested above (essentially, BASE_CFLAGS instead of FLAGS, and CFLAGS adding on top of those). -- Stefano

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). I guess if we can "fix" it (see my concerns about 4/13) quickly, then it's useful, and I'll try to pick it up like that (feel free to post a new version yourself, of course!). But if it needs major reworks I'd rather try to come up with something minimal instead, for the moment. -- Stefano

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.

I guess if we can "fix" it (see my concerns about 4/13) quickly, then it's useful, and I'll try to pick it up like that (feel free to post a new version yourself, of course!).

But if it needs major reworks I'd rather try to come up with something minimal instead, for the moment.

Thanks, Laurent

On Tue, May 05, 2026 at 01:10:30AM +0200, Stefano Brivio wrote:

On Mon, 4 May 2026 14:47:05 +1000 David Gibson wrote:

...

On Sun, May 03, 2026 at 11:56:47PM +0200, Stefano Brivio wrote:

...

On Wed, 29 Apr 2026 13:47:13 +1000 David Gibson wrote:

...

On Tue, Apr 28, 2026 at 09:17:31AM +0200, Stefano Brivio wrote:

...

On Tue, 21 Apr 2026 13:23:29 +1000 David Gibson wrote:

...

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.

I was about to add CFLAGS back to test/build/build.py (see below), but then I realised that this patch actually defeats the purpose of FLAGS, see commit 512f5b1aab2a ("Makefile: Allow define overrides by prepending, not appending, CFLAGS") for the actual reason behind it.

That is, with this patch:

...

...

...

Replace it with the more conventional CFLAGS, which now *can* be reasonable overridden from the command line.

...passing CFLAGS completely overrides the default CFLAGS (instead of prepending them, that is, overriding single existing options).

Yes, that was intentional. Fully overriding CFLAGS isn't always the most convenient, but it is how CFLAGS conventionally works in Makefiles, so least surprise and all that.

...

So you can't practically pass (add) "-Werror" anymore, as that will drop stuff like -std=c11, which means one can't use -Werror in build tests.

Right, that's the most awkward consequence that I've spotted. For this case we can neutralize that by including -Werror by default, which I think is worthwhile anyway.

...except we can't do that because of the other reasons I mentioned.

Yeah, sorry, I hadn't read that far when I wrote this.

...

...

It also drops bits like -pie -fPIE, so distributions (e.g. openSUSE) can't use that to override FORTIFY_SOURCE.

Hm, ok. I guess I'm not really sure what the purpose of the -pie -fPIE is, so I'm not clear whether it's essential for the build, or just a sensible default that users/distros could have reason to override.

It's essential for security as it enables usage of ASLR (address space layout randomization) and most distributions turn it on by default (or require it in guidelines these days).

We want to avoid that some distributions might miss that by mistake.

Ah, ok. Yeah, that sounds essential to me. I think you could also make the argument that -pie is about the type of output we want, more than a "flag" per se, so it doesn't belong in CFLAGS any more than -c or -o would. -fPIE follows as a requirement for -pie.

...

I didn't realise it was connected to FORTIFY_SOURCE.

It's not directly connected, but if openSUSE just needs to redefine FORTIFY_SOURCE, then they can't just add it by passing CFLAGS, because that would also disable -pie -fPIE, plus whatever flag we want to have on for security reasons that are independent from a specific distribution.

Ok, I see.

...

...

When I committed 512f5b1aab2a, I realised that, with the "prepending" semantics, there's no way to completely override / clear CFLAGS, but:

Right. I think gcc is "last option wins".

Yes, as described in that commit (same for Clang).

Ah, yes. I think som of the $(filter) and $(filter-out) confused be in terms of how FLAGS and CFLAGS interact(ed).

...

...

- it doesn't seem like a common use case anyway

Maybe?

...

- what passing CFLAGS from the command line does isn't really standardised, and it seems to be common to use it as "extra" CFLAGS

Huh.. ok. That's the opposite of the impression I've generally had. Although that said I guess "essential" compiler flags aren't usually in CFLAGS, which could be interpreted as extra flags.

Maybe EXTRA_CFLAGS is more common, but we don't have it...

I don't recall EXTRA_CFLAGS being particularly standard, but I'm not very certain.

...

...

Now, it would be possible to introduce something like EXTRA_CFLAGS and ask users and distributions to switch to it, but it needs to happen in two steps:

Actually, I'd suggest inverting the pattern. Put vital flags in another variable (say BASE_CFLAGS) and have the default CFLAGS contain only flags that are a suggested default build, but can be safely overridden. This is the same pattern I've used for BASE_CPPFLAGS/CPPFLAGS in the earlier patches.

Ah, good idea, that would avoid bothering users and distributions. By the way, I think all current FLAGS are vital flags that should be in BASE_CFLAGS,

I'm not really convinced for -Owhatever, -pedantic, -fstack-protectr-strong or the warning options.

and one can still override -O or similar by passing CFLAGS if CFLAGS is appended.

It kind of depends depends what the caller is expecting. Do they expect omitting -Oxyz from CFLAGS to imply -O0, or "project default"?

...

I didn't implement that in this patch originally, because I hadn't recognized that anything in FLAGS (other than the -D options I already moved to BASE_CPPFLAGS) was virtal in that sense. But it sounds like I was incorrect about that, so we can change accordingly.

...

- add EXTRA_CFLAGS and ask distribution maintainers to switch to that

- after a reasonable amount of time (I would say six months or so) make command-line CFLAGS replace the default CFLAGS

Splitting BASE_CPPFLAGS/CFLAGS instead of CFLAGS/EXTRA_CFLAGS should make that transitions significantly less awkward.

Right.

Details about what belongs in BASE_CFLAGS versus CFLAGS can be thrashed out later. The vital thing for this series is to split $(FLAGS) into preprocessor versus compiler flags (with clearer names for both). That's what we need in order to properly invoke the static checkers for both passt and pesto (more particularly, cppcheck needs -DPESTO with pesto and -UPESTO with passt, or log.h will give it conniptions).

...

...

I don't really see a strong motivation to do this though, at the moment.

...

...

...

Signed-off-by: David Gibson --- Makefile | 21 ++++++++++----------- test/build/build.py | 4 ++-- 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile index e89e5556..1e5f0282 100644 --- a/Makefile +++ b/Makefile @@ -36,8 +36,8 @@ BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ -DVERSION=\"$(VERSION)\" CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS)

-FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security -FLAGS += -pedantic -std=c11 -O2 -pie -fPIE +WARNINGS = -Wall -Wextra -Wno-format-zero-length -Wformat-security +CFLAGS = -pedantic -std=c11 -O2 -pie -fPIE $(WARNINGS)

PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ @@ -66,7 +66,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) endif

ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0) - FLAGS += -fstack-protector-strong + CFLAGS += -fstack-protector-strong endif

prefix ?= /usr/local @@ -85,7 +85,7 @@ endif

all: $(BIN) $(MANPAGES) docs

-static: FLAGS += -static +static: CFLAGS += -static static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS static: clean all

@@ -96,12 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS)

passt: $(PASST_SRCS) $(HEADERS) - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS)

-passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops +passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops passt.avx2: $(PASST_SRCS) $(HEADERS) - $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \ - $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS)

passt.avx2: passt

@@ -109,16 +108,16 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% ln -sf $< $@

qrap: $(QRAP_SRCS) passt.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS)

passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ rt_sigreturn getpid gettid kill clock_gettime \ mmap|mmap2 munmap open unlink gettimeofday futex \ statx readlink -valgrind: FLAGS += -g +valgrind: CFLAGS += -g valgrind: CPPFLAGS += -DVALGRIND valgrind: all

diff --git a/test/build/build.py b/test/build/build.py index e3de8305..7c9cbb44 100755 --- a/test/build/build.py +++ b/test/build/build.py @@ -60,7 +60,7 @@ def test_make(target: str, expected_files: list[str]) -> None: with clone_sources(): for p in ex_paths: assert not p.exists(), f"{p} existed before make" - sh(f'make {target} CFLAGS="-Werror"') + sh(f'make {target}') for p in ex_paths: assert p.exists(), f"{p} wasn't made" sh('make clean') @@ -90,7 +90,7 @@ def test_install_uninstall() -> None: progs = ['passt', 'pasta', 'qrap']

# Install - sh(f'make install CFLAGS="-Werror" prefix={prefix}') + sh(f'make install prefix={prefix}')

Here, and above: I don't understand what (if anything) implies -Werror now.

Ah, oops. I misread the -Werror in test/Makefile, thinking it was in Makefile and applied to everything. I think the right fix is to put -Werror in the default CFLAGS and remove it from here.

That's not really doable as it would occasionally break distributions, where specific toolchain or architecture combinations lead quite often to harmless warnings. I can remember dozens of those, and not a single one that was actually a critical problem that made it preferable to have missing packages.

Ah, right. And those distributions don't already override CFLAGS with something that doesn't include -Werror?

We don't have -Werror on. That's just for tests.

...

...

It's also very annoying for developers, especially for myself as I often have to run quick tests with different compilers.

Ok, fair point.

...

I would rather add -Werror back to test/build/build.py for the moment being by dropping this patch, and then drop patches that non-trivially depend on this one (due to lack of time, not because I have anything against the other patches, which look good to me except for 13/13).

That is, I would reduce this series to the bare minimum that's needed for the "RFC: Dynamic configuration update implementation" series, to avoid blocking progress there. I haven't quite figured out how to do that yet, but that's next on my list unless you get to that first.

I'm still unwell, so unlikely.

Maybe it can all be made simpler by keeping this series substantially as it is with the change you suggested above (essentially, BASE_CFLAGS instead of FLAGS, and CFLAGS adding on top of those).

Right, sounds basically the same as my suggestion above. -- 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

On Tue, 21 Apr 2026 13:23:27 +1000 David Gibson 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 --- 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