CDRIVER-4679 Revert BSON_HAVE_XSI_STRERROR_R and use strerror_l instead (#1370)

* Revert commit ac436db.

* Depend on XSI-compliant strerror_l instead of strerror_r

Author: eramongodb

CMakeLists.txt

@@ -369,7 +369,7 @@ endif ()
 # Enable POSIX features up to POSIX.1-2008 plus the XSI extension and BSD-derived definitions.
 # Both _BSD_SOURCE and _DEFAULT_SOURCE are defined for backwards-compatibility with glibc 2.19 and earlier.
 # _BSD_SOURCE and _DEFAULT_SOURCE are required by `getpagesize`, `h_errno`, etc.
-# _XOPEN_SOURCE=700 is required by `strnlen`, `strerror_r`, etc.
+# _XOPEN_SOURCE=700 is required by `strnlen`, `strerror_l`, etc.
 add_definitions (-D_XOPEN_SOURCE=700 -D_BSD_SOURCE -D_DEFAULT_SOURCE)
 list (APPEND CMAKE_REQUIRED_DEFINITIONS -D_XOPEN_SOURCE=700 -D_BSD_SOURCE -D_DEFAULT_SOURCE)
 

src/libbson/CMakeLists.txt

@@ -14,14 +14,12 @@ include (MongoC-Warnings)
 
 set (BSON_OUTPUT_BASENAME "bson" CACHE STRING "Output bson library base name")
 
-include (CheckCSourceCompiles)
 include (CheckFunctionExists)
 include (CheckIncludeFile)
-include (CheckIncludeFiles)
 include (CheckStructHasMember)
 include (CheckSymbolExists)
-include (InstallRequiredSystemLibraries)
 include (TestBigEndian)
+include (InstallRequiredSystemLibraries)
 
 set (CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${PROJECT_SOURCE_DIR}/build/cmake)
 
@@ -85,6 +83,8 @@ else ()
    set (BSON_OS 1)
 endif ()
 
+include (CheckIncludeFiles)
+
 CHECK_INCLUDE_FILE (strings.h BSON_HAVE_STRINGS_H)
 if (NOT BSON_HAVE_STRINGS_H)
    set (BSON_HAVE_STRINGS_H 0)
@@ -119,38 +119,6 @@ else ()
    set (BSON_BYTE_ORDER 1234)
 endif ()
 
-# 1. Normally, `defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 600` should be enough
-#    to detect if an XSI-compliant `strerror_r` is available.
-# 2. However, GNU provides an incompatible `strerror_r` as an extension,
-#    requiring an additional test for `!defined(_GNU_SOURCE)`.
-# 3. However, musl does not support the GNU extension even when `_GNU_SOURCE` is
-#    defined, preventing `!defined(_GNU_SOURCE)` from being a portable
-#    XSI-compliance test.
-# 4. However, musl does not provide a feature test macro to detect compilation
-#    with musl, and workarounds that use internal glibc feature test macros such
-#    as `defined(__USE_GNU)` are explicitly forbidden by glibc documentation.
-# 5. Therefore, give up on using feature test macros: use `CheckCSourceCompiles`
-#    to test if the current `strerror_r` is XSI-compliant if present.
-if (NOT WIN32)
-   CHECK_C_SOURCE_COMPILES(
-      [[
-         #include <string.h>
-         int test(int errnum, char *buf, size_t buflen) {
-            return strerror_r (errnum, buf, buflen);
-         }
-         int main(void) {}
-      ]]
-      BSON_HAVE_XSI_STRERROR_R
-      FAIL_REGEX "int-conversion"
-   )
-endif ()
-
-if (BSON_HAVE_XSI_STRERROR_R)
-   set(BSON_HAVE_XSI_STRERROR_R 1)
-else()
-   set(BSON_HAVE_XSI_STRERROR_R 0)
-endif ()
-
 configure_file (
    "${PROJECT_SOURCE_DIR}/src/bson/bson-config.h.in"
    "${PROJECT_BINARY_DIR}/src/bson/bson-config.h"

src/libbson/src/bson/bson-config.h.in

@@ -122,13 +122,4 @@
 # undef BSON_HAVE_STRLCPY
 #endif
 
-
-/*
- * Define to 1 if you have an XSI-compliant strerror_r on your platform.
- */
-#define BSON_HAVE_XSI_STRERROR_R @BSON_HAVE_XSI_STRERROR_R@
-#if BSON_HAVE_XSI_STRERROR_R != 1
-# undef BSON_HAVE_XSI_STRERROR_R
-#endif
-
 #endif /* BSON_CONFIG_H */

src/libbson/src/bson/bson-error.c

@@ -25,6 +25,11 @@
 #include "bson-string.h"
 #include "bson-types.h"
 
+// See `bson_strerror_r()` definition below.
+#if !defined(_WIN32) && !defined(__APPLE__)
+#include <locale.h> // uselocale()
+#endif
+
 
 /*
  *--------------------------------------------------------------------------
@@ -98,25 +103,71 @@ bson_set_error (bson_error_t *error, /* OUT */
  */
 
 char *
-bson_strerror_r (int err_code,  /* IN */
-                 char *buf,     /* IN */
-                 size_t buflen) /* IN */
+bson_strerror_r (int err_code,                    /* IN */
+                 char *buf BSON_MAYBE_UNUSED,     /* IN */
+                 size_t buflen BSON_MAYBE_UNUSED) /* IN */
 {
    static const char *unknown_msg = "Unknown error";
    char *ret = NULL;
 
 #if defined(_WIN32)
+   // Windows does not provide `strerror_l` or `strerror_r`, but it does
+   // unconditionally provide `strerror_s`.
    if (strerror_s (buf, buflen, err_code) != 0) {
       ret = buf;
    }
-#elif defined(BSON_HAVE_XSI_STRERROR_R)
-   if (strerror_r (err_code, buf, buflen) == 0) {
-      ret = buf;
+#elif defined(__APPLE__)
+   // Apple does not provide `strerror_l`, but it does unconditionally provide
+   // the XSI-compliant `strerror_r`, but only when compiling with Apple Clang.
+   // GNU extensions may still be a problem if we are being compiled with GCC on
+   // Apple. Avoid the compatibility headaches with GNU extensions and the musl
+   // library by assuming the implementation will not cause UB when reading the
+   // error message string even when `strerror_r` fails, as encouraged (but not
+   // required) by the POSIX spec (see:
+   // https://pubs.opengroup.org/onlinepubs/9699919799/functions/strerror.html#tag_16_574_08).
+   (void) strerror_r (err_code, buf, buflen);
+#elif defined(_XOPEN_SOURCE) && _XOPEN_SOURCE >= 700
+   // The behavior (of `strerror_l`) is undefined if the locale argument to
+   // `strerror_l()` is the special locale object LC_GLOBAL_LOCALE or is not a
+   // valid locale object handle.
+   locale_t locale = uselocale ((locale_t) 0);
+   // No need to test for error (it can only be [EINVAL]).
+   if (locale == LC_GLOBAL_LOCALE) {
+      // Only use our own locale if a thread-local locale was not already set.
+      // This is just to satisfy `strerror_l`. We do NOT want to unconditionally
+      // set a thread-local locale.
+      locale = newlocale (LC_MESSAGES_MASK, "C", (locale_t) 0);
+   }
+   BSON_ASSERT (locale != LC_GLOBAL_LOCALE);
+
+   // Avoid `strerror_r` compatibility headaches with GNU extensions and the
+   // musl library by using `strerror_l` instead. Furthermore, `strerror_r` is
+   // scheduled to be marked as obsolete in favor of `strerror_l` in the
+   // upcoming POSIX Issue 8 (see:
+   // https://www.austingroupbugs.net/view.php?id=655).
+   //
+   // POSIX Spec: since strerror_l() is required to return a string for some
+   // errors, an application wishing to check for all error situations should
+   // set errno to 0, then call strerror_l(), then check errno.
+   if (locale != (locale_t) 0) {
+      errno = 0;
+      ret = strerror_l (err_code, locale);
+
+      if (errno != 0) {
+         ret = NULL;
+      }
+
+      freelocale (locale);
+   } else {
+      // Could not obtain a valid `locale_t` object to satisfy `strerror_l`.
+      // Fallback to `bson_strncpy` below.
    }
 #elif defined(_GNU_SOURCE)
+   // Unlikely, but continue supporting use of GNU extension in cases where the
+   // C Driver is being built without _XOPEN_SOURCE=700.
    ret = strerror_r (err_code, buf, buflen);
 #else
-   #error "Unable to find a supported strerror_r candidate"
+#error "Unable to find a supported strerror_r candidate"
 #endif
 
    if (!ret) {