[PATCH 0/6] Assorted makefile cleanups

[PATCH 0/6] Assorted makefile cleanups

[David Gibson]

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

These are not directly apropose anything, but I encountered a number
of minor uglies in the Makefiles while I was working on other stuff,
so heres a bunch of cleanups.

David Gibson (6):
  Makefile: Avoid using wildcard sources
  Makefile: Use $(BIN) and $(MANPAGES) variable to simplify several
    targets
  Makefile: Simplify pasta* targets with a pattern rule
  Makefile: Tweak $(RM) usage
  Makefile: Don't create extraneous -.s file
  Makefile: Spell prefix as PREFIX

 Makefile   | 77 +++++++++++++++++++++++++++---------------------------
 seccomp.sh |  5 ++--
 2 files changed, 41 insertions(+), 41 deletions(-)

-- 
2.36.1

[PATCH 1/6] Makefile: Avoid using wildcard sources

[David Gibson]

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

The passt/pasta Makefile makes fairly heavy use of GNU make's $(wildcard)
function to locate the sources and headers to build.  Using wildcards for
the things to compile is usually a bad idea though: if somehow you end up
with a .c or .h file in your tree you didn't expect it can misbuild in an
exceedingly confusing way.  In particular this can sometimes happen if
switching between releases / branches where files have been added or
removed without 100% cleaning the tree.

It also makes life a bit complicated if building multiple different
binaries in the same tree: we already have some rather awkward
$(filter-out) constructions to avoid including qrap.c in the passt build.

Replace use of $(wildcard) with the more idiomatic approach of defining
variables listing all the relevant source files then using that throughout.
In the rule for seccomp.h there was also a bare "*.c" which caused make to
always rebuild that target.  Fix that as well.

Similarly, seccomp.sh uses a wildcard to locate the sources, which is
unwise for the same reasons.  Make it take the sources to examine on the
command line instead, and have the Makefile pass them in from the same
variables.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile   | 37 ++++++++++++++++++++++---------------
 seccomp.sh |  5 +++--
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/Makefile b/Makefile
index 9f2ec3a..f7ca3ef 100644
--- a/Makefile
+++ b/Makefile
@@ -31,6 +31,17 @@ CFLAGS += -DPASST_AUDIT_ARCH=AUDIT_ARCH_$(AUDIT_ARCH)
 CFLAGS += -DRLIMIT_STACK_VAL=$(RLIMIT_STACK_VAL)
 CFLAGS += -DARCH=\"$(TARGET_ARCH)\"
 
+PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
+	mld.c ndp.c netlink.c packet.c passt.c pasta.c pcap.c siphash.c \
+	tap.c tcp.c tcp_splice.c udp.c util.c
+QRAP_SRCS = qrap.c
+SRCS = $(PASST_SRCS) $(QRAP_SRCS)
+
+PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
+	ndp.h netlink.h packet.h passt.h pasta.h pcap.h siphash.h \
+	tap.h tcp.h tcp_splice.h udp.h util.h
+HEADERS = $(PASST_HEADERS)
+
 # On gcc 11.2, with -O2 and -flto, tcp_hash() and siphash_20b(), if inlined,
 # seem to be hitting something similar to:
 #	https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78993
@@ -82,18 +93,15 @@ endif
 static: CFLAGS += -static -DGLIBC_NO_STATIC_NSS
 static: clean all
 
-seccomp.h: *.c $(filter-out seccomp.h,$(wildcard *.h))
-	@ EXTRA_SYSCALLS=$(EXTRA_SYSCALLS) ./seccomp.sh
+seccomp.h: $(PASST_SRCS) $(PASST_HEADERS)
+	@ EXTRA_SYSCALLS=$(EXTRA_SYSCALLS) ./seccomp.sh $^
 
-passt: $(filter-out qrap.c,$(wildcard *.c)) \
-	$(filter-out qrap.h,$(wildcard *.h)) seccomp.h
-	$(CC) $(CFLAGS) $(filter-out qrap.c,$(wildcard *.c)) -o passt
+passt: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h
+	$(CC) $(CFLAGS) $(PASST_SRCS) -o passt
 
 passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops
-passt.avx2: $(filter-out qrap.c,$(wildcard *.c)) \
-	$(filter-out qrap.h,$(wildcard *.h)) seccomp.h
-	$(CC) $(filter-out -O2,$(CFLAGS)) $(filter-out qrap.c,$(wildcard *.c)) \
-		-o passt.avx2
+passt.avx2: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h
+	$(CC) $(filter-out -O2,$(CFLAGS)) $(PASST_SRCS) -o passt.avx2
 
 passt.avx2: passt
 
@@ -104,9 +112,8 @@ pasta: passt
 	ln -s passt pasta
 	ln -s passt.1 pasta.1
 
-qrap: qrap.c passt.h
-	$(CC) $(CFLAGS) \
-		qrap.c -o qrap
+qrap: $(QRAP_SRCS) passt.h
+	$(CC) $(CFLAGS) $(QRAP_SRCS) -o qrap
 
 valgrind: EXTRA_SYSCALLS="rt_sigprocmask rt_sigtimedwait rt_sigaction \
 			  getpid gettid kill clock_gettime mmap munmap open \
@@ -203,7 +210,7 @@ pkgs: static
 # - concurrency-mt-unsafe
 #	TODO: check again if multithreading is implemented
 
-clang-tidy: $(wildcard *.c) $(wildcard *.h)
+clang-tidy: $(SRCS) $(HEADERS)
 	clang-tidy -checks=*,-modernize-*,\
 	-clang-analyzer-valist.Uninitialized,\
 	-cppcoreguidelines-init-variables,\
@@ -227,7 +234,7 @@ clang-tidy: $(wildcard *.c) $(wildcard *.h)
 	-altera-struct-pack-align,\
 	-concurrency-mt-unsafe \
 	-config='{CheckOptions: [{key: bugprone-suspicious-string-compare.WarnOnImplicitComparison, value: "false"}]}' \
-	--warnings-as-errors=* $(wildcard *.c) -- $(filter-out -pie,$(CFLAGS))
+	--warnings-as-errors=* $(SRCS) -- $(filter-out -pie,$(CFLAGS))
 
 ifeq ($(shell $(CC) -v 2>&1 | grep -c "gcc version"),1)
 TARGET := $(shell ${CC} -v 2>&1 | sed -n 's/Target: \(.*\)/\1/p')
@@ -237,7 +244,7 @@ EXTRA_INCLUDES_OPT := -I$(EXTRA_INCLUDES)
 else
 EXTRA_INCLUDES_OPT :=
 endif
-cppcheck: $(wildcard *.c) $(wildcard *.h)
+cppcheck: $(SRCS) $(HEADERS)
 	cppcheck --std=c99 --error-exitcode=1 --enable=all --force	\
 	--inconclusive --library=posix					\
 	-I/usr/include $(EXTRA_INCLUDES_OPT)				\
diff --git a/seccomp.sh b/seccomp.sh
index 74eeb4b..17def4d 100755
--- a/seccomp.sh
+++ b/seccomp.sh
@@ -14,6 +14,7 @@
 # Author: Stefano Brivio <sbrivio(a)redhat.com>
 
 TMP="$(mktemp)"
+IN="$@"
 OUT="seccomp.h"
 
 HEADER="/* This file was automatically generated by $(basename ${0}) */
@@ -231,9 +232,9 @@ gen_profile() {
 }
 
 printf '%s\n' "${HEADER}" > "${OUT}"
-__profiles="$(sed -n 's/[\t ]*\*[\t ]*#syscalls:\([^ ]*\).*/\1/p' *.[ch] | sort -u)"
+__profiles="$(sed -n 's/[\t ]*\*[\t ]*#syscalls:\([^ ]*\).*/\1/p' ${IN} | sort -u)"
 for __p in ${__profiles}; do
-	__calls="$(sed -n 's/[\t ]*\*[\t ]*#syscalls\(:'"${__p}"'\|\)[\t ]\{1,\}\(.*\)/\2/p' *.[ch])"
+	__calls="$(sed -n 's/[\t ]*\*[\t ]*#syscalls\(:'"${__p}"'\|\)[\t ]\{1,\}\(.*\)/\2/p' ${IN})"
 	__calls="${__calls} ${EXTRA_SYSCALLS:-}"
 	__calls="$(filter ${__calls})"
 	echo "seccomp profile ${__p} allows: ${__calls}" | tr '\n' ' ' | fmt -t
-- 
@@ -14,6 +14,7 @@
 # Author: Stefano Brivio <sbrivio(a)redhat.com>
 
 TMP="$(mktemp)"
+IN="$@"
 OUT="seccomp.h"
 
 HEADER="/* This file was automatically generated by $(basename ${0}) */
@@ -231,9 +232,9 @@ gen_profile() {
 }
 
 printf '%s\n' "${HEADER}" > "${OUT}"
-__profiles="$(sed -n 's/[\t ]*\*[\t ]*#syscalls:\([^ ]*\).*/\1/p' *.[ch] | sort -u)"
+__profiles="$(sed -n 's/[\t ]*\*[\t ]*#syscalls:\([^ ]*\).*/\1/p' ${IN} | sort -u)"
 for __p in ${__profiles}; do
-	__calls="$(sed -n 's/[\t ]*\*[\t ]*#syscalls\(:'"${__p}"'\|\)[\t ]\{1,\}\(.*\)/\2/p' *.[ch])"
+	__calls="$(sed -n 's/[\t ]*\*[\t ]*#syscalls\(:'"${__p}"'\|\)[\t ]\{1,\}\(.*\)/\2/p' ${IN})"
 	__calls="${__calls} ${EXTRA_SYSCALLS:-}"
 	__calls="$(filter ${__calls})"
 	echo "seccomp profile ${__p} allows: ${__calls}" | tr '\n' ' ' | fmt -t
-- 
2.36.1

[PATCH 2/6] Makefile: Use $(BIN) and $(MANPAGES) variable to simplify several targets

[David Gibson]

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

There are several places which explicitly list the various generated
binaries, even though a $(BIN) variable already lists them.  There are
several more places that list all the manpage files, introduce a
$(MANPAGES) variable to remove that repetition as well.

Tweak the generation of pasta.1 as a link to passt.1 so it's not just made
as a side effect of the pasta target.
---
 Makefile | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/Makefile b/Makefile
index f7ca3ef..88a3f47 100644
--- a/Makefile
+++ b/Makefile
@@ -37,6 +37,8 @@ PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
 QRAP_SRCS = qrap.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS)
 
+MANPAGES = passt.1 pasta.1 qrap.1
+
 PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
 	ndp.h netlink.h packet.h passt.h pasta.h pcap.h siphash.h \
 	tap.h tcp.h tcp_splice.h udp.h util.h
@@ -83,13 +85,13 @@ endif
 prefix ?= /usr/local
 
 ifeq ($(TARGET_ARCH),X86_64)
-all: passt passt.avx2 pasta pasta.avx2 qrap
 BIN := passt passt.avx2 pasta pasta.avx2 qrap
 else
-all: passt pasta qrap
 BIN := passt pasta qrap
 endif
 
+all: $(BIN) $(MANPAGES)
+
 static: CFLAGS += -static -DGLIBC_NO_STATIC_NSS
 static: clean all
 
@@ -110,6 +112,8 @@ pasta.avx2: passt.avx2
 
 pasta: passt
 	ln -s passt pasta
+
+pasta.1: passt.1
 	ln -s passt.1 pasta.1
 
 qrap: $(QRAP_SRCS) passt.h
@@ -123,28 +127,22 @@ valgrind: all
 
 .PHONY: clean
 clean:
-	-${RM} passt passt.avx2 *.o seccomp.h qrap pasta pasta.avx2 pasta.1 \
+	-${RM} $(BIN) *.o seccomp.h pasta.1 \
 		passt.tar passt.tar.gz *.deb *.rpm
 
-install: $(BIN)
+install: $(BIN) $(MANPAGES)
 	mkdir -p $(DESTDIR)$(prefix)/bin $(DESTDIR)$(prefix)/share/man/man1
 	cp -d $(BIN) $(DESTDIR)$(prefix)/bin
-	cp -d passt.1 pasta.1 qrap.1 $(DESTDIR)$(prefix)/share/man/man1
+	cp -d $(MANPAGES) $(DESTDIR)$(prefix)/share/man/man1
 
 uninstall:
-	-${RM} $(DESTDIR)$(prefix)/bin/passt
-	-${RM} $(DESTDIR)$(prefix)/bin/passt.avx2
-	-${RM} $(DESTDIR)$(prefix)/bin/pasta
-	-${RM} $(DESTDIR)$(prefix)/bin/pasta.avx2
-	-${RM} $(DESTDIR)$(prefix)/bin/qrap
-	-${RM} $(DESTDIR)$(prefix)/share/man/man1/passt.1
-	-${RM} $(DESTDIR)$(prefix)/share/man/man1/pasta.1
-	-${RM} $(DESTDIR)$(prefix)/share/man/man1/qrap.1
+	-${RM} $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
+	-${RM} $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
 
 pkgs: static
 	tar cf passt.tar -P --xform 's//\/usr\/bin\//' $(BIN)
 	tar rf passt.tar -P --xform 's//\/usr\/share\/man\/man1\//' \
-		passt.1 pasta.1 qrap.1
+		$(MANPAGES)
 	gzip passt.tar
 	EMAIL="sbrivio(a)redhat.com" fakeroot alien --to-deb \
 		--description="User-mode networking for VMs and namespaces" \
-- 
@@ -37,6 +37,8 @@ PASST_SRCS = arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c icmp.c igmp.c \
 QRAP_SRCS = qrap.c
 SRCS = $(PASST_SRCS) $(QRAP_SRCS)
 
+MANPAGES = passt.1 pasta.1 qrap.1
+
 PASST_HEADERS = arch.h arp.h checksum.h conf.h dhcp.h dhcpv6.h icmp.h \
 	ndp.h netlink.h packet.h passt.h pasta.h pcap.h siphash.h \
 	tap.h tcp.h tcp_splice.h udp.h util.h
@@ -83,13 +85,13 @@ endif
 prefix ?= /usr/local
 
 ifeq ($(TARGET_ARCH),X86_64)
-all: passt passt.avx2 pasta pasta.avx2 qrap
 BIN := passt passt.avx2 pasta pasta.avx2 qrap
 else
-all: passt pasta qrap
 BIN := passt pasta qrap
 endif
 
+all: $(BIN) $(MANPAGES)
+
 static: CFLAGS += -static -DGLIBC_NO_STATIC_NSS
 static: clean all
 
@@ -110,6 +112,8 @@ pasta.avx2: passt.avx2
 
 pasta: passt
 	ln -s passt pasta
+
+pasta.1: passt.1
 	ln -s passt.1 pasta.1
 
 qrap: $(QRAP_SRCS) passt.h
@@ -123,28 +127,22 @@ valgrind: all
 
 .PHONY: clean
 clean:
-	-${RM} passt passt.avx2 *.o seccomp.h qrap pasta pasta.avx2 pasta.1 \
+	-${RM} $(BIN) *.o seccomp.h pasta.1 \
 		passt.tar passt.tar.gz *.deb *.rpm
 
-install: $(BIN)
+install: $(BIN) $(MANPAGES)
 	mkdir -p $(DESTDIR)$(prefix)/bin $(DESTDIR)$(prefix)/share/man/man1
 	cp -d $(BIN) $(DESTDIR)$(prefix)/bin
-	cp -d passt.1 pasta.1 qrap.1 $(DESTDIR)$(prefix)/share/man/man1
+	cp -d $(MANPAGES) $(DESTDIR)$(prefix)/share/man/man1
 
 uninstall:
-	-${RM} $(DESTDIR)$(prefix)/bin/passt
-	-${RM} $(DESTDIR)$(prefix)/bin/passt.avx2
-	-${RM} $(DESTDIR)$(prefix)/bin/pasta
-	-${RM} $(DESTDIR)$(prefix)/bin/pasta.avx2
-	-${RM} $(DESTDIR)$(prefix)/bin/qrap
-	-${RM} $(DESTDIR)$(prefix)/share/man/man1/passt.1
-	-${RM} $(DESTDIR)$(prefix)/share/man/man1/pasta.1
-	-${RM} $(DESTDIR)$(prefix)/share/man/man1/qrap.1
+	-${RM} $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
+	-${RM} $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
 
 pkgs: static
 	tar cf passt.tar -P --xform 's//\/usr\/bin\//' $(BIN)
 	tar rf passt.tar -P --xform 's//\/usr\/share\/man\/man1\//' \
-		passt.1 pasta.1 qrap.1
+		$(MANPAGES)
 	gzip passt.tar
 	EMAIL="sbrivio(a)redhat.com" fakeroot alien --to-deb \
 		--description="User-mode networking for VMs and namespaces" \
-- 
2.36.1

[PATCH 3/6] Makefile: Simplify pasta* targets with a pattern rule

[David Gibson]

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

pasta, pasta.avx2 and pasta.1 are all generated as a link to the
corresponding passt file.  We can consolidate the 3 rules for these targets
into a single pattern rule.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index 88a3f47..760d458 100644
--- a/Makefile
+++ b/Makefile
@@ -107,14 +107,8 @@ passt.avx2: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h
 
 passt.avx2: passt
 
-pasta.avx2: passt.avx2
-	ln -s passt.avx2 pasta.avx2
-
-pasta: passt
-	ln -s passt pasta
-
-pasta.1: passt.1
-	ln -s passt.1 pasta.1
+pasta.avx2 pasta.1 pasta: pasta%: passt%
+	ln -s $< $@
 
 qrap: $(QRAP_SRCS) passt.h
 	$(CC) $(CFLAGS) $(QRAP_SRCS) -o qrap
-- 
@@ -107,14 +107,8 @@ passt.avx2: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h
 
 passt.avx2: passt
 
-pasta.avx2: passt.avx2
-	ln -s passt.avx2 pasta.avx2
-
-pasta: passt
-	ln -s passt pasta
-
-pasta.1: passt.1
-	ln -s passt.1 pasta.1
+pasta.avx2 pasta.1 pasta: pasta%: passt%
+	ln -s $< $@
 
 qrap: $(QRAP_SRCS) passt.h
 	$(CC) $(CFLAGS) $(QRAP_SRCS) -o qrap
-- 
2.36.1

[PATCH 4/6] Makefile: Tweak $(RM) usage

[David Gibson]

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

The use of rm commands in the clean and uninstall targets adds an explicit
leading - to ignore errors.  However the built-in RM variable in make is
actually "rm -f" which already ignores errors, so the - isn't neccessary.

Also replace ${RM} with $(RM) which is the more conventional form in
Makefiles.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 760d458..f918bb8 100644
--- a/Makefile
+++ b/Makefile
@@ -121,7 +121,7 @@ valgrind: all
 
 .PHONY: clean
 clean:
-	-${RM} $(BIN) *.o seccomp.h pasta.1 \
+	$(RM) $(BIN) *.o seccomp.h pasta.1 \
 		passt.tar passt.tar.gz *.deb *.rpm
 
 install: $(BIN) $(MANPAGES)
@@ -130,8 +130,8 @@ install: $(BIN) $(MANPAGES)
 	cp -d $(MANPAGES) $(DESTDIR)$(prefix)/share/man/man1
 
 uninstall:
-	-${RM} $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
-	-${RM} $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
+	$(RM) $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
+	$(RM) $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
 
 pkgs: static
 	tar cf passt.tar -P --xform 's//\/usr\/bin\//' $(BIN)
-- 
@@ -121,7 +121,7 @@ valgrind: all
 
 .PHONY: clean
 clean:
-	-${RM} $(BIN) *.o seccomp.h pasta.1 \
+	$(RM) $(BIN) *.o seccomp.h pasta.1 \
 		passt.tar passt.tar.gz *.deb *.rpm
 
 install: $(BIN) $(MANPAGES)
@@ -130,8 +130,8 @@ install: $(BIN) $(MANPAGES)
 	cp -d $(MANPAGES) $(DESTDIR)$(prefix)/share/man/man1
 
 uninstall:
-	-${RM} $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
-	-${RM} $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
+	$(RM) $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
+	$(RM) $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
 
 pkgs: static
 	tar cf passt.tar -P --xform 's//\/usr\/bin\//' $(BIN)
-- 
2.36.1

[PATCH 5/6] Makefile: Don't create extraneous -.s file

[David Gibson]

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

In order to probe availability of certain features the Makefile test
compiles a handful of tiny snippets, feeding those in from stdin.  However
in one case - the one for -fstack-protector - it forgets to redirect the
output to stdout, meaning it creates a stray '-.s' file when make is
invoked (even make clean).

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index f918bb8..b0dde68 100644
--- a/Makefile
+++ b/Makefile
@@ -78,7 +78,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
 	CFLAGS += -DHAS_GETRANDOM
 endif
 
-ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - >/dev/null 2>&1; echo $$?),0)
+ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
 	CFLAGS += -fstack-protector-strong
 endif
 
-- 
@@ -78,7 +78,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
 	CFLAGS += -DHAS_GETRANDOM
 endif
 
-ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - >/dev/null 2>&1; echo $$?),0)
+ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0)
 	CFLAGS += -fstack-protector-strong
 endif
 
-- 
2.36.1

[PATCH 6/6] Makefile: Spell prefix as PREFIX

[David Gibson]

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

Makefile conventions (at least in the GNUish world) tend to control the
base install directory with the variable $(PREFIX) (all caps).  The passt
Makefile has the same thing but in lower case $(prefix).  Capitalize it to
match conventions.

Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
 Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index b0dde68..c334605 100644
--- a/Makefile
+++ b/Makefile
@@ -82,7 +82,7 @@ ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; ec
 	CFLAGS += -fstack-protector-strong
 endif
 
-prefix ?= /usr/local
+PREFIX ?= /usr/local
 
 ifeq ($(TARGET_ARCH),X86_64)
 BIN := passt passt.avx2 pasta pasta.avx2 qrap
@@ -125,13 +125,13 @@ clean:
 		passt.tar passt.tar.gz *.deb *.rpm
 
 install: $(BIN) $(MANPAGES)
-	mkdir -p $(DESTDIR)$(prefix)/bin $(DESTDIR)$(prefix)/share/man/man1
-	cp -d $(BIN) $(DESTDIR)$(prefix)/bin
-	cp -d $(MANPAGES) $(DESTDIR)$(prefix)/share/man/man1
+	mkdir -p $(DESTDIR)$(PREFIX)/bin $(DESTDIR)$(PREFIX)/share/man/man1
+	cp -d $(BIN) $(DESTDIR)$(PREFIX)/bin
+	cp -d $(MANPAGES) $(DESTDIR)$(PREFIX)/share/man/man1
 
 uninstall:
-	$(RM) $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
-	$(RM) $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
+	$(RM) $(BIN:%=$(DESTDIR)$(PREFIX)/bin/%)
+	$(RM) $(MANPAGES:%=$(DESTDIR)$(PREFIX)/share/man/man1/%)
 
 pkgs: static
 	tar cf passt.tar -P --xform 's//\/usr\/bin\//' $(BIN)
-- 
@@ -82,7 +82,7 @@ ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; ec
 	CFLAGS += -fstack-protector-strong
 endif
 
-prefix ?= /usr/local
+PREFIX ?= /usr/local
 
 ifeq ($(TARGET_ARCH),X86_64)
 BIN := passt passt.avx2 pasta pasta.avx2 qrap
@@ -125,13 +125,13 @@ clean:
 		passt.tar passt.tar.gz *.deb *.rpm
 
 install: $(BIN) $(MANPAGES)
-	mkdir -p $(DESTDIR)$(prefix)/bin $(DESTDIR)$(prefix)/share/man/man1
-	cp -d $(BIN) $(DESTDIR)$(prefix)/bin
-	cp -d $(MANPAGES) $(DESTDIR)$(prefix)/share/man/man1
+	mkdir -p $(DESTDIR)$(PREFIX)/bin $(DESTDIR)$(PREFIX)/share/man/man1
+	cp -d $(BIN) $(DESTDIR)$(PREFIX)/bin
+	cp -d $(MANPAGES) $(DESTDIR)$(PREFIX)/share/man/man1
 
 uninstall:
-	$(RM) $(BIN:%=$(DESTDIR)$(prefix)/bin/%)
-	$(RM) $(MANPAGES:%=$(DESTDIR)$(prefix)/share/man/man1/%)
+	$(RM) $(BIN:%=$(DESTDIR)$(PREFIX)/bin/%)
+	$(RM) $(MANPAGES:%=$(DESTDIR)$(PREFIX)/share/man/man1/%)
 
 pkgs: static
 	tar cf passt.tar -P --xform 's//\/usr\/bin\//' $(BIN)
-- 
2.36.1

Re: [PATCH 1/6] Makefile: Avoid using wildcard sources

[Stefano Brivio]

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

On Tue, 14 Jun 2022 15:12:21 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> The passt/pasta Makefile makes fairly heavy use of GNU make's $(wildcard)
> function to locate the sources and headers to build.  Using wildcards for
> the things to compile is usually a bad idea though: if somehow you end up
> with a .c or .h file in your tree you didn't expect it can misbuild in an
> exceedingly confusing way.  In particular this can sometimes happen if
> switching between releases / branches where files have been added or
> removed without 100% cleaning the tree.

Thanks for fixing this, I have to admit I hit this very issue all the
time ;)

-- 
Stefano

Re: [PATCH 6/6] Makefile: Spell prefix as PREFIX

[Stefano Brivio]

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

On Tue, 14 Jun 2022 15:12:26 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> Makefile conventions (at least in the GNUish world) tend to control the
> base install directory with the variable $(PREFIX) (all caps).  The passt
> Makefile has the same thing but in lower case $(prefix).  Capitalize it to
> match conventions.

I've been wondering about this when I first added it to the Makefile,
and I couldn't really find a rationale for it. From the documentation
of GNU make:

	http://www.gnu.org/software/make/manual/make.html#Directory-Variables
	http://www.gnu.org/software/make/manual/make.html#DESTDIR

prefix is (conventionally) lowercase, DESTDIR is uppercase, so I'd
stick to that. I guess there's just some historic reason behind it. Some
Makefiles of long-standing projects use prefix, some PREFIX.

By the way, the rest of the series all look good to me (and is very
appreciated!).

-- 
Stefano

Re: [PATCH 6/6] Makefile: Spell prefix as PREFIX

[David Gibson]

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

On Tue, Jun 14, 2022 at 04:45:05PM +0200, Stefano Brivio wrote:
> On Tue, 14 Jun 2022 15:12:26 +1000
> David Gibson <david(a)gibson.dropbear.id.au> wrote:
> 
> > Makefile conventions (at least in the GNUish world) tend to control the
> > base install directory with the variable $(PREFIX) (all caps).  The passt
> > Makefile has the same thing but in lower case $(prefix).  Capitalize it to
> > match conventions.
> 
> I've been wondering about this when I first added it to the Makefile,
> and I couldn't really find a rationale for it. From the documentation
> of GNU make:
> 
> 	http://www.gnu.org/software/make/manual/make.html#Directory-Variables
> 	http://www.gnu.org/software/make/manual/make.html#DESTDIR
> 
> prefix is (conventionally) lowercase, DESTDIR is uppercase, so I'd
> stick to that. I guess there's just some historic reason behind it. Some
> Makefiles of long-standing projects use prefix, some PREFIX.

Huh.. interesting.  I guess the things I've looked at personally must
have tended towards PREFIX.  I'd assumed the GNU conventions specified
that, but apparently not.

Ok, let's drop this one.

> By the way, the rest of the series all look good to me (and is very
> appreciated!).

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

Re: [PATCH 2/6] Makefile: Use $(BIN) and $(MANPAGES) variable to simplify several targets

[Stefano Brivio]

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

On Tue, 14 Jun 2022 15:12:22 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> There are several places which explicitly list the various generated
> binaries, even though a $(BIN) variable already lists them.  There are
> several more places that list all the manpage files, introduce a
> $(MANPAGES) variable to remove that repetition as well.

This also needs (sorry ;))

diff --git a/test/distro/debian b/test/distro/debian
index f748dea..239e225 100644
--- a/test/distro/debian
+++ b/test/distro/debian
@@ -39,7 +39,7 @@ endef
 hostb  ./passt -P __PIDFILE__ &
 sleep  1
 host   echo
-hout   GUEST_FILES ls -1 *.c *.h *.sh Makefile | tr '\n' ' '; echo
+hout   GUEST_FILES ls -1 *.c *.h *.sh passt.1 qrap.1 Makefile | tr '\n' ' '; echo
 
 
 test   Debian GNU/Linux 8 (jessie), amd64
diff --git a/test/distro/fedora b/test/distro/fedora
index 7a5eaef..013cb45 100644
--- a/test/distro/fedora
+++ b/test/distro/fedora
@@ -60,7 +60,7 @@ hostb ./passt -P __PIDFILE__ &
 sleep  1
 host   echo
 hout   DNS6 sed -n 's/^nameserver \([^:]*:\)\([^%]*\).*/\1\2/p' /etc/resolv.conf | head -1
-hout   GUEST_FILES ls -1 *.c *.h *.sh Makefile | tr '\n' ' '; echo
+hout   GUEST_FILES ls -1 *.c *.h *.sh passt.1 qrap.1 Makefile | tr '\n' ' '; echo
 
 
 test   Fedora 26, x86_64
diff --git a/test/distro/opensuse b/test/distro/opensuse
index 39f059a..3d71c42 100644
--- a/test/distro/opensuse
+++ b/test/distro/opensuse
@@ -39,7 +39,7 @@ hostb ./passt -P __PIDFILE__ &
 sleep  1
 host   echo
 hout   DNS6 sed -n 's/^nameserver \([^:]*:\)\([^%]*\).*/\1\2/p' /etc/resolv.conf | head -1
-hout   GUEST_FILES ls -1 *.c *.h *.sh Makefile | tr '\n' ' '; echo
+hout   GUEST_FILES ls -1 *.c *.h *.sh passt.1 qrap.1 Makefile | tr '\n' ' '; echo
 
 
 test   OpenSUSE Leap 15.1
diff --git a/test/distro/ubuntu b/test/distro/ubuntu
index c9a2b4d..c3d1630 100644
--- a/test/distro/ubuntu
+++ b/test/distro/ubuntu
@@ -38,7 +38,7 @@ endef
 hostb  ./passt -P __PIDFILE__ &
 sleep  1
 host   echo
-hout   GUEST_FILES ls -1 *.c *.h *.sh Makefile | tr '\n' ' '; echo
+hout   GUEST_FILES ls -1 *.c *.h *.sh passt.1 qrap.1 Makefile | tr '\n' ' '; echo
 
 
 test   Ubuntu 14.04.5 LTS (Trusty Tahr), amd64

...added.

-- 
@@ -38,7 +38,7 @@ endef
 hostb  ./passt -P __PIDFILE__ &
 sleep  1
 host   echo
-hout   GUEST_FILES ls -1 *.c *.h *.sh Makefile | tr '\n' ' '; echo
+hout   GUEST_FILES ls -1 *.c *.h *.sh passt.1 qrap.1 Makefile | tr '\n' ' '; echo
 
 
 test   Ubuntu 14.04.5 LTS (Trusty Tahr), amd64

...added.

-- 
Stefano

Re: [PATCH 0/6] Assorted makefile cleanups

[Stefano Brivio]

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

On Tue, 14 Jun 2022 15:12:20 +1000
David Gibson <david(a)gibson.dropbear.id.au> wrote:

> These are not directly apropose anything, but I encountered a number
> of minor uglies in the Makefiles while I was working on other stuff,
> so heres a bunch of cleanups.
> 
> David Gibson (6):
>   Makefile: Avoid using wildcard sources
>   Makefile: Use $(BIN) and $(MANPAGES) variable to simplify several
>     targets
>   Makefile: Simplify pasta* targets with a pattern rule
>   Makefile: Tweak $(RM) usage
>   Makefile: Don't create extraneous -.s file
>   Makefile: Spell prefix as PREFIX
> 
>  Makefile   | 77 +++++++++++++++++++++++++++---------------------------
>  seccomp.sh |  5 ++--
>  2 files changed, 41 insertions(+), 41 deletions(-)

Series applied, along with the previous ones, thanks!

-- 
Stefano