Re: [PATCH v6 09/18] pesto, log: Share log.h (but not log.c) with pesto tool

On Mon, 4 May 2026 11:49:06 +0200 Laurent Vivier wrote:

On 5/3/26 23:55, Stefano Brivio wrote:

...

From: David Gibson

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.

Signed-off-by: Stefano Brivio [dwg: Based on changes part of a larger patch by Stefano] Signed-off-by: David Gibson

Reviewed-by: Laurent Vivier

One little nit below

...

--- Makefile | 6 +++++- common.h | 32 ++++++++++++++++++++++++++++++ log.h | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- pesto.c | 14 ++++---------- util.h | 32 ------------------------------ 5 files changed, 99 insertions(+), 44 deletions(-)

diff --git a/Makefile b/Makefile index 030681b..f6cec8a 100644 --- a/Makefile +++ b/Makefile @@ -61,7 +61,7 @@ PASST_HEADERS = arch.h arp.h bitmap.h checksum.h common.h conf.h dhcp.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 -PESTO_HEADERS = common.h pesto.h +PESTO_HEADERS = common.h pesto.h log.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) @@ -121,6 +121,7 @@ qrap: $(QRAP_SRCS) $(QRAP_HEADERS)

passt-repair: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto: BASE_CPPFLAGS += -DPESTO pesto: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ @@ -221,9 +222,12 @@ cppcheck: passt.cppcheck passt-repair.cppcheck pesto.cppcheck qrap.cppcheck %.cppcheck: $(CPPCHECK) $(CPPCHECK_FLAGS) $(BASE_CPPFLAGS) $^

+passt.cppcheck: BASE_CPPFLAGS += -UPESTO passt.cppcheck: $(PASST_SRCS) $(PASST_HEADERS) seccomp.h + passt-repair.cppcheck: $(PASST_REPAIR_SRCS) $(PASST_REPAIR_HEADERS) seccomp_repair.h

+pesto.cppcheck: BASE_CPPFLAGS += -DPESTO pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unmatchedSuppression pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h

diff --git a/common.h b/common.h index a9c115a..2f2e6f1 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/log.h b/log.h index dbab006..1058ca5 100644 --- a/log.h +++ b/log.h @@ -6,8 +6,63 @@ #ifndef LOG_H #define LOG_H

-#include #include +#include +#include + +#ifdef PESTO + +#include + +#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) + +#define debug_perror_(...) \

Why is this "debug_perror_()" and not "debug_perror()"?

I'm not sure, but it's not used anyway, so I dropped it altogether in v7.

...

+ do { \ + if (debug_flag) \ + msg_perror(__VA_ARGS__); \ + } while (0) + +#else /* !PESTO */

-- Stefano