Skip to content

Commit 10c2ddc

Browse files
mattjalamattjala
authored
Implement fallback for systems without qsort_r (#5931)
Also includes a new workflow to test FreeBSD, which has a different qsort signature and was previously untested Resolves #5896
1 parent 85f7f8f commit 10c2ddc

File tree

7 files changed

+260
-6
lines changed

7 files changed

+260
-6
lines changed

‎.github/workflows/freebsd.yml‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
name: FreeBSD CI
2+
3+
# Triggers the workflow on push or pull request or on demand
4+
on:
5+
workflow_dispatch:
6+
push:
7+
pull_request:
8+
branches: [ develop ]
9+
paths-ignore:
10+
- '.github/CODEOWNERS'
11+
- '.github/FUNDING.yml'
12+
- 'doc/**'
13+
- 'release_docs/**'
14+
- 'ACKNOWLEDGEMENTS'
15+
- 'LICENSE**'
16+
- '**.md'
17+
18+
# Using concurrency to cancel any in-progress job or run
19+
concurrency:
20+
group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }}
21+
cancel-in-progress: true
22+
23+
permissions:
24+
contents: read
25+
26+
jobs:
27+
freebsd-build-and-test:
28+
runs-on: ubuntu-latest
29+
name: FreeBSD ${{ matrix.freebsd-version }} Build and Test
30+
31+
# Don't run the action if the commit message says to skip CI
32+
if: "!contains(github.event.head_commit.message, 'skip-ci')"
33+
34+
strategy:
35+
fail-fast: false
36+
matrix:
37+
freebsd-version: ['13.5', '14.3', '15.0']
38+
39+
steps:
40+
- name: Checkout repository
41+
uses: actions/checkout@v5
42+
43+
- name: Build and test on FreeBSD
44+
uses: vmactions/freebsd-vm@v1
45+
with:
46+
release: ${{ matrix.freebsd-version }}
47+
usesh: true
48+
prepare: |
49+
pkg install -y cmake ninja pkgconf bash curl
50+
run: |
51+
set -e
52+
53+
# Configure the build
54+
mkdir build
55+
cd build
56+
cmake -C ../config/cmake/cacheinit.cmake \
57+
-G Ninja \
58+
--log-level=VERBOSE \
59+
-DCMAKE_BUILD_TYPE=Release \
60+
-DBUILD_SHARED_LIBS:BOOL=ON \
61+
-DHDF5_ENABLE_ALL_WARNINGS:BOOL=ON \
62+
-DHDF5_ENABLE_PARALLEL:BOOL=OFF \
63+
-DHDF5_BUILD_CPP_LIB:BOOL=ON \
64+
-DHDF5_BUILD_FORTRAN:BOOL=OFF \
65+
-DHDF5_BUILD_JAVA:BOOL=OFF \
66+
-DHDF5_BUILD_DOC:BOOL=OFF \
67+
-DHDF5_ENABLE_ZLIB_SUPPORT:BOOL=ON \
68+
-DHDF5_ENABLE_SZIP_SUPPORT:BOOL=ON \
69+
-DLIBAEC_USE_LOCALCONTENT:BOOL=OFF \
70+
-DZLIB_USE_LOCALCONTENT:BOOL=OFF \
71+
-DHDF5_TEST_API:BOOL=ON \
72+
-DHDF5_TEST_SHELL_SCRIPTS:BOOL=OFF \
73+
-DENABLE_EXTENDED_TESTS:BOOL=OFF \
74+
..
75+
echo ""
76+
77+
# Build
78+
cmake --build . --parallel 3 --config Release
79+
echo ""
80+
81+
# Run tests
82+
ctest . --parallel 2 -C Release -V
83+
echo ""

‎.github/workflows/openbsd.yml‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
name: OpenBSD CI
2+
3+
# Triggers the workflow on push or pull request or on demand
4+
on:
5+
workflow_dispatch:
6+
push:
7+
pull_request:
8+
branches: [ develop ]
9+
paths-ignore:
10+
- '.github/CODEOWNERS'
11+
- '.github/FUNDING.yml'
12+
- 'doc/**'
13+
- 'release_docs/**'
14+
- 'ACKNOWLEDGEMENTS'
15+
- 'LICENSE**'
16+
- '**.md'
17+
18+
# Using concurrency to cancel any in-progress job or run
19+
concurrency:
20+
group: ${{ github.workflow }}-${{ github.head_ref && github.ref || github.run_id }}
21+
cancel-in-progress: true
22+
23+
permissions:
24+
contents: read
25+
26+
jobs:
27+
openbsd-build-and-test:
28+
runs-on: ubuntu-latest
29+
name: OpenBSD ${{ matrix.openbsd-version }} Build and Test
30+
31+
# Don't run the action if the commit message says to skip CI
32+
if: "!contains(github.event.head_commit.message, 'skip-ci')"
33+
34+
strategy:
35+
fail-fast: false
36+
matrix:
37+
openbsd-version: ['7.5']
38+
39+
steps:
40+
- name: Checkout repository
41+
uses: actions/checkout@v5
42+
43+
- name: Build and test on OpenBSD
44+
uses: vmactions/openbsd-vm@v1
45+
with:
46+
release: ${{ matrix.openbsd-version }}
47+
usesh: true
48+
prepare: |
49+
echo "https://ftp.openbsd.org/pub/OpenBSD" > /etc/installurl
50+
pkg_add cmake gmake pkgconf curl gcc-11.2.0p11
51+
run: |
52+
set -e
53+
54+
# Set up library path for gcc
55+
export LD_LIBRARY_PATH=/usr/local/lib:$LD_LIBRARY_PATH
56+
57+
# Get number of processors (OpenBSD uses sysctl in /sbin)
58+
NPROC=$(/sbin/sysctl -n hw.ncpu)
59+
echo "Number of processors: $NPROC"
60+
61+
# Configure the build
62+
mkdir build
63+
cd build
64+
cmake -C ../config/cmake/cacheinit.cmake \
65+
--log-level=VERBOSE \
66+
-DCMAKE_BUILD_TYPE=Release \
67+
-DCMAKE_C_COMPILER=egcc \
68+
-DCMAKE_CXX_COMPILER=eg++ \
69+
-DCMAKE_MAKE_PROGRAM=gmake \
70+
-DBUILD_SHARED_LIBS:BOOL=ON \
71+
-DHDF5_ENABLE_ALL_WARNINGS:BOOL=ON \
72+
-DHDF5_ENABLE_PARALLEL:BOOL=OFF \
73+
-DHDF5_BUILD_CPP_LIB:BOOL=OFF \
74+
-DHDF5_BUILD_FORTRAN:BOOL=OFF \
75+
-DHDF5_BUILD_JAVA:BOOL=OFF \
76+
-DHDF5_BUILD_DOC:BOOL=OFF \
77+
-DHDF5_BUILD_HL_LIB:BOOL=OFF \
78+
-DHDF5_ENABLE_ZLIB_SUPPORT:BOOL=ON \
79+
-DHDF5_ENABLE_SZIP_SUPPORT:BOOL=ON \
80+
-DLIBAEC_USE_LOCALCONTENT:BOOL=OFF \
81+
-DZLIB_USE_LOCALCONTENT:BOOL=OFF \
82+
-DHDF5_TEST_API:BOOL=ON \
83+
-DHDF5_TEST_SHELL_SCRIPTS:BOOL=OFF \
84+
-DENABLE_EXTENDED_TESTS:BOOL=OFF \
85+
..
86+
echo ""
87+
88+
# Build
89+
cmake --build . --parallel $NPROC
90+
echo ""
91+
92+
# Run tests
93+
ctest . --parallel $NPROC
94+
echo ""

‎config/ConfigureChecks.cmake‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,13 @@ CHECK_FUNCTION_EXISTS (asprintf ${HDF_PREFIX}_HAVE_ASPRINTF)
443443
CHECK_FUNCTION_EXISTS (vasprintf ${HDF_PREFIX}_HAVE_VASPRINTF)
444444
CHECK_FUNCTION_EXISTS (waitpid ${HDF_PREFIX}_HAVE_WAITPID)
445445

446+
# Check for reentrant qsort variants (qsort_r on Unix/BSD, qsort_s on Windows)
447+
CHECK_FUNCTION_EXISTS (qsort_r _HAVE_QSORT_R_TMP)
448+
CHECK_FUNCTION_EXISTS (qsort_s _HAVE_QSORT_S_TMP)
449+
if (_HAVE_QSORT_R_TMP OR _HAVE_QSORT_S_TMP)
450+
set (${HDF_PREFIX}_HAVE_QSORT_REENTRANT 1)
451+
endif ()
452+
446453
#-----------------------------------------------------------------------------
447454
# sigsetjmp is special; may actually be a macro
448455
#-----------------------------------------------------------------------------

‎src/H5RT.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,9 @@ H5RT__bulk_load(H5RT_node_t *node, int rank, H5RT_leaf_t *leaves, size_t count,
394394
if (prev_sort_dim != rank - 1) {
395395
assert(prev_sort_dim < rank - 1);
396396
sort_dim = prev_sort_dim + 1;
397-
HDqsort_r((void *)leaves, count, sizeof(H5RT_leaf_t), H5RT__leaf_compare, (void *)&sort_dim);
397+
if (H5_UNLIKELY(HDqsort_r((void *)leaves, count, sizeof(H5RT_leaf_t), H5RT__leaf_compare,
398+
(void *)&sort_dim) < 0))
399+
HGOTO_ERROR(H5E_INTERNAL, H5E_CANTSORT, FAIL, "failed to sort R-tree leaves");
398400
}
399401
else {
400402
sort_dim = prev_sort_dim;
@@ -459,6 +461,11 @@ H5RT__bulk_load(H5RT_node_t *node, int rank, H5RT_leaf_t *leaves, size_t count,
459461
* On success, the R-tree takes ownership of the caller-allocated
460462
* leaves array.
461463
*
464+
* NOTE: This routine uses a global variable internally, and
465+
* is therefore not thread-safe. See the 'qsort_r_threadsafe'
466+
* branch of the HDF5 GitHub repository for a beta
467+
* implementation that is threadsafe.
468+
*
462469
* Return: A valid pointer to the new R-tree on success/NULL on failure
463470
*
464471
*-------------------------------------------------------------------------

‎src/H5private.h‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -650,8 +650,13 @@ H5_DLL H5_ATTR_CONST int Nflock(int fd, int operation);
650650
#endif /* HDflock */
651651

652652
#if defined(H5_HAVE_WIN32_API) || defined(H5_HAVE_DARWIN) || (defined(__FreeBSD__) && __FreeBSD__ < 14)
653-
H5_DLL void HDqsort_context(void *base, size_t nel, size_t size,
654-
int (*compar)(const void *, const void *, void *), void *arg);
653+
H5_DLL herr_t HDqsort_context(void *base, size_t nel, size_t size,
654+
int (*compar)(const void *, const void *, void *), void *arg);
655+
#endif
656+
657+
#ifndef H5_HAVE_QSORT_REENTRANT
658+
H5_DLL herr_t HDqsort_fallback(void *base, size_t nel, size_t size,
659+
int (*compar)(const void *, const void *, void *), void *arg);
655660
#endif
656661

657662
#ifndef HDfseek
@@ -770,10 +775,17 @@ H5_DLL void HDqsort_context(void *base, size_t nel, size_t size,
770775
#define HDunsetenv(S) unsetenv(S)
771776
#endif
772777
#ifndef HDqsort_r
773-
#ifdef H5_HAVE_DARWIN
778+
#ifdef H5_HAVE_QSORT_REENTRANT
779+
#if defined(H5_HAVE_DARWIN) || (defined(__FreeBSD__) && __FreeBSD__ < 14)
780+
/* Darwin and FreeBSD < 14 use BSD-style qsort_r with different signature/argument order */
774781
#define HDqsort_r(B, N, S, C, A) HDqsort_context(B, N, S, C, A)
775782
#else
776-
#define HDqsort_r(B, N, S, C, A) qsort_r(B, N, S, C, A)
783+
/* Wrap native GNU qsort_r to vacuously return success */
784+
#define HDqsort_r(B, N, S, C, A) (qsort_r(B, N, S, C, A), SUCCEED)
785+
#endif
786+
#else
787+
/* No native qsort_r/qsort_s available - use fallback implementation */
788+
#define HDqsort_r(B, N, S, C, A) HDqsort_fallback(B, N, S, C, A)
777789
#endif
778790
#endif
779791
#ifndef HDvasprintf

‎src/H5pubconf.h.in‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,9 @@
258258
/* Define to 1 if you have the <pwd.h> header file. */
259259
#cmakedefine H5_HAVE_PWD_H @H5_HAVE_PWD_H@
260260

261+
/* Define to 1 if you have a reentrant qsort function (qsort_r or qsort_s). */
262+
#cmakedefine H5_HAVE_QSORT_REENTRANT @H5_HAVE_QSORT_REENTRANT@
263+
261264
/* Define whether the Read-Only S3 virtual file driver (VFD) should be
262265
compiled */
263266
#cmakedefine H5_HAVE_ROS3_VFD @H5_HAVE_ROS3_VFD@

‎src/H5system.c‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@ HDqsort_context_wrapper_func(void *wrapper_arg, const void *a, const void *b)
14411441
return w->gnu_compar(a, b, w->gnu_arg);
14421442
}
14431443

1444-
void
1444+
herr_t
14451445
HDqsort_context(void *base, size_t nel, size_t size, int (*compar)(const void *, const void *, void *),
14461446
void *arg)
14471447
{
@@ -1454,5 +1454,53 @@ HDqsort_context(void *base, size_t nel, size_t size, int (*compar)(const void *,
14541454
/* Old BSD-style: context parameter comes before comparator function */
14551455
qsort_r(base, nel, size, &wrapper, HDqsort_context_wrapper_func);
14561456
#endif
1457+
return SUCCEED;
14571458
}
14581459
#endif
1460+
1461+
/*
1462+
* HDqsort_fallback - Fallback qsort implementation for platforms without qsort_r/qsort_s
1463+
*
1464+
* This implementation is not threadsafe, since it uses a global variable to store the
1465+
* comparator context, then uses standard qsort(). A beta branch of a threadsafe implementation
1466+
* of these routines may be found in the 'qsort_r_threadsafe' branch of the HDF5 GitHub repository.
1467+
*
1468+
*/
1469+
#ifndef H5_HAVE_QSORT_REENTRANT
1470+
1471+
typedef struct HDqsort_fallback_context_t {
1472+
int (*gnu_compar)(const void *, const void *, void *);
1473+
void *gnu_arg;
1474+
} HDqsort_fallback_context_t;
1475+
1476+
/* Non-threadsafe: use global variable */
1477+
static HDqsort_fallback_context_t *HDqsort_fallback_global_ctx = NULL;
1478+
1479+
static int
1480+
HDqsort_fallback_wrapper(const void *a, const void *b)
1481+
{
1482+
/* Call the original GNU-style comparator with context from global */
1483+
return HDqsort_fallback_global_ctx->gnu_compar(a, b, HDqsort_fallback_global_ctx->gnu_arg);
1484+
}
1485+
1486+
herr_t
1487+
HDqsort_fallback(void *base, size_t nel, size_t size, int (*compar)(const void *, const void *, void *),
1488+
void *arg)
1489+
{
1490+
HDqsort_fallback_context_t ctx;
1491+
1492+
ctx.gnu_compar = compar;
1493+
ctx.gnu_arg = arg;
1494+
1495+
/* Store context in global variable */
1496+
HDqsort_fallback_global_ctx = &ctx;
1497+
1498+
qsort(base, nel, size, HDqsort_fallback_wrapper);
1499+
1500+
/* Clear the global pointer */
1501+
HDqsort_fallback_global_ctx = NULL;
1502+
1503+
return SUCCEED;
1504+
}
1505+
1506+
#endif /* !H5_HAVE_QSORT_REENTRANT */

0 commit comments

Comments
 (0)