[PATCH v10 09/23] pesto, log: Share log.h (but not log.c) with pesto tool

[Stefano Brivio <sbrivio@redhat.com>]

From: David Gibson <david@gibson.dropbear.id.au>

In pesto we're going to want several levels of error/warning messages, much
like passt itself.  Particularly as we start to share mode code between
passt and pesto, we want to use a similar interface to emit those.  However
we don't want to use the same implementation - logging to a file or syslog
doesn't make sense for the command line tool.

To accomplish this loosely share log.h, but not log.c between pesto and
passt.  In fact, an #ifdef means even most of log.h isn't actually shared,
but we do provide similar warn(), die() etc. macros.

This includes the *_perror() variants, which need strerror().  However,
we want to avoid allocations for pesto as we do for passt, and strerror()
allocates in some libc versions.  Therefore, also move our workaround for
this to be shared with pesto.

Reviewed-by: Laurent Vivier <lvivier@redhat.com>
[dwg: Based on changes part of a larger patch by Stefano]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
[sbrivio: Dropped debug_perror_() as it's not used anyway, Laurent was
 asking about its name]
[sbrivio: Fix conflicts in the Makefile caused by the fact that I'm
 not merging a previous series reworking it]
[sbrivio: For some reason, this triggers some unrelated, but valid,
 cppcheck warnings in tap.c and conf.c: fix / suppress them]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 Makefile | 10 +++++-----
 common.h | 32 ++++++++++++++++++++++++++++++++
 conf.c   |  2 +-
 log.h    | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 pasta.c  |  4 ++--
 pesto.c  | 14 ++++----------
 tap.c    | 12 +++++++++++-
 util.h   | 32 --------------------------------
 8 files changed, 107 insertions(+), 52 deletions(-)

diff --git a/Makefile b/Makefile
index 76b9b5c..2639472 100644
--- a/Makefile
+++ b/Makefile
@@ -50,10 +50,10 @@ SRCS = $(PASST_SRCS) $(QRAP_SRCS) $(PASST_REPAIR_SRCS) $(PESTO_SRCS)
 
 MANPAGES = passt.1 pasta.1 pesto.1 qrap.1 passt-repair.1
 
-PASST_HEADERS = arch.h arp.h bitmap.h checksum.h conf.h dhcp.h dhcpv6.h \
-	epoll_ctl.h flow.h fwd.h fwd_rule.h flow_table.h icmp.h icmp_flow.h \
-	inany.h iov.h ip.h isolation.h lineread.h log.h migrate.h ndp.h \
-	netlink.h packet.h passt.h pasta.h pesto.h pcap.h pif.h repair.h \
+PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.h \
+	dhcpv6.h epoll_ctl.h flow.h fwd.h fwd_rule.h flow_table.h icmp.h \
+	icmp_flow.h inany.h iov.h ip.h isolation.h lineread.h log.h migrate.h \
+	ndp.h netlink.h packet.h passt.h pasta.h pcap.h pesto.h pif.h repair.h \
 	serialise.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
@@ -116,7 +116,7 @@ passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h
 	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS)
 
 pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
-	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) $(PESTO_SRCS) -o pesto $(LDFLAGS)
+	$(CC) $(FLAGS) $(CFLAGS) $(CPPFLAGS) -DPESTO $(PESTO_SRCS) -o pesto $(LDFLAGS)
 
 valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction	\
 			    rt_sigreturn getpid gettid kill clock_gettime \
diff --git a/common.h b/common.h
index f3506b4..4251781 100644
--- a/common.h
+++ b/common.h
@@ -21,4 +21,36 @@
 /* FPRINTF() intentionally silences cert-err33-c clang-tidy warnings */
 #define FPRINTF(f, ...)	(void)fprintf(f, __VA_ARGS__)
 
+/*
+ * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror,
+ * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs
+ * getrandom(2) and brk(2) as it allocates memory for the locale-translated
+ * error description, but our seccomp profiles forbid both.
+ *
+ * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get
+ * a static untranslated string. It's a GNU implementation, but also defined by
+ * bionic.
+ *
+ * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries
+ * not defining strerrordesc_np() are expected to provide strerror()
+ * implementations that are simple enough for us to call.
+ */
+__attribute__ ((weak)) const char *strerrordesc_np(int errnum);
+
+/**
+ * strerror_() - strerror() wrapper calling strerrordesc_np() if available
+ * @errnum:	Error code
+ *
+ * Return: error description string
+ */
+static inline const char *strerror_(int errnum)
+{
+	if (strerrordesc_np)
+		return strerrordesc_np(errnum);
+
+	return strerror(errnum);
+}
+
+#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead"
+
 #endif /* COMMON_H */
diff --git a/conf.c b/conf.c
index 0586107..27aded8 100644
--- a/conf.c
+++ b/conf.c
@@ -308,7 +308,7 @@ static void conf_netns_opt(char *netns, const char *arg)
  * @argv:	Command line arguments
  */
 static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
-			  int optind, int argc, char *argv[])
+			  int optind, int argc, const char *argv[])
 {
 	if (*netns && optind != argc)
 		die("Both --netns and PID or command given");
diff --git a/log.h b/log.h
index dbab006..c6befe3 100644
--- a/log.h
+++ b/log.h
@@ -6,8 +6,57 @@
 #ifndef LOG_H
 #define LOG_H
 
-#include <stdarg.h>
 #include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef PESTO
+
+#include <errno.h>
+
+#include "common.h"
+
+extern bool debug_flag;
+
+#define msg(...)							\
+	do {								\
+		FPRINTF(stderr, __VA_ARGS__);				\
+		FPRINTF(stderr, "\n");					\
+	} while (0)
+
+#define msg_perror(...)							\
+	do {								\
+		int errno_ = errno;					\
+		FPRINTF(stderr, __VA_ARGS__);				\
+		FPRINTF(stderr, ": %s\n", strerror_(errno_));		\
+	} while (0)
+
+#define die(...)							\
+	do {								\
+		msg(__VA_ARGS__);					\
+		exit(EXIT_FAILURE);					\
+	} while (0)
+
+#define die_perror(...)							\
+	do {								\
+		msg_perror(__VA_ARGS__);				\
+		exit(EXIT_FAILURE);					\
+	} while (0)
+
+#define warn(...)		msg(__VA_ARGS__)
+#define warn_perror(...)	msg_perror(__VA_ARGS__)
+#define info(...)		msg(__VA_ARGS__)
+#define info_perror(...)	msg_perror(__VA_ARGS__)
+
+#define debug(...)							\
+	do {								\
+		if (debug_flag)						\
+			msg(__VA_ARGS__);				\
+	} while (0)
+
+#else /* !PESTO */
+
+#include <stdarg.h>
 #include <stddef.h>
 #include <syslog.h>
 
@@ -109,4 +158,6 @@ void __openlog(const char *ident, int option, int facility);
 void logfile_init(const char *name, const char *path, size_t size);
 void __setlogmask(int mask);
 
+#endif /* !PESTO */
+
 #endif /* LOG_H */
diff --git a/pasta.c b/pasta.c
index bab945f..4e7ee54 100644
--- a/pasta.c
+++ b/pasta.c
@@ -521,7 +521,7 @@ void pasta_netns_quit_init(const struct ctx *c)
  * @c:		Execution context
  * @inotify_fd:	inotify file descriptor with watch on namespace directory
  */
-void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)
+void pasta_netns_quit_inotify_handler(const struct ctx *c, int inotify_fd)
 {
 	char buf[sizeof(struct inotify_event) + NAME_MAX + 1]
 		__attribute__ ((aligned(__alignof__(struct inotify_event))));
@@ -547,7 +547,7 @@ void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)
  * @c:		Execution context
  * @ref:	epoll reference for timer descriptor
  */
-void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref)
+void pasta_netns_quit_timer_handler(const struct ctx *c, union epoll_ref ref)
 {
 	uint64_t expirations;
 	ssize_t n;
diff --git a/pesto.c b/pesto.c
index 9f2fa5d..f0916e8 100644
--- a/pesto.c
+++ b/pesto.c
@@ -34,18 +34,12 @@
 #include "common.h"
 #include "seccomp_pesto.h"
 #include "pesto.h"
+#include "log.h"
 
-static bool debug_flag = false;
+bool debug_flag = false;
 
 static char stdout_buf[BUFSIZ];
 
-#define die(...)							\
-	do {								\
-		FPRINTF(stderr, __VA_ARGS__);				\
-		FPRINTF(stderr, "\n");					\
-		exit(EXIT_FAILURE);					\
-	} while (0)
-
 /**
  * usage() - Print usage, exit with given status code
  * @name:	Executable name
@@ -99,7 +93,7 @@ int main(int argc, char **argv)
 	 * breaking our seccomp profile.
 	 */
 	if (setvbuf(stdout, stdout_buf, _IOFBF, sizeof(stdout_buf)))
-		die("Failed to set stdout buffer");
+		die_perror("Failed to set stdout buffer");
 
 	do {
 		optname = getopt_long(argc, argv, optstring, options, NULL);
@@ -126,7 +120,7 @@ int main(int argc, char **argv)
 	if (argc - optind != 1)
 		usage(argv[0], stderr, EXIT_FAILURE);
 
-	printf("debug_flag=%d, path=\"%s\"\n", debug_flag, argv[optind]);
+	debug("debug_flag=%d, path=\"%s\"", debug_flag, argv[optind]);
 
 	die("pesto is not implemented yet");
 }
diff --git a/tap.c b/tap.c
index 7d06189..412f437 100644
--- a/tap.c
+++ b/tap.c
@@ -756,6 +756,11 @@ resume:
 
 		if (IN4_IS_ADDR_LOOPBACK(&iph->saddr) ||
 		    IN4_IS_ADDR_LOOPBACK(&iph->daddr)) {
+			/* The scope of sstr and dstr could be in theory reduced
+			 * into the conditional block debug() expands to, but
+			 * it's awkward and unreadable, so ignore this warning.
+			 */
+			/* cppcheck-suppress [variableScope,unmatchedSuppression] */
 			char sstr[INET_ADDRSTRLEN], dstr[INET_ADDRSTRLEN];
 
 			debug("Loopback address on tap interface: %s -> %s",
@@ -929,6 +934,11 @@ resume:
 			continue;
 
 		if (IN6_IS_ADDR_LOOPBACK(saddr) || IN6_IS_ADDR_LOOPBACK(daddr)) {
+			/* The scope of sstr and dstr could be in theory reduced
+			 * into the conditional block debug() expands to, but
+			 * it's awkward and unreadable, so ignore this warning.
+			 */
+			/* cppcheck-suppress [variableScope,unmatchedSuppression] */
 			char sstr[INET6_ADDRSTRLEN], dstr[INET6_ADDRSTRLEN];
 
 			debug("Loopback address on tap interface: %s -> %s",
@@ -1304,7 +1314,7 @@ void tap_handler_pasta(struct ctx *c, uint32_t events,
  * tap_backend_show_hints() - Give help information to start QEMU
  * @c:		Execution context
  */
-static void tap_backend_show_hints(struct ctx *c)
+static void tap_backend_show_hints(const struct ctx *c)
 {
 	switch (c->mode) {
 	case MODE_PASTA:
diff --git a/util.h b/util.h
index 770ff93..e90be47 100644
--- a/util.h
+++ b/util.h
@@ -302,38 +302,6 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)
 
 void raw_random(void *buf, size_t buflen);
 
-/*
- * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror,
- * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs
- * getrandom(2) and brk(2) as it allocates memory for the locale-translated
- * error description, but our seccomp profiles forbid both.
- *
- * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get
- * a static untranslated string. It's a GNU implementation, but also defined by
- * bionic.
- *
- * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries
- * not defining strerrordesc_np() are expected to provide strerror()
- * implementations that are simple enough for us to call.
- */
-__attribute__ ((weak)) const char *strerrordesc_np(int errnum);
-
-/**
- * strerror_() - strerror() wrapper calling strerrordesc_np() if available
- * @errnum:	Error code
- *
- * Return: error description string
- */
-static inline const char *strerror_(int errnum)
-{
-	if (strerrordesc_np)
-		return strerrordesc_np(errnum);
-
-	return strerror(errnum);
-}
-
-#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead"
-
 /*
  * Workarounds for https://github.com/llvm/llvm-project/issues/58992
  *
-- 
2.43.0