Skip to content

CDRIVER-4679 Prefer XSI-compliant strerror_r when available #1350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eramongodb merged 3 commits into from
Jul 24, 2023
Merged

CDRIVER-4679 Prefer XSI-compliant strerror_r when available #1350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b221b80
CDRIVER-4679 Prefer XSI-compliant strerror_r when available
eramongodb Jul 18, 2023
caf670c
Use CheckCSourceCompiles to test for XSI-compliant strerror_r
eramongodb Jul 21, 2023
37abf49
Slight wording improvements
eramongodb Jul 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,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`, etc.
# _XOPEN_SOURCE=700 is required by `strnlen`, `strerror_r`, 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)

Expand Down
38 changes: 35 additions & 3 deletions src/libbson/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@ 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 (TestBigEndian)
include (InstallRequiredSystemLibraries)
include (TestBigEndian)

set (CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${PROJECT_SOURCE_DIR}/build/cmake)

Expand Down Expand Up @@ -83,8 +85,6 @@ 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)
Expand Down Expand Up @@ -119,6 +119,38 @@ 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 ()
Copy link
Member

@jmikola jmikola Jul 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gave up on using feature test macros and instead opted to use CMake's CheckCSourceCompiles module to obtain the definitive truth.

This does seem like the most reliable solution. Since CMake doesn't apply to the PHP driver, I created PHPC-2263 to port this to Autotools.


configure_file (
"${PROJECT_SOURCE_DIR}/src/bson/bson-config.h.in"
"${PROJECT_BINARY_DIR}/src/bson/bson-config.h"
Expand Down
9 changes: 9 additions & 0 deletions src/libbson/src/bson/bson-config.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,13 @@
# 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 */
8 changes: 5 additions & 3 deletions src/libbson/src/bson/bson-error.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,14 @@ bson_strerror_r (int err_code, /* IN */
if (strerror_s (buf, buflen, err_code) != 0) {
ret = buf;
}
#elif defined(__GNUC__) && defined(_GNU_SOURCE)
ret = strerror_r (err_code, buf, buflen);
#else /* XSI strerror_r */
#elif defined(BSON_HAVE_XSI_STRERROR_R)
if (strerror_r (err_code, buf, buflen) == 0) {
ret = buf;
}
#elif defined(_GNU_SOURCE)
ret = strerror_r (err_code, buf, buflen);
#else
#error "Unable to find a supported strerror_r candidate"
#endif

if (!ret) {
Expand Down