Skip to content

Commit

Permalink
Clean up random number generator code (#4338)
Browse files Browse the repository at this point in the history
* Clean up random number generator code

Depending on the platform, we use a mix of random, rand, and rand_r
to generate pseudo-random numbers, along with a messy set of ifdefs
in H5private.h. We are not a cryptographic library, only use random
numbers in our test code, and have no need for anything more than the
C standard's (s)rand(). There's no point dithering about using rand()
vs random() when we're also doing bad things like using mod to
restrict the range, which introduces bias.

Also removes CMake/configure checks for rand_r and random

* Remove random/rand_r checks from build system

* Fix missed HDrandom after GitHub merge
  • Loading branch information
derobins authored and lrknox committed Apr 22, 2024
1 parent faf9cf3 commit 58e4d08
Show file tree
Hide file tree
Showing 48 changed files with 236 additions and 334 deletions.
2 changes: 0 additions & 2 deletions config/cmake/ConfigureChecks.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -408,8 +408,6 @@ CHECK_FUNCTION_EXISTS (getrusage ${HDF_PREFIX}_HAVE_GETRUSAGE)

CHECK_FUNCTION_EXISTS (pread ${HDF_PREFIX}_HAVE_PREAD)
CHECK_FUNCTION_EXISTS (pwrite ${HDF_PREFIX}_HAVE_PWRITE)
CHECK_FUNCTION_EXISTS (rand_r ${HDF_PREFIX}_HAVE_RAND_R)
CHECK_FUNCTION_EXISTS (random ${HDF_PREFIX}_HAVE_RANDOM)

CHECK_FUNCTION_EXISTS (strcasestr ${HDF_PREFIX}_HAVE_STRCASESTR)
CHECK_FUNCTION_EXISTS (strdup ${HDF_PREFIX}_HAVE_STRDUP)
Expand Down
6 changes: 0 additions & 6 deletions config/cmake/H5pubconf.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,6 @@
/* Define to 1 if you have the <quadmath.h> header file. */
#cmakedefine H5_HAVE_QUADMATH_H @H5_HAVE_QUADMATH_H@

/* Define to 1 if you have the `random' function. */
#cmakedefine H5_HAVE_RANDOM @H5_HAVE_RANDOM@

/* Define to 1 if you have the `rand_r' function. */
#cmakedefine H5_HAVE_RAND_R @H5_HAVE_RAND_R@

/* Define whether the Read-Only S3 virtual file driver (VFD) should be
compiled */
#cmakedefine H5_HAVE_ROS3_VFD @H5_HAVE_ROS3_VFD@
Expand Down
1 change: 0 additions & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -2393,7 +2393,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
AC_SEARCH_LIBS([clock_gettime], [rt posix4])
AC_CHECK_FUNCS([asprintf clock_gettime fcntl flock fork])
AC_CHECK_FUNCS([gethostname getrusage gettimeofday])
AC_CHECK_FUNCS([rand_r random])
AC_CHECK_FUNCS([strcasestr strdup symlink])
AC_CHECK_FUNCS([tmpfile vasprintf waitpid])

Expand Down
40 changes: 0 additions & 40 deletions src/H5private.h
Original file line number Diff line number Diff line change
Expand Up @@ -786,46 +786,6 @@ H5_DLL H5_ATTR_CONST int Nflock(int fd, int operation);
#ifndef HDpwrite
#define HDpwrite(F, B, C, O) pwrite(F, B, C, O)
#endif

/* clang-format off */
#ifdef H5_HAVE_RAND_R
# ifndef HDrandom
# define HDrandom() HDrand()
# endif
H5_DLL int HDrand(void);
# ifndef HDsrandom
# define HDsrandom(S) HDsrand(S)
# endif
H5_DLL void HDsrand(unsigned int seed);
#elif defined(H5_HAVE_RANDOM)
# ifndef HDrand
# define HDrand() random()
# endif
# ifndef HDrandom
# define HDrandom() random()
# endif
# ifndef HDsrand
# define HDsrand(S) srandom(S)
# endif
# ifndef HDsrandom
# define HDsrandom(S) srandom(S)
# endif
#else
# ifndef HDrand
# define HDrand() rand()
# endif
# ifndef HDrandom
# define HDrandom() rand()
# endif
# ifndef HDsrand
# define HDsrand(S) srand(S)
# endif
# ifndef HDsrandom
# define HDsrandom(S) srand(S)
# endif
#endif
/* clang-format on */

#ifndef HDread
#define HDread(F, M, Z) read(F, M, Z)
#endif
Expand Down
33 changes: 0 additions & 33 deletions src/H5system.c
Original file line number Diff line number Diff line change
Expand Up @@ -93,39 +93,6 @@ HDvasprintf(char **bufp, const char *fmt, va_list _ap)
}
#endif /* H5_HAVE_VASPRINTF */

/*-------------------------------------------------------------------------
* Function: HDrand/HDsrand
*
* Purpose: Wrapper function for rand. If rand_r exists on this system,
* use it.
*
* Wrapper function for srand. If rand_r is available, it will keep
* track of the seed locally instead of using srand() which modifies
* global state and can break other programs.
*
* Return: Success: Random number from 0 to RAND_MAX
*
* Failure: Cannot fail.
*
*-------------------------------------------------------------------------
*/
#ifdef H5_HAVE_RAND_R

static unsigned int g_seed = 42;

int
HDrand(void)
{
return rand_r(&g_seed);
}

void
HDsrand(unsigned int seed)
{
g_seed = seed;
}
#endif /* H5_HAVE_RAND_R */

/*-------------------------------------------------------------------------
* Function: Pflock
*
Expand Down
8 changes: 4 additions & 4 deletions test/accum.c
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,7 @@ test_random_write(H5F_t *f)
/* seed = (unsigned)1155438845; */
fprintf(stderr, "Random # seed was: %u\n", seed);
#endif
HDsrandom(seed);
srand(seed);

/* Allocate space for the segment length buffer */
off = (size_t *)malloc(MAX_RANDOM_SEGMENTS * sizeof(size_t));
Expand All @@ -1940,8 +1940,8 @@ fprintf(stderr, "Random # seed was: %u\n", seed);

/* Choose random length of segment, allowing for variance */
do {
length += (size_t)(HDrandom() % RAND_SEG_LEN) + 1;
} while ((HDrandom() & 256) >= 128); /* end while */
length += (size_t)(rand() % RAND_SEG_LEN) + 1;
} while ((rand() & 256) >= 128); /* end while */

/* Check for going off end of buffer */
if ((cur_off + length) > RANDOM_BUF_SIZE)
Expand Down Expand Up @@ -1972,7 +1972,7 @@ fprintf(stderr, "Random # seed was: %u\n", seed);
size_t tmp; /* Temporary holder for offset & length values */

/* Choose value within next few elements to to swap with */
swap = ((size_t)HDrandom() % 8) + u;
swap = ((size_t)rand() % 8) + u;
if (swap >= nsegments)
swap = nsegments - 1;

Expand Down
4 changes: 2 additions & 2 deletions test/app_ref.c
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* 1 to MAX_NINC). Assumes integers i and ninc are in scope. */
#define RAND_INC(id) \
do { \
ninc = (HDrand() % MAX_NINC) + 1; \
ninc = (rand() % MAX_NINC) + 1; \
\
for (i = 0; i < ninc; i++) \
if (H5Iinc_ref(ids[id]) != i + 2) \
Expand Down Expand Up @@ -89,7 +89,7 @@ main(void)
h5_reset();
h5_fixname(FILENAME[0], H5P_DEFAULT, filename, sizeof filename);

HDsrand((unsigned)time(NULL));
srand((unsigned)time(NULL));

TESTING("library shutdown with reference count > 1");

Expand Down
6 changes: 3 additions & 3 deletions test/big.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ randll(hsize_t limit, int current_index)
/* Generate up to MAX_TRIES random numbers until one of them */
/* does not overlap with any previous writes */
while (overlap != 0 && tries < MAX_TRIES) {
acc = (hsize_t)HDrandom();
acc *= (hsize_t)HDrandom();
acc = (hsize_t)rand();
acc *= (hsize_t)rand();
acc = acc % limit;
overlap = 0;

Expand Down Expand Up @@ -757,7 +757,7 @@ main(int ac, char **av)
/* seed = (unsigned long)1155438845; */
fprintf(stderr, "Random # seed was: %lu\n", seed);
#endif
HDsrandom((unsigned)seed);
srand((unsigned)seed);

/* run VFD-specific test */
if (H5FD_SEC2 == driver) {
Expand Down
32 changes: 16 additions & 16 deletions test/bittests.c
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ test_copy(void)
TESTING("bit copy operations");

for (i = 0; i < NTESTS; i++) {
s_offset = (size_t)HDrand() % (8 * sizeof v1);
d_offset = (size_t)HDrand() % (8 * sizeof v2);
size = (unsigned)HDrand() % MIN(8 * sizeof(v1), 8 * sizeof(v2));
s_offset = (size_t)rand() % (8 * sizeof v1);
d_offset = (size_t)rand() % (8 * sizeof v2);
size = (unsigned)rand() % MIN(8 * sizeof(v1), 8 * sizeof(v2));
size = MIN3(size, 8 * sizeof(v1) - s_offset, 8 * sizeof(v2) - d_offset);
memset(v1, 0xff, sizeof v1);
memset(v2, 0x00, sizeof v2);
Expand Down Expand Up @@ -277,12 +277,12 @@ test_shift(void)
TESTING("bit shift operations");

for (i = 0; i < NTESTS; i++) {
offset = (size_t)HDrand() % (8 * sizeof vector);
size = (size_t)HDrand() % (8 * sizeof(vector) - offset);
offset = (size_t)rand() % (8 * sizeof vector);
size = (size_t)rand() % (8 * sizeof(vector) - offset);
/* Don't want size to be 0 */
if (size == 0)
continue;
shift_dist = (ssize_t)((size_t)HDrand() % size);
shift_dist = (ssize_t)((size_t)rand() % size);

/*-------- LEFT-shift some bits and make sure something was shifted --------*/
memset(vector, 0x00, sizeof vector);
Expand Down Expand Up @@ -411,8 +411,8 @@ test_increment(void)
TESTING("bit increment operations");

for (i = 0; i < NTESTS; i++) {
offset = (size_t)HDrand() % (8 * sizeof vector);
size = (size_t)HDrand() % (8 * sizeof(vector) - offset);
offset = (size_t)rand() % (8 * sizeof vector);
size = (size_t)rand() % (8 * sizeof(vector) - offset);
/* Don't want size to be 0 */
if (size == 0)
continue;
Expand Down Expand Up @@ -497,8 +497,8 @@ test_decrement(void)
TESTING("bit decrement operations");

for (i = 0; i < NTESTS; i++) {
offset = (size_t)HDrand() % (8 * sizeof vector);
size = (size_t)HDrand() % (8 * sizeof(vector) - offset);
offset = (size_t)rand() % (8 * sizeof vector);
size = (size_t)rand() % (8 * sizeof(vector) - offset);
/* Don't want size to be 0 */
if (size == 0)
continue;
Expand Down Expand Up @@ -566,8 +566,8 @@ test_negate(void)
TESTING("bit negate operations");

for (i = 0; i < NTESTS; i++) {
offset = (size_t)HDrand() % (8 * sizeof vector);
size = (size_t)HDrand() % (8 * sizeof(vector) - offset);
offset = (size_t)rand() % (8 * sizeof vector);
size = (size_t)rand() % (8 * sizeof(vector) - offset);
/* Don't want size to be 0 */
if (size == 0)
continue;
Expand Down Expand Up @@ -665,8 +665,8 @@ test_set(void)
TESTING("bit set operations");

for (i = 0; i < NTESTS; i++) {
d_offset = (size_t)HDrand() % (8 * sizeof v2);
size = (size_t)HDrand() % (8 * sizeof(v2));
d_offset = (size_t)rand() % (8 * sizeof v2);
size = (size_t)rand() % (8 * sizeof(v2));
size = MIN(size, 8 * sizeof(v2) - d_offset);
memset(v2, 0x00, sizeof v2);

Expand Down Expand Up @@ -780,8 +780,8 @@ test_clear(void)
TESTING("bit clear operations");

for (i = 0; i < NTESTS; i++) {
d_offset = (size_t)HDrand() % (8 * sizeof v2);
size = (size_t)HDrand() % (8 * sizeof(v2));
d_offset = (size_t)rand() % (8 * sizeof v2);
size = (size_t)rand() % (8 * sizeof(v2));
size = MIN(size, 8 * sizeof(v2) - d_offset);
memset(v2, 0xff, sizeof v2);

Expand Down
24 changes: 12 additions & 12 deletions test/btree2.c
Original file line number Diff line number Diff line change
Expand Up @@ -2908,7 +2908,7 @@ test_insert_lots(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t
curr_time=1109170019;
fprintf(stderr,"curr_time=%lu\n",(unsigned long)curr_time);
#endif
HDsrandom((unsigned)curr_time);
srand((unsigned)curr_time);

/*
* Test inserting many records into v2 B-tree
Expand All @@ -2925,7 +2925,7 @@ fprintf(stderr,"curr_time=%lu\n",(unsigned long)curr_time);

/* Shuffle record #'s */
for (u = 0; u < INSERT_MANY; u++) {
swap_idx = ((unsigned)HDrandom() % (INSERT_MANY - u)) + u;
swap_idx = ((unsigned)rand() % (INSERT_MANY - u)) + u;
temp_rec = records[u];
records[u] = records[swap_idx];
records[swap_idx] = temp_rec;
Expand Down Expand Up @@ -3015,7 +3015,7 @@ fprintf(stderr,"curr_time=%lu\n",(unsigned long)curr_time);
/* Find random records */
for (u = 0; u < FIND_MANY; u++) {
/* Pick random record */
idx = (hsize_t)(HDrandom() % INSERT_MANY);
idx = (hsize_t)(rand() % INSERT_MANY);

/* Attempt to find existent record in root of level-4 B-tree */
found = false;
Expand Down Expand Up @@ -3046,7 +3046,7 @@ fprintf(stderr,"curr_time=%lu\n",(unsigned long)curr_time);
/* Find random records */
for (u = 0; u < FIND_MANY; u++) {
/* Pick random record */
idx = (hsize_t)(HDrandom() % INSERT_MANY);
idx = (hsize_t)(rand() % INSERT_MANY);

/* Attempt to find existent record in root of level-4 B-tree */
/* (in increasing order) */
Expand Down Expand Up @@ -4978,7 +4978,7 @@ test_update_lots(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t
curr_time = 1451342093;
fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
#endif
HDsrandom((unsigned)curr_time);
srand((unsigned)curr_time);

/*
* Test inserting many records into v2 B-tree
Expand All @@ -5000,7 +5000,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
H5B2_test_rec_t temp_rec; /* Temporary record */
unsigned swap_idx; /* Location to swap with when shuffling */

swap_idx = ((unsigned)HDrandom() % (INSERT_MANY_REC - u)) + u;
swap_idx = ((unsigned)rand() % (INSERT_MANY_REC - u)) + u;
temp_rec = records[u];
records[u] = records[swap_idx];
records[swap_idx] = temp_rec;
Expand Down Expand Up @@ -5076,7 +5076,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
/* Find random records */
for (u = 0; u < FIND_MANY_REC; u++) {
/* Pick random record */
find.key = (hsize_t)(HDrandom() % INSERT_MANY_REC);
find.key = (hsize_t)(rand() % INSERT_MANY_REC);
find.val = (hsize_t)-1;

/* Attempt to find existent record in level-4 B-tree */
Expand Down Expand Up @@ -5112,7 +5112,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
hsize_t idx; /* Record index */

/* Pick random record */
idx = (hsize_t)(HDrandom() % INSERT_MANY_REC);
idx = (hsize_t)(rand() % INSERT_MANY_REC);

/* Reset find record */
find.key = (hsize_t)-1;
Expand Down Expand Up @@ -8624,7 +8624,7 @@ test_remove_lots(const char *driver_name, hid_t fapl, const H5B2_create_t *cpara
curr_time = 1163537969;
fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
#endif
HDsrandom((unsigned)curr_time);
srand((unsigned)curr_time);

/*
* Test removing many records into v2 B-tree
Expand All @@ -8643,7 +8643,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
hsize_t temp_rec; /* Temporary record */
unsigned swap_idx; /* Location to swap with when shuffling */

swap_idx = ((unsigned)HDrandom() % (INSERT_MANY - u)) + u;
swap_idx = ((unsigned)rand() % (INSERT_MANY - u)) + u;
temp_rec = records[u];
records[u] = records[swap_idx];
records[swap_idx] = temp_rec;
Expand Down Expand Up @@ -8703,7 +8703,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
hsize_t temp_rec; /* Temporary record */
unsigned swap_idx; /* Location to swap with when shuffling */

swap_idx = ((unsigned)HDrandom() % (INSERT_MANY - u)) + u;
swap_idx = ((unsigned)rand() % (INSERT_MANY - u)) + u;
temp_rec = records[u];
records[u] = records[swap_idx];
records[swap_idx] = temp_rec;
Expand Down Expand Up @@ -8797,7 +8797,7 @@ fprintf(stderr, "curr_time = %lu\n", (unsigned long)curr_time);
/* Remove all records */
for (u = 0; u < INSERT_MANY; u++) {
/* Pick a record index to remove from randomly */
rem_idx = ((unsigned)HDrandom() % (INSERT_MANY - u));
rem_idx = ((unsigned)rand() % (INSERT_MANY - u));
rrecord = HSIZET_MAX;

/* Remove random record */
Expand Down
10 changes: 5 additions & 5 deletions test/cache_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -1184,9 +1184,9 @@ mdc_api_call_smoke_check(int express_test, unsigned paged, hid_t fcpl_id)
/* do random reads on all datasets */
n = 0;
while ((pass) && (n < NUM_RANDOM_ACCESSES)) {
m = HDrand() % NUM_DSETS;
i = (HDrand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;
j = (HDrand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;
m = rand() % NUM_DSETS;
i = (rand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;
j = (rand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;

/* select on disk hyperslab */
offset[0] = (hsize_t)i; /*offset of hyperslab in file*/
Expand Down Expand Up @@ -1282,8 +1282,8 @@ mdc_api_call_smoke_check(int express_test, unsigned paged, hid_t fcpl_id)
m = 0;
n = 0;
while ((pass) && (n < NUM_RANDOM_ACCESSES)) {
i = (HDrand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;
j = (HDrand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;
i = (rand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;
j = (rand() % (DSET_SIZE / CHUNK_SIZE)) * CHUNK_SIZE;

/* select on disk hyperslab */
offset[0] = (hsize_t)i; /*offset of hyperslab in file*/
Expand Down
Loading

0 comments on commit 58e4d08

Please sign in to comment.