From 68de0c1021bced3f9db8a12c336de701581bdc89 Mon Sep 17 00:00:00 2001 From: Ralph Castain Date: Thu, 22 Jun 2017 20:11:36 -0700 Subject: [PATCH] Fix the pmix_query API when it asks for something that returns an array of pmix_info_t. Protect the PMIX_INFO_FREE macro from NULL arrays. Update the mpi_memprobe scaling test Signed-off-by: Ralph Castain (cherry picked from commit 6ec2ad5288b3f70c004887c577a778c6d7c66c49) --- contrib/scaling/mpi_memprobe.c | 38 +++++++++---------- .../pmix/pmix2x/pmix/include/pmix_common.h | 2 +- .../pmix/pmix2x/pmix/src/buffer_ops/unpack.c | 11 +++++- 3 files changed, 28 insertions(+), 23 deletions(-) diff --git a/contrib/scaling/mpi_memprobe.c b/contrib/scaling/mpi_memprobe.c index 9661a09dd86..75ab6c174ca 100644 --- a/contrib/scaling/mpi_memprobe.c +++ b/contrib/scaling/mpi_memprobe.c @@ -10,6 +10,7 @@ #include #include "mpi.h" #include "opal/mca/pmix/pmix.h" +#include "opal/util/argv.h" #include "orte/runtime/runtime.h" #include "orte/util/proc_info.h" #include "orte/util/name_fns.h" @@ -117,17 +118,19 @@ static void sample(void) free(tmp); OPAL_LIST_FOREACH(kv, &response, opal_value_t) { lt = (opal_list_t*)kv->data.ptr; - OPAL_LIST_FOREACH(ival, lt, opal_value_t) { - if (0 == strcmp(ival->key, OPAL_PMIX_DAEMON_MEMORY)) { - asprintf(&tmp, "\tDaemon: %f", ival->data.fval); - opal_argv_append_nosize(&answer, tmp); - free(tmp); - } else if (0 == strcmp(ival->key, OPAL_PMIX_CLIENT_AVG_MEMORY)) { - asprintf(&tmp, "\tClient: %f", ival->data.fval); - opal_argv_append_nosize(&answer, tmp); - free(tmp); - } else { - fprintf(stderr, "\tUnknown key: %s", ival->key); + if (NULL != lt) { + OPAL_LIST_FOREACH(ival, lt, opal_value_t) { + if (0 == strcmp(ival->key, OPAL_PMIX_DAEMON_MEMORY)) { + asprintf(&tmp, "\tDaemon: %f", ival->data.fval); + opal_argv_append_nosize(&answer, tmp); + free(tmp); + } else if (0 == strcmp(ival->key, OPAL_PMIX_CLIENT_AVG_MEMORY)) { + asprintf(&tmp, "\tClient: %f", ival->data.fval); + opal_argv_append_nosize(&answer, tmp); + free(tmp); + } else { + fprintf(stderr, "\tUnknown key: %s", ival->key); + } } } } @@ -149,7 +152,6 @@ static void sample(void) } OPAL_LIST_DESTRUCT(&response); - if (0 == rank) { /* send the notification to release the other procs */ wait_for_release = true; @@ -162,19 +164,15 @@ static void sample(void) active = -1; if (OPAL_SUCCESS != opal_pmix.notify_event(MEMPROBE_RELEASE, NULL, OPAL_PMIX_RANGE_GLOBAL, &response, - notifycbfunc, (void*)&active)) { + NULL, NULL)) { fprintf(stderr, "Notify event failed\n"); exit(1); } - while (-1 == active) { + } else { + /* now wait for notification */ + while (wait_for_release) { usleep(10); } - OPAL_LIST_DESTRUCT(&response); - } - - /* now wait for notification */ - while (wait_for_release) { - usleep(10); } } diff --git a/opal/mca/pmix/pmix2x/pmix/include/pmix_common.h b/opal/mca/pmix/pmix2x/pmix/include/pmix_common.h index e2cc36d8a3f..e4b8e8884b9 100644 --- a/opal/mca/pmix/pmix2x/pmix/include/pmix_common.h +++ b/opal/mca/pmix/pmix2x/pmix/include/pmix_common.h @@ -905,7 +905,7 @@ typedef struct pmix_value { free((m)->data.bo.bytes); \ } \ } else if (PMIX_DATA_ARRAY == (m)->type) { \ - if (NULL != (m)->data.darray) { \ + if (NULL != (m)->data.darray && NULL != (m)->data.darray->array) { \ if (PMIX_STRING == (m)->data.darray->type) { \ char **_str = (char**)(m)->data.darray->array; \ for (_n=0; _n < (m)->data.darray->size; _n++) { \ diff --git a/opal/mca/pmix/pmix2x/pmix/src/buffer_ops/unpack.c b/opal/mca/pmix/pmix2x/pmix/src/buffer_ops/unpack.c index 53e73ac1c9b..0deec55adfc 100644 --- a/opal/mca/pmix/pmix2x/pmix/src/buffer_ops/unpack.c +++ b/opal/mca/pmix/pmix2x/pmix/src/buffer_ops/unpack.c @@ -714,8 +714,8 @@ pmix_status_t pmix_bfrop_unpack_status(pmix_buffer_t *buffer, void *dest, break; /********************/ default: - pmix_output(0, "UNPACK-PMIX-VALUE: UNSUPPORTED TYPE %d", (int)val->type); - return PMIX_ERROR; + pmix_output(0, "UNPACK-PMIX-VALUE: UNSUPPORTED TYPE %d", (int)val->type); + return PMIX_ERROR; } return PMIX_SUCCESS; @@ -765,6 +765,7 @@ pmix_status_t pmix_bfrop_unpack_info(pmix_buffer_t *buffer, void *dest, m=1; tmp = NULL; if (PMIX_SUCCESS != (ret = pmix_bfrop_unpack_string(buffer, &tmp, &m, PMIX_STRING))) { + PMIX_ERROR_LOG(ret); return ret; } if (NULL == tmp) { @@ -775,6 +776,7 @@ pmix_status_t pmix_bfrop_unpack_info(pmix_buffer_t *buffer, void *dest, /* unpack the flags */ m=1; if (PMIX_SUCCESS != (ret = pmix_bfrop_unpack_infodirs(buffer, &ptr[i].flags, &m, PMIX_INFO_DIRECTIVES))) { + PMIX_ERROR_LOG(ret); return ret; } /* unpack value - since the value structure is statically-defined @@ -782,12 +784,14 @@ pmix_status_t pmix_bfrop_unpack_info(pmix_buffer_t *buffer, void *dest, * avoid the malloc */ m=1; if (PMIX_SUCCESS != (ret = pmix_bfrop_unpack_int(buffer, &ptr[i].value.type, &m, PMIX_INT))) { + PMIX_ERROR_LOG(ret); return ret; } pmix_output_verbose(20, pmix_globals.debug_output, "pmix_bfrop_unpack: info type %d", ptr[i].value.type); m=1; if (PMIX_SUCCESS != (ret = unpack_val(buffer, &ptr[i].value))) { + PMIX_ERROR_LOG(ret); return ret; } } @@ -1272,6 +1276,9 @@ pmix_status_t pmix_bfrop_unpack_darray(pmix_buffer_t *buffer, void *dest, case PMIX_STATUS: nbytes = sizeof(pmix_status_t); break; + case PMIX_INFO: + nbytes = sizeof(pmix_info_t); + break; case PMIX_PROC: nbytes = sizeof(pmix_proc_t); break;