Skip to content

Commit

Permalink
ess/singleton: expects 4 PMIX_* environment variables or more
Browse files Browse the repository at this point in the history
  • Loading branch information
ggouaillardet committed Aug 23, 2016
1 parent 696121c commit a1e8e58
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions orte/mca/ess/singleton/ess_singleton_module.c
Expand Up @@ -15,6 +15,8 @@
* Copyright (c) 2013-2016 Intel, Inc. All rights reserved.
* Copyright (c) 2015 Los Alamos National Security, LLC. All rights
* reserved.
* Copyright (c) 2016 Research Organization for Information Science
* and Technology (RIST). All rights reserved.
* $COPYRIGHT$
*
* Additional copyrights may follow
Expand Down Expand Up @@ -564,6 +566,8 @@ static int fork_hnp(void)
exit(1);

} else {
int count;

free(cmd);
/* I am the parent - wait to hear something back and
* report results
Expand Down Expand Up @@ -630,12 +634,13 @@ static int fork_hnp(void)

/* split the pmix_uri into its parts */
argv = opal_argv_split(cptr, ',');
if (4 != opal_argv_count(argv)) {
count = opal_argv_count(argv);
if (4 > count) {
opal_argv_free(argv);
return ORTE_ERR_BAD_PARAM;
}
/* push each piece into the environment */
for (i=0; i < 4; i++) {
for (i=0; i < count; i++) {
pmixenvars[i] = strdup(argv[i]);
putenv(pmixenvars[i]);
}
Expand Down

3 comments on commit a1e8e58

@ggouaillardet
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhc54 can you please double check this ?

current internal pmix has now 5 PMIX_* environment variables.

that being said, i did not simply replace if (4 != opal_argv_count(argv)) with if (5 != opal_argv_count(argv)) and for (i=0; i < 4; i++) with for (i=0; i < 5; i++) since (older) external pmix might still use 4 PMIX_* environment variables.

@rhc54
Copy link
Contributor

@rhc54 rhc54 commented on a1e8e58 Aug 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest just dropping the test altogether - just push everything we get into the environment.

@ggouaillardet
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will do from now

Please sign in to comment.