printf %m is a glibc extension; using it directly can cause odd output on BSD. Most of our uses of %m are via nbdkit wrappers, which will be fixed in a future patch; this was the only %m I could find in a direct call to the printf family, so it doesn't hurt to just open-code it. This particular error is reached early enough in the process that nbdkit_error() is not a viable substitute. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.c b/src/main.c index e80333a..fd6e30e 100644 --- a/src/main.c +++ b/src/main.c @@ -1204,8 +1204,8 @@ get_socket_activation (void) * and we should exit. */ fprintf (stderr, "%s: socket activation: " - "invalid file descriptor fd = %d: %m\n", - program_name, fd); + "invalid file descriptor fd = %d: %s\n", + program_name, fd, strerror(errno)); exit (EXIT_FAILURE); } } -- 2.17.2

printf("%m") is a useful glibc extension, if you are sure that errno is unchanged between the time of the actual problem and your output message; it beats the longhand of writing strerror(errno) yourself. However, BSD libc does not support the extension, and can result in awkward error messages like pread: m instead of an intended pread: Input/output error Solve the problem by probing at configure time if %m works, (this is a runtime test, but can be overridden for testing or cross-compiling by setting the nbdkit_cv_func_printf_percent_m cache variable), and if not, inserting our own wrapper around vfprintf to manually expand a single instance of %m. (Thankfully, it is MUCH easier to do this rewrite for %m, since it does not consume anything from the va_list arg, than it would be for any other % sequence where we'd have to write a full printf parser). As a caller is unlikely to pass multiple %m in a single format string, I didn't bother with replacing a second instance; the documentation updates mention this restriction. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- I have not actually tested on a BSD machine, but did prove to myself that it works on a glibc system with: ./configure nbdkit_cv_func_printf_percent_m=no and comparing gdb traces of default and override behaviors. --- docs/nbdkit-filter.pod | 13 +++++++++---- docs/nbdkit-plugin.pod | 13 +++++++++---- configure.ac | 27 +++++++++++++++++++++++++++ src/internal.h | 6 ++++++ src/log.c | 21 +++++++++++++++++++++ 5 files changed, 72 insertions(+), 8 deletions(-) diff --git a/docs/nbdkit-filter.pod b/docs/nbdkit-filter.pod index 7d5a549..77ecd7b 100644 --- a/docs/nbdkit-filter.pod +++ b/docs/nbdkit-filter.pod @@ -524,7 +524,10 @@ L<printf(3)>: void nbdkit_error (const char *fs, ...); void nbdkit_verror (const char *fs, va_list args); -For convenience, C<nbdkit_error> preserves the value of C<errno>. +For convenience, C<nbdkit_error> preserves the value of C<errno>, and +also supports the glibc extension of a single C<%m> in a format string +expanding to C<strerror(errno)>, even on platforms that don't support +that natively. =head1 DEBUGGING @@ -540,9 +543,11 @@ L<printf(3)>: void nbdkit_debug (const char *fs, ...); void nbdkit_vdebug (const char *fs, va_list args); -For convenience, C<nbdkit_debug> preserves the value of C<errno>. -Note that C<nbdkit_debug> only prints things when the server is in -verbose mode (I<-v> option). +For convenience, C<nbdkit_debug> preserves the value of C<errno>, and +also supports the glibc extension of a single C<%m> in a format string +expanding to C<strerror(errno)>, even on platforms that don't support +that natively. Note that C<nbdkit_debug> only prints things when the +server is in verbose mode (I<-v> option). =head2 Debug Flags diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod index 4754d2c..0f8ef79 100644 --- a/docs/nbdkit-plugin.pod +++ b/docs/nbdkit-plugin.pod @@ -254,7 +254,10 @@ L<printf(3)>: void nbdkit_error (const char *fs, ...); void nbdkit_verror (const char *fs, va_list args); -For convenience, C<nbdkit_error> preserves the value of C<errno>. +For convenience, C<nbdkit_error> preserves the value of C<errno>, and +also supports the glibc extension of a single C<%m> in a format string +expanding to C<strerror(errno)>, even on platforms that don't support +that natively. C<nbdkit_set_error> can be called at any time, but only has an impact during callbacks for serving data, and only when the callback returns @@ -831,9 +834,11 @@ L<printf(3)>: void nbdkit_debug (const char *fs, ...); void nbdkit_vdebug (const char *fs, va_list args); -For convenience, C<nbdkit_debug> preserves the value of C<errno>. -Note that C<nbdkit_debug> only prints things when the server is in -verbose mode (I<-v> option). +For convenience, C<nbdkit_debug> preserves the value of C<errno>, and +also supports the glibc extension of a single C<%m> in a format string +expanding to C<strerror(errno)>, even on platforms that don't support +that natively. Note that C<nbdkit_debug> only prints things when the +server is in verbose mode (I<-v> option). =head2 Debug Flags diff --git a/configure.ac b/configure.ac index 4b1b004..5735921 100644 --- a/configure.ac +++ b/configure.ac @@ -57,6 +57,7 @@ dnl Check for basic C environment. AC_PROG_CC_STDC AC_PROG_INSTALL AC_PROG_CPP +AC_CANONICAL_HOST AC_C_PROTOTYPES test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI compliant]) @@ -173,6 +174,32 @@ AC_CHECK_FUNCS([\ get_current_dir_name \ mkostemp]) +dnl Check whether printf("%m") works +AC_CACHE_CHECK([whether the printf family supports %m], + [nbdkit_cv_func_printf_percent_m], + [AC_RUN_IFELSE( + [AC_LANG_PROGRAM([[ +#include <stdio.h> +#include <string.h> +#include <errno.h> + ]], [[ + char buf[200] = ""; + errno = EINVAL; + snprintf(buf, sizeof buf, "%m"); + return !!strcmp (buf, strerror (EINVAL)); + ]])], + [nbdkit_cv_func_printf_percent_m=yes], + [nbdkit_cv_func_printf_percent_m=no], + [[ + case "$host_os" in + *-gnu* | gnu*) nbdkit_cv_func_printf_percent_m=yes;; + *) nbdkit_cv_func_printf_percent_m="guessing no";; + esac + ]])]) +AS_IF([test "x$nbdkit_cv_func_printf_percent_m" = xyes], + [AC_DEFINE([HAVE_VFPRINTF_PERCENT_M],[1], + [Define to 1 if vfprintf supports %m.])]) + old_LIBS="$LIBS" AC_SEARCH_LIBS([dlopen], [dl dld], [ AS_IF([test "x$ac_cv_search_dlopen" != "xnone required"], diff --git a/src/internal.h b/src/internal.h index 4da8851..29ab843 100644 --- a/src/internal.h +++ b/src/internal.h @@ -148,6 +148,12 @@ extern int crypto_negotiate_tls (struct connection *conn, int sockin, int sockou #define debug nbdkit_debug /* log-*.c */ +#if !HAVE_VFPRINTF_PERCENT_M +#include <stdio.h> +#define vfprintf nbdkit_vfprintf +int __attribute__ ((format (printf, 2, 0))) +nbdkit_vfprintf(FILE *f, const char *fmt, va_list args); +#endif void log_stderr_verror (const char *fs, va_list args); void log_syslog_verror (const char *fs, va_list args); diff --git a/src/log.c b/src/log.c index 148df5f..b314ca0 100644 --- a/src/log.c +++ b/src/log.c @@ -75,3 +75,24 @@ nbdkit_error (const char *fs, ...) nbdkit_verror (fs, args); va_end (args); } + +#if !HAVE_VFPRINTF_PERCENT_M +/* Work around lack of %m in BSD */ +#undef vfprintf + +/* Call the real vfprintf after first changing %m into strerror(errno). */ +int +nbdkit_vfprintf(FILE *f, const char *fmt, va_list args) +{ + char *repl = NULL; + char *p = strstr (fmt, "%m"); /* assume strstr doesn't touch errno */ + int ret; + + if (p && asprintf(&repl, "%.*s%s%s", (int) (p - fmt), fmt, strerror (errno), + p + 2) > 0) + fmt = repl; + ret = vfprintf (f, fmt, args); + free (repl); + return ret; +} +#endif -- 2.17.2