Skip to content

Commit

Permalink
DAOS-14896 gurt: Fix d_getenv with negative int
Browse files Browse the repository at this point in the history 
Fix regression of d_getenv_xxx() functions used for retrieve int
envioronment variable: support of string reprsenting signed integer.

Test-tag: pr mpiio DfuseMUPerms
Required-githooks: true
Signed-off-by: Cedric Koch-Hofer <cedric.koch-hofer@intel.com>
  • Loading branch information
@kanard38 @knard-intel
kanard38 authored and knard-intel committed Jan 11, 2024
1 parent 9a2a674 commit ac60f58
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 70 deletions.
90 changes: 47 additions & 43 deletions src/gurt/misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
#include <gurt/common.h>
#include <gurt/atomic.h>

#define UINT64_MAX_STR "18446744073709551615"

/* state buffer for DAOS rand and srand calls, NOT thread safe */
static struct drand48_data randBuffer = {0};

Expand Down Expand Up @@ -951,18 +949,17 @@ d_rank_range_list_free(d_rank_range_list_t *range_list)
}

static inline bool
dis_unsigned_str(char *str)
dis_signed_str(char *str)
{
char *eos;

if (str == NULL || str[0] == '\0')
return false;
char *eos;
size_t str_size;

eos = str + (sizeof(UINT64_MAX_STR) - 1);
while (str != eos && *str != '\0' && *str >= '0' && *str <= '9')
str_size = strlen(str);
eos = str + str_size;
while (str != eos && *str != '-' && (*str < '0' || *str > '9'))
++str;

return *str == '\0';
return *str == '-';
}

static inline bool
Expand Down Expand Up @@ -1214,40 +1211,68 @@ d_getenv_char(const char *name, char *char_val)
}

static int
d_getenv_ull(unsigned long long *val, const char *name)
d_getenv_ull(unsigned long long *val, const char *name, size_t val_size)
{
char *env;
char *env_tmp = NULL;
char *endptr;
unsigned long long tmp;
unsigned long long val_tmp;
int rc;

assert(val != NULL);
assert(name != NULL);
assert(val_size <= sizeof(unsigned long long));

d_env_rwlock_rdlock();
env = getenv(name);
if (env == NULL) {
rc = -DER_NONEXIST;
d_env_rwlock_unlock();
goto out;
}

if (!dis_unsigned_str(env)) {
rc = -DER_INVAL;
/* DAOS-14896 NOTES:
* - Duplicate env to reduce data race condition with external libraries not using the DAOS
* thread safe environment variables management API.
* - Use of strdup() as there is no limit to environment variable size.
*/
env_tmp = strdup(env);
if (env_tmp == NULL) {
rc = -DER_NOMEM;
d_env_rwlock_unlock();
goto out;
}
d_env_rwlock_unlock();

errno = 0;
tmp = strtoull(env, &endptr, 0);
if (errno != 0 || endptr == env || *endptr != '\0') {
errno = 0;
val_tmp = strtoull(env_tmp, &endptr, 10);
if (errno != 0 || endptr == env_tmp || *endptr != '\0') {
rc = -DER_INVAL;
goto out;
}

*val = tmp;
if (val_size != sizeof(unsigned long long)) {
const unsigned long long val_max = (1ull << val_size * 8) - 1;
const bool is_signed = dis_signed_str(env_tmp);

if (is_signed)
val_tmp = ~val_tmp;
if (val_tmp > val_max || (is_signed && val_tmp >= val_max)) {
rc = -DER_INVAL;
goto out;
}
if (is_signed) {
val_tmp = ~val_tmp;
val_tmp <<= (sizeof(unsigned long long) - val_size) * 8;
val_tmp >>= (sizeof(unsigned long long) - val_size) * 8;
}
}

*val = val_tmp;
rc = -DER_SUCCESS;

out:
d_env_rwlock_unlock();
free(env_tmp);

return rc;
}
Expand All @@ -1269,17 +1294,10 @@ d_getenv_uint(const char *name, unsigned *uint_val)
assert(uint_val != NULL);
assert(name != NULL);

rc = d_getenv_ull(&tmp, name);
rc = d_getenv_ull(&tmp, name, sizeof(unsigned));
if (rc != -DER_SUCCESS)
return rc;

#if UINT_MAX != ULLONG_MAX
assert(sizeof(unsigned) < sizeof(unsigned long long));
if (tmp > UINT_MAX) {
return -DER_INVAL;
}
#endif

*uint_val = (unsigned)tmp;
return -DER_SUCCESS;
}
Expand All @@ -1301,17 +1319,10 @@ d_getenv_uint32_t(const char *name, uint32_t *uint32_val)
assert(uint32_val != NULL);
assert(name != NULL);

rc = d_getenv_ull(&tmp, name);
rc = d_getenv_ull(&tmp, name, sizeof(uint32_t));
if (rc != -DER_SUCCESS)
return rc;

#if UINT32_MAX != ULLONG_MAX
assert(sizeof(uint32_t) < sizeof(unsigned long long));
if (tmp > UINT32_MAX) {
return -DER_INVAL;
}
#endif

*uint32_val = (uint32_t)tmp;
return -DER_SUCCESS;
}
Expand All @@ -1333,17 +1344,10 @@ d_getenv_uint64_t(const char *name, uint64_t *uint64_val)
assert(uint64_val != NULL);
assert(name != NULL);

rc = d_getenv_ull(&tmp, name);
rc = d_getenv_ull(&tmp, name, sizeof(uint64_t));
if (rc != -DER_SUCCESS)
return rc;

#if UINT64_MAX != ULLONG_MAX
assert(sizeof(uint64_t) < sizeof(unsigned long long));
if (tmp > UINT64_MAX) {
return -DER_INVAL;
}
#endif

*uint64_val = (uint64_t)tmp;
return -DER_SUCCESS;
}
Expand Down
123 changes: 96 additions & 27 deletions src/gurt/tests/test_gurt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2288,35 +2288,63 @@ test_d_getenv_uint(void **state)
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT_MAX);

getenv_return = "42";
getenv_return = "-1";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT_MAX);

getenv_return = "-10";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT_MAX - 9);

getenv_return = "-4294967294";
rc = d_getenv_uint("foo", &val);
assert_true(val == 2);

getenv_return = "-4294967295";
rc = d_getenv_uint("foo", &val);
assert_true(val == 1);

getenv_return = " 000042";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == 42);

getenv_return = " -000042";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == -42);

getenv_return = "4294967296";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "-42";
getenv_return = "-4294967296";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "booo";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "42booo";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "";
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == -42);

getenv_return = NULL;
rc = d_getenv_uint("foo", &val);
assert_int_equal(rc, -DER_NONEXIST);
assert_true(val == 42);
assert_true(val == -42);
}

static void
Expand All @@ -2330,40 +2358,63 @@ test_d_getenv_uint32_t(void **state)
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT32_MAX);

getenv_return = "42";
getenv_return = "-1";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT32_MAX);

getenv_return = "-10";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT32_MAX - 9);

getenv_return = "-4294967294";
rc = d_getenv_uint32_t("foo", &val);
assert_true(val == 2);

getenv_return = "-4294967295";
rc = d_getenv_uint32_t("foo", &val);
assert_true(val == 1);

getenv_return = " 000042";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == 42);

getenv_return = " -000042";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == -42);

getenv_return = "4294967296";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "-42";
getenv_return = "-4294967296";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "booo";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "42booo";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "";
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = NULL;
rc = d_getenv_uint32_t("foo", &val);
assert_int_equal(rc, -DER_NONEXIST);
assert_true(val == 42);
assert_true(val == -42);
}

static void
Expand All @@ -2377,45 +2428,63 @@ test_d_getenv_uint64_t(void **state)
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT64_MAX);

getenv_return = "42";
getenv_return = "-1";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == 42);
assert_true(val == UINT64_MAX);

getenv_return = "18446744073709551616";
getenv_return = "-10";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == UINT64_MAX - 9);

getenv_return = "-18446744073709551614";
rc = d_getenv_uint64_t("foo", &val);
assert_true(val == 2);

getenv_return = "-18446744073709551615";
rc = d_getenv_uint64_t("foo", &val);
assert_true(val == 1);

getenv_return = " 000042";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == 42);

getenv_return = "012345678901234567890";
getenv_return = " -000042";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_SUCCESS);
assert_true(val == -42);

getenv_return = "18446744073709551616";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "-42";
getenv_return = "-18446744073709551616";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "booo";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "42booo";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = "";
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_INVAL);
assert_true(val == 42);
assert_true(val == -42);

getenv_return = NULL;
rc = d_getenv_uint64_t("foo", &val);
assert_int_equal(rc, -DER_NONEXIST);
assert_true(val == 42);
assert_true(val == -42);
}

static void
Expand Down

0 comments on commit ac60f58

Please sign in to comment.