diff --git a/.gitignore b/.gitignore index 089d60d9614..63e6fbe444b 100644 --- a/.gitignore +++ b/.gitignore @@ -633,6 +633,7 @@ test/class/ompi_rb_tree test/class/ompi_bitmap test/class/opal_bitmap test/class/opal_fifo +test/class/opal_cstring test/class/opal_hash_table test/class/opal_lifo test/class/opal_list diff --git a/ompi/mca/osc/base/osc_base_init.c b/ompi/mca/osc/base/osc_base_init.c index bd949b6d242..41d8cb03afb 100644 --- a/ompi/mca/osc/base/osc_base_init.c +++ b/ompi/mca/osc/base/osc_base_init.c @@ -57,10 +57,6 @@ ompi_osc_base_select(ompi_win_t *win, priority = component->osc_query(win, base, size, disp_unit, comm, win->super.s_info, flavor); if (priority < 0) { - if (MPI_WIN_FLAVOR_SHARED == flavor && OMPI_ERR_RMA_SHARED == priority) { - /* NTH: quick fix to return OMPI_ERR_RMA_SHARED */ - return OMPI_ERR_RMA_SHARED; - } continue; } diff --git a/ompi/mca/osc/monitoring/osc_monitoring_component.c b/ompi/mca/osc/monitoring/osc_monitoring_component.c index f91cd8f1160..26b49ea865b 100644 --- a/ompi/mca/osc/monitoring/osc_monitoring_component.c +++ b/ompi/mca/osc/monitoring/osc_monitoring_component.c @@ -90,10 +90,6 @@ static int mca_osc_monitoring_component_select(struct ompi_win_t *win, void **ba priority = component->osc_query(win, base, size, disp_unit, comm, info, flavor); if (priority < 0) { - if (MPI_WIN_FLAVOR_SHARED == flavor && OMPI_ERR_RMA_SHARED == priority) { - /* NTH: quick fix to return OMPI_ERR_RMA_SHARED */ - return OMPI_ERR_RMA_SHARED; - } continue; } diff --git a/ompi/mca/osc/rdma/osc_rdma_component.c b/ompi/mca/osc/rdma/osc_rdma_component.c index 3bf2a00d5f8..cdc71ad3056 100644 --- a/ompi/mca/osc/rdma/osc_rdma_component.c +++ b/ompi/mca/osc/rdma/osc_rdma_component.c @@ -383,7 +383,7 @@ static int ompi_osc_rdma_component_query (struct ompi_win_t *win, void **base, s { if (MPI_WIN_FLAVOR_SHARED == flavor) { - return OMPI_ERR_RMA_SHARED; + return -1; } #if OPAL_CUDA_SUPPORT @@ -403,7 +403,7 @@ static int ompi_osc_rdma_component_query (struct ompi_win_t *win, void **base, s return mca_osc_rdma_component.priority; } - return OMPI_ERROR; + return -1; } static int ompi_osc_rdma_initialize_region (ompi_osc_rdma_module_t *module, void **base, size_t size) { diff --git a/ompi/mca/osc/sm/osc_sm.h b/ompi/mca/osc/sm/osc_sm.h index 4ea903d03e2..b97329bfc63 100644 --- a/ompi/mca/osc/sm/osc_sm.h +++ b/ompi/mca/osc/sm/osc_sm.h @@ -53,6 +53,9 @@ typedef struct ompi_osc_sm_node_state_t ompi_osc_sm_node_state_t; struct ompi_osc_sm_component_t { ompi_osc_base_component_t super; + /** Priority of the osc/sm component */ + unsigned int priority; + char *backing_directory; }; typedef struct ompi_osc_sm_component_t ompi_osc_sm_component_t; diff --git a/ompi/mca/osc/sm/osc_sm_component.c b/ompi/mca/osc/sm/osc_sm_component.c index 6b51c6d9403..1edf77e8386 100644 --- a/ompi/mca/osc/sm/osc_sm_component.c +++ b/ompi/mca/osc/sm/osc_sm_component.c @@ -115,6 +115,8 @@ ompi_osc_sm_module_t ompi_osc_sm_module_template = { static int component_register (void) { + char *description_str; + if (0 == access ("/dev/shm", W_OK)) { mca_osc_sm_component.backing_directory = "/dev/shm"; } else { @@ -128,6 +130,16 @@ static int component_register (void) MCA_BASE_VAR_TYPE_STRING, NULL, 0, 0, OPAL_INFO_LVL_3, MCA_BASE_VAR_SCOPE_READONLY, &mca_osc_sm_component.backing_directory); + mca_osc_sm_component.priority = 100; + opal_asprintf(&description_str, "Priority of the osc/sm component (default: %d)", + mca_osc_sm_component.priority); + (void)mca_base_component_var_register(&mca_osc_sm_component.super.osc_version, + "priority", description_str, + MCA_BASE_VAR_TYPE_UNSIGNED_INT, NULL, 0, 0, + OPAL_INFO_LVL_3, MCA_BASE_VAR_SCOPE_GROUP, + &mca_osc_sm_component.priority); + free(description_str); + return OPAL_SUCCESS; } @@ -154,36 +166,32 @@ component_finalize(void) } -static int -check_win_ok(ompi_communicator_t *comm, int flavor) -{ - if (! (MPI_WIN_FLAVOR_SHARED == flavor - || MPI_WIN_FLAVOR_ALLOCATE == flavor) ) { - return OMPI_ERR_NOT_SUPPORTED; - } - - if (ompi_group_have_remote_peers (comm->c_local_group)) { - return OMPI_ERR_RMA_SHARED; - } - - return OMPI_SUCCESS; -} - - static int component_query(struct ompi_win_t *win, void **base, size_t size, int disp_unit, struct ompi_communicator_t *comm, struct opal_info_t *info, int flavor) { int ret; - if (OMPI_SUCCESS != (ret = check_win_ok(comm, flavor))) { - if (OMPI_ERR_NOT_SUPPORTED == ret) { + + /* component only supports shared or allocate flavors */ + if (! (MPI_WIN_FLAVOR_SHARED == flavor || + MPI_WIN_FLAVOR_ALLOCATE == flavor)) { + return -1; + } + + /* If flavor is win_allocate, we can't run if there are remote + * peers in the group. The same check for flavor_shared happens + * in select(), so that we can return an error to the user (since + * we should be able to run for all flavor_shared use cases. + * There's no way to return an error from component_query to the + * user, hence the delayed check. */ + if (MPI_WIN_FLAVOR_ALLOCATE == flavor) { + if (ompi_group_have_remote_peers(comm->c_local_group)) { return -1; } - return ret; } - return 100; + return mca_osc_sm_component.priority; } @@ -198,8 +206,10 @@ component_select(struct ompi_win_t *win, void **base, size_t size, int disp_unit int ret = OMPI_ERROR; size_t memory_alignment = OPAL_ALIGN_MIN; - if (OMPI_SUCCESS != (ret = check_win_ok(comm, flavor))) { - return ret; + assert(MPI_WIN_FLAVOR_SHARED == flavor || MPI_WIN_FLAVOR_ALLOCATE == flavor); + + if (ompi_group_have_remote_peers(comm->c_local_group)) { + return OMPI_ERR_RMA_SHARED; } /* create module structure */ diff --git a/opal/class/opal_cstring.c b/opal/class/opal_cstring.c index c5d230d19b1..674c3a6741b 100644 --- a/opal/class/opal_cstring.c +++ b/opal/class/opal_cstring.c @@ -33,7 +33,7 @@ static void opal_cstring_ctor(opal_cstring_t *obj) { *(size_t *) &(obj->length) = 0; /* make sure the string is null-terminated */ - ((char *) obj->string)[0] = '\n'; + ((char *) obj->string)[0] = '\0'; } static inline size_t opal_cstring_alloc_size(size_t len) @@ -90,10 +90,18 @@ int opal_cstring_to_int(opal_cstring_t *string, int *interp) if (*endp != '\0') { return OPAL_ERR_BAD_PARAM; } - /* underflow */ + /* base errors */ if (tmp == 0 && errno == EINVAL) { return OPAL_ERR_BAD_PARAM; } + /* overflow/underflow of long int (return of strtol) */ + if (errno == ERANGE && (tmp == LONG_MIN || tmp == LONG_MAX)) { + return OPAL_ERR_BAD_PARAM; + } + /* overflow/underflow of int (for cases where long int is larger) */ + if (tmp < INT_MIN || tmp > INT_MAX) { + return OPAL_ERR_BAD_PARAM; + } *interp = (int) tmp; @@ -104,24 +112,28 @@ static int opal_str_to_bool_impl(const char *string, bool *interp) { const char *ptr = string; - /* Trim leading whitespace */ - while (isspace(*ptr)) { - ++ptr; - } + if (NULL != string) { + /* Trim leading whitespace */ + while (isspace(*ptr)) { + ++ptr; + } - if ('\0' != *ptr) { - if (isdigit(*ptr)) { - *interp = (bool) atoi(ptr); - } else if (0 == strncasecmp(ptr, "yes", 3) || 0 == strncasecmp(ptr, "true", 4)) { - *interp = true; - } else if (0 == strncasecmp(ptr, "no", 2) && 0 == strncasecmp(ptr, "false", 5)) { - *interp = false; - } else { - *interp = false; - return OPAL_ERR_BAD_PARAM; + if ('\0' != *ptr) { + if (isdigit(*ptr)) { + *interp = (bool) atoi(ptr); + return OPAL_SUCCESS; + } else if (0 == strncasecmp(ptr, "yes", 3) || 0 == strncasecmp(ptr, "true", 4)) { + *interp = true; + return OPAL_SUCCESS; + } else if (0 == strncasecmp(ptr, "no", 2) || 0 == strncasecmp(ptr, "false", 5)) { + *interp = false; + return OPAL_SUCCESS; + } } } - return OPAL_SUCCESS; + + *interp = false; + return OPAL_ERR_BAD_PARAM; } int opal_cstring_to_bool(opal_cstring_t *string, bool *interp) diff --git a/opal/class/opal_cstring.h b/opal/class/opal_cstring.h index a5def2e075f..c92bac70400 100644 --- a/opal/class/opal_cstring.h +++ b/opal/class/opal_cstring.h @@ -43,7 +43,6 @@ #ifndef OPAL_STRING_H #define OPAL_STRING_H -#include "opal_config.h" #include "opal/class/opal_object.h" #include "opal/mca/base/mca_base_var_enum.h" diff --git a/test/class/Makefile.am b/test/class/Makefile.am index ee37245f909..1e39017584b 100644 --- a/test/class/Makefile.am +++ b/test/class/Makefile.am @@ -35,7 +35,8 @@ check_PROGRAMS = \ opal_value_array \ opal_pointer_array \ opal_lifo \ - opal_fifo + opal_fifo \ + opal_cstring TESTS = $(check_PROGRAMS) @@ -94,6 +95,12 @@ opal_fifo_LDADD = \ $(top_builddir)/test/support/libsupport.a opal_fifo_DEPENDENCIES = $(opal_fifo_LDADD) +opal_cstring_SOURCES = opal_cstring.c +opal_cstring_LDADD = \ + $(top_builddir)/opal/lib@OPAL_LIB_NAME@.la \ + $(top_builddir)/test/support/libsupport.a +opal_cstring_DEPENDENCIES = $(opal_cstring_LDADD) + clean-local: rm -f opal_bitmap_test_out.txt opal_hash_table_test_out.txt opal_proc_table_test_out.txt diff --git a/test/class/opal_cstring.c b/test/class/opal_cstring.c new file mode 100644 index 00000000000..d59c3573f48 --- /dev/null +++ b/test/class/opal_cstring.c @@ -0,0 +1,247 @@ +/* + * Copyright (c) 2022 Amazon.com, Inc. or its affiliates. + * All Rights reserved. + * $COPYRIGHT$ + * + * Additional copyrights may follow + * + * $HEADER$ + */ + +#include "opal_config.h" + +#include + +#include "opal/class/opal_cstring.h" +#include +#include + +#define CHECK_INT(actual, expected) \ + if (expected != actual) { \ + printf("%s:%d: expected %d, got %d\n", \ + __FILE__, __LINE__, expected, actual); \ + exit(1); \ + } + +#define CHECK_BOOL(actual, expected) \ + if (expected != actual) { \ + printf("%s:%d: expected %s, got %s\n", \ + __FILE__, __LINE__, \ + expected ? "true" : "false", \ + actual ? "true" : "false"); \ + exit(1); \ + } + +int main(int argc, char *argv[]) +{ + int ret, int_val; + bool bool_val; + opal_cstring_t *cstr; + + /* + * bool tests + */ + cstr = opal_cstring_create("true"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, true); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("yes"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, true); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("false"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, false); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("no"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, false); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("1"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, true); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("0"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, false); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("1.0"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, true); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("0.0"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_BOOL(bool_val, false); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("foobar"); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create(NULL); + ret = opal_cstring_to_bool(cstr, &bool_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + + /* + * bool string tests + */ + bool_val = opal_str_to_bool("true"); + CHECK_BOOL(bool_val, true); + + bool_val = opal_str_to_bool("yes"); + CHECK_BOOL(bool_val, true); + + bool_val = opal_str_to_bool("false"); + CHECK_BOOL(bool_val, false); + + bool_val = opal_str_to_bool("no"); + CHECK_BOOL(bool_val, false); + + bool_val = opal_str_to_bool("1"); + CHECK_BOOL(bool_val, true); + + bool_val = opal_str_to_bool("0"); + CHECK_BOOL(bool_val, false); + + bool_val = opal_str_to_bool("1.0"); + CHECK_BOOL(bool_val, true); + + bool_val = opal_str_to_bool("0.0"); + CHECK_BOOL(bool_val, false); + + bool_val = opal_str_to_bool("foobar"); + CHECK_BOOL(bool_val, false); + + bool_val = opal_str_to_bool(NULL); + CHECK_BOOL(bool_val, false); + + /* + * integer tests + */ + cstr = opal_cstring_create("1"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_INT(int_val, 1); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("0"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_SUCCESS); + CHECK_INT(int_val, 0); + OBJ_RELEASE(cstr); + + /* test intentional overflow cases */ + if (sizeof(long) > sizeof(int)) { + long input = (long)INT_MAX + 1L; + char *input_str; + + ret = asprintf(&input_str, "%ld", input); + if (ret < 0) { + printf("%s:%d: asprintf()", __FILE__, __LINE__); + exit(1); + } + + cstr = opal_cstring_create(input_str); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + free(input_str); + } + + /* yes, this may be redundant from above, but always try both + cases for simplicity */ + if (sizeof(long long) > sizeof(int)) { + long long input = (long long)INT_MAX + 1L; + char *input_str; + + ret = asprintf(&input_str, "%lld", input); + if (ret < 0) { + printf("%s:%d: asprintf()", __FILE__, __LINE__); + exit(1); + } + + cstr = opal_cstring_create(input_str); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + free(input_str); + } + + { + long input = (long)LONG_MAX; + char *input_str; + + ret = asprintf(&input_str, "%ld0", input); + if (ret < 0) { + printf("%s:%d: asprintf()", __FILE__, __LINE__); + exit(1); + } + + cstr = opal_cstring_create(input_str); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + free(input_str); + } + + + cstr = opal_cstring_create("1.0"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("0.0"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("foobar"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create(NULL); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("true"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("yes"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("false"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + cstr = opal_cstring_create("no"); + ret = opal_cstring_to_int(cstr, &int_val); + CHECK_INT(ret, OPAL_ERR_BAD_PARAM); + OBJ_RELEASE(cstr); + + return 0; +}