xfs_scrub: fix strerror_r usage yet again

In commit 75faf2bc907584, someone tried to fix scrub to use the POSIX
version of strerror_r so that the build would work with musl.
Unfortunately, neither the author nor myself remembered that GNU libc
imposes its own version any time _GNU_SOURCE is defined, which
builddefs.in always does.  Regrettably, the POSIX and GNU versions have
different return types and the GNU version can return any random
pointer, so now this code is broken on glibc.

"Fix" this standards body own goal by casting the return value to
intptr_t and employing some gross heuristics to guess at the location of
the actual error string.

Fixes: 75faf2bc907584 ("xfs_scrub: Use POSIX-conformant strerror_r")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: A. Wilcox <AWilcox@Wilcox-Tech.com>

diff --git a/configure.ac b/configure.ac
index 0ba371c3314704ab4e112ecdb505b97d958b2834..7a670f4be99fd4304c4286f36b1b36a7b81a2a1c 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -181,6 +181,7 @@ AC_CONFIG_CROND_DIR
 AC_CONFIG_UDEV_RULE_DIR
 AC_HAVE_BLKID_TOPO
 AC_HAVE_TRIVIAL_AUTO_VAR_INIT
+AC_STRERROR_R_RETURNS_STRING
 
 if test "$enable_ubsan" = "yes" || test "$enable_ubsan" = "probe"; then
         AC_PACKAGE_CHECK_UBSAN

diff --git a/include/builddefs.in b/include/builddefs.in
index b5aa1640711b64133ee929a96f4618a1d00dce1d..b38a099b7d525ab8fdc8aa17743831fba15e0de3 100644 (file)
--- a/include/builddefs.in
+++ b/include/builddefs.in
@@ -117,6 +117,7 @@ CROND_DIR = @crond_dir@
 HAVE_UDEV = @have_udev@
 UDEV_RULE_DIR = @udev_rule_dir@
 HAVE_LIBURCU_ATOMIC64 = @have_liburcu_atomic64@
+STRERROR_R_RETURNS_STRING = @strerror_r_returns_string@
 
 GCCFLAGS = -funsigned-char -fno-strict-aliasing -Wall
 #         -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-decl

diff --git a/m4/package_libcdev.m4 b/m4/package_libcdev.m4
index ce1ba47264659c252c647181a5da926c7396fa3c..c5538c30d2518a8871c19464559696ba7b7c1ea3 100644 (file)
--- a/m4/package_libcdev.m4
+++ b/m4/package_libcdev.m4
@@ -301,3 +301,49 @@ syscall(__NR_file_getattr, 0, 0, 0, 0, 0);
        AC_MSG_RESULT(no))
     AC_SUBST(have_file_getattr)
   ])
+
+#
+# Check if strerror_r returns an int, as opposed to a char *, because there are
+# two versions of this function, with differences that are hard to detect.
+#
+# GNU strerror_r returns a pointer to a string on success, but the returned
+# pointer might point to a static buffer and not buf, so you have to use the
+# return value.  The declaration has the __warn_unused_result__ attribute to
+# enforce this.
+#
+# XSI strerror_r always writes to buf and returns 0 on success, -1 on error.
+#
+# How do you select a particular version?  By defining macros, of course!
+# _GNU_SOURCE always gets you the GNU version, and _POSIX_C_SOURCE >= 200112L
+# gets you the XSI version but only if _GNU_SOURCE isn't defined.
+#
+# The build system #defines _GNU_SOURCE unconditionally, so when compiling
+# against glibc we get the GNU version.  However, when compiling against musl,
+# the _GNU_SOURCE definition does nothing and we get the XSI version anyway.
+# Not definining _GNU_SOURCE breaks the build in many areas, so we'll create
+# yet another #define for just this weird quirk so that we can patch around it
+# in the one place we need it.
+#
+# Note that we have to force erroring out on the int conversion warnings
+# because C doesn't consider it a hard error to cast a char pointer to an int
+# even when CFLAGS contains -std=gnu11.
+AC_DEFUN([AC_STRERROR_R_RETURNS_STRING],
+  [AC_MSG_CHECKING([if strerror_r returns char *])
+    OLD_CFLAGS="$CFLAGS"
+    CFLAGS="$CFLAGS -Wall -Werror"
+    AC_LINK_IFELSE(
+    [AC_LANG_PROGRAM([[
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <string.h>
+  ]], [[
+char buf[1024];
+puts(strerror_r(0, buf, sizeof(buf)));
+  ]])
+    ],
+       strerror_r_returns_string=yes
+       AC_MSG_RESULT(yes),
+       AC_MSG_RESULT(no))
+    CFLAGS="$OLD_CFLAGS"
+    AC_SUBST(strerror_r_returns_string)
+  ])

diff --git a/scrub/Makefile b/scrub/Makefile
index 3636a47942e98e45e91266a6b6c99c465c90f342..6375d77a291bcb5c65b7a5ebca8ae43ee839ef07 100644 (file)
--- a/scrub/Makefile
+++ b/scrub/Makefile
@@ -105,6 +105,10 @@ CFILES += unicrash.c
 LCFLAGS += -DHAVE_LIBICU $(LIBICU_CFLAGS)
 endif
 
+ifeq ($(STRERROR_R_RETURNS_STRING),yes)
+LCFLAGS += -DSTRERROR_R_RETURNS_STRING
+endif
+
 # Automatically trigger a media scan once per month
 XFS_SCRUB_ALL_AUTO_MEDIA_SCAN_INTERVAL=1mo
 

diff --git a/scrub/common.c b/scrub/common.c
index 9437d0abb8698b437a2f64b75c8a67ab4cfc98c7..9a33e2a9d54ed42cf422e70335e2a09a2ab95a8f 100644 (file)
--- a/scrub/common.c
+++ b/scrub/common.c
@@ -126,8 +126,12 @@ __str_out(
        fprintf(stream, "%s%s: %s: ", stream_start(stream),
                        _(err_levels[level].string), descr);
        if (error) {
-               strerror_r(error, buf, DESCR_BUFSZ);
-               fprintf(stream, _("%s."), buf);
+#ifdef STRERROR_R_RETURNS_STRING
+               fprintf(stream, _("%s."), strerror_r(error, buf, DESCR_BUFSZ));
+#else
+               if (strerror_r(error, buf, DESCR_BUFSZ) == 0)
+                       fprintf(stream, _("%s."), buf);
+#endif
        } else {
                va_start(args, format);
                vfprintf(stream, format, args);