Merge pull request pmodels#6682 from hzhou/2309_hydra_args

hydra: remove hard-coded token limit

Approved-by: Ken Raffenetti

src/pm/hydra/lib/hydra.h

@@ -298,7 +298,8 @@ struct HYD_env_global {
 
 /* Executable information */
 struct HYD_exec {
-    char *exec[HYD_NUM_TMP_STRINGS];
+    char **exec;
+    int exec_len, exec_size;
     char *wdir;
 
     int proc_count;
@@ -476,6 +477,7 @@ HYD_status HYDU_alloc_node(struct HYD_node **node);
 void HYDU_free_node_list(struct HYD_node *node_list);
 void HYDU_free_proxy_list(struct HYD_proxy *proxy_list, int count);
 HYD_status HYDU_alloc_exec(struct HYD_exec **exec);
+HYD_status HYDU_exec_add_arg(struct HYD_exec *exec, const char *arg);
 void HYDU_free_exec_list(struct HYD_exec *exec_list);
 HYD_status HYDU_create_proxy_list_singleton(struct HYD_node *node, int pgid,
                                             int *proxy_count_p, struct HYD_proxy **proxy_list_p);

src/pm/hydra/lib/utils/alloc.c

@@ -155,7 +155,9 @@ HYD_status HYDU_alloc_exec(struct HYD_exec **exec)
     HYDU_FUNC_ENTER();
 
     HYDU_MALLOC_OR_JUMP(*exec, struct HYD_exec *, sizeof(struct HYD_exec), status);
-    (*exec)->exec[0] = NULL;
+    (*exec)->exec = NULL;
+    (*exec)->exec_len = 0;
+    (*exec)->exec_size = 0;
     (*exec)->wdir = NULL;
     (*exec)->proc_count = -1;
     (*exec)->env_prop = NULL;
@@ -171,6 +173,33 @@ HYD_status HYDU_alloc_exec(struct HYD_exec **exec)
     goto fn_exit;
 }
 
+HYD_status HYDU_exec_add_arg(struct HYD_exec *exec, const char *arg)
+{
+    HYD_status status = HYD_SUCCESS;
+
+    if (exec->exec_size == 0) {
+        exec->exec_size = 10;
+        exec->exec = MPL_malloc(sizeof(char *) * exec->exec_size, MPL_MEM_OTHER);
+        if (!exec->exec) {
+            HYDU_ERR_SETANDJUMP(status, HYD_INTERNAL_ERROR, "Can't allocate exec\n");
+        }
+    } else if (exec->exec_len + 1 >= exec->exec_size) {
+        exec->exec_size = exec->exec_size * 2;
+        exec->exec = MPL_realloc(exec->exec, sizeof(char *) * exec->exec_size, MPL_MEM_OTHER);
+        if (!exec->exec) {
+            HYDU_ERR_SETANDJUMP(status, HYD_INTERNAL_ERROR, "Can't allocate exec\n");
+        }
+    }
+
+    exec->exec[exec->exec_len++] = MPL_strdup(arg);
+    exec->exec[exec->exec_len] = NULL;
+
+  fn_exit:
+    return status;
+  fn_fail:
+    goto fn_exit;
+}
+
 void HYDU_free_exec_list(struct HYD_exec *exec_list)
 {
     struct HYD_exec *exec, *run;
@@ -180,7 +209,13 @@ void HYDU_free_exec_list(struct HYD_exec *exec_list)
     exec = exec_list;
     while (exec) {
         run = exec->next;
-        HYDU_free_strlist(exec->exec);
+        if (exec->exec) {
+            HYDU_free_strlist(exec->exec);
+            MPL_free(exec->exec);
+            exec->exec = NULL;
+            exec->exec_len = 0;
+            exec->exec_size = 0;
+        }
 
         MPL_free(exec->wdir);
         MPL_free(exec->env_prop);
@@ -205,9 +240,10 @@ static HYD_status add_exec_to_proxy(struct HYD_exec *exec, struct HYD_proxy *pro
         status = HYDU_alloc_exec(&proxy->exec_list);
         HYDU_ERR_POP(status, "unable to allocate proxy exec\n");
 
-        for (i = 0; exec->exec[i]; i++)
-            proxy->exec_list->exec[i] = MPL_strdup(exec->exec[i]);
-        proxy->exec_list->exec[i] = NULL;
+        for (i = 0; exec->exec[i]; i++) {
+            status = HYDU_exec_add_arg(proxy->exec_list, exec->exec[i]);
+            HYDU_ERR_POP(status, "unable to add exec arg\n");
+        }
 
         proxy->exec_list->wdir = exec->wdir ? MPL_strdup(exec->wdir) : NULL;
         proxy->exec_list->proc_count = num_procs;
@@ -221,9 +257,10 @@ static HYD_status add_exec_to_proxy(struct HYD_exec *exec, struct HYD_proxy *pro
 
         texec = texec->next;
 
-        for (i = 0; exec->exec[i]; i++)
-            texec->exec[i] = MPL_strdup(exec->exec[i]);
-        texec->exec[i] = NULL;
+        for (i = 0; exec->exec[i]; i++) {
+            status = HYDU_exec_add_arg(texec, exec->exec[i]);
+            HYDU_ERR_POP(status, "unable to add exec arg\n");
+        }
 
         texec->wdir = exec->wdir ? MPL_strdup(exec->wdir) : NULL;
         texec->proc_count = num_procs;
@@ -366,14 +403,14 @@ HYD_status HYDU_create_proxy_list(int count, struct HYD_exec *exec_list, struct
 
 HYD_status HYDU_correct_wdir(char **wdir)
 {
-    char *tmp[HYD_NUM_TMP_STRINGS];
     HYD_status status = HYD_SUCCESS;
 
     HYDU_FUNC_ENTER();
 
     if (*wdir == NULL) {
         *wdir = HYDU_getcwd();
     } else if (*wdir[0] != '/') {
+        char *tmp[4];
         tmp[0] = HYDU_getcwd();
         tmp[1] = MPL_strdup("/");
         tmp[2] = MPL_strdup(*wdir);

src/pm/hydra/lib/utils/args.c

@@ -60,7 +60,7 @@ static HYD_status get_abs_wd(const char *wd, char **abs_wd)
 
 HYD_status HYDU_find_in_path(const char *execname, char **path)
 {
-    char *tmp[HYD_NUM_TMP_STRINGS], *path_loc = NULL, *test_loc, *user_path;
+    char *tmp[4], *path_loc = NULL, *test_loc, *user_path;
     HYD_status status = HYD_SUCCESS;
 
     HYDU_FUNC_ENTER();
@@ -365,7 +365,7 @@ HYD_status HYDU_parse_hostfile(const char *hostfile, void *data,
 
 char *HYDU_find_full_path(const char *execname)
 {
-    char *tmp[HYD_NUM_TMP_STRINGS] = { NULL }, *path = NULL, *test_path = NULL;
+    char *tmp[3] = { NULL }, *path = NULL, *test_path = NULL;
     HYD_status status = HYD_SUCCESS;
 
     HYDU_FUNC_ENTER();

src/pm/hydra/lib/utils/env.c

@@ -9,7 +9,7 @@
 HYD_status HYDU_env_to_str(struct HYD_env *env, char **str)
 {
     int i;
-    char *tmp[HYD_NUM_TMP_STRINGS];
+    char *tmp[10];
     HYD_status status = HYD_SUCCESS;
 
     HYDU_FUNC_ENTER();
@@ -253,7 +253,7 @@ HYD_status HYDU_append_env_str_to_list(const char *str, struct HYD_env **env_lis
 
 HYD_status HYDU_putenv(struct HYD_env *env, HYD_env_overwrite_t overwrite)
 {
-    char *tmp[HYD_NUM_TMP_STRINGS], *str;
+    char *tmp[10], *str;
     int i;
     HYD_status status = HYD_SUCCESS;
 

src/pm/hydra/lib/utils/string.c

@@ -223,43 +223,50 @@ int HYDU_strlist_lastidx(char **strlist)
 
 char **HYDU_str_to_strlist(char *str)
 {
-    int argc = 0, i;
+    int argc = 0;
+    int capacity = 0;
     char **strlist = NULL;
     char *p;
     HYD_status status = HYD_SUCCESS;
 
     HYDU_FUNC_ENTER();
 
-    HYDU_MALLOC_OR_JUMP(strlist, char **, HYD_NUM_TMP_STRINGS * sizeof(char *), status);
+    capacity = 10;
+    strlist = MPL_malloc(capacity * sizeof(char *), MPL_MEM_OTHER);
     if (!strlist)
         HYDU_ERR_SETANDJUMP(status, HYD_INTERNAL_ERROR, "Unable to allocate mem for strlist\n");
 
-    for (i = 0; i < HYD_NUM_TMP_STRINGS; i++)
-        strlist[i] = NULL;
-
     p = str;
     while (*p) {
         while (isspace(*p))
             p++;
 
-        if (argc >= HYD_NUM_TMP_STRINGS)
-            HYDU_ERR_SETANDJUMP(status, HYD_INTERNAL_ERROR, "too many arguments in line\n");
-
-        HYDU_MALLOC_OR_JUMP(strlist[argc], char *, HYD_TMP_STRLEN, status);
-
         /* Copy till you hit a space */
-        i = 0;
+        char *start = p;
         while (*p && !isspace(*p)) {
-            strlist[argc][i] = *p;
-            i++;
             p++;
         }
-        if (i) {
-            strlist[argc][i] = 0;
+        int len = p - start;
+        if (len > 0) {
+            char *s;
+            HYDU_MALLOC_OR_JUMP(s, char *, len + 1, status);
+            MPL_strncpy(s, start, len);
+
+            strlist[argc] = s;
             argc++;
+            if (argc == capacity) {
+                capacity *= 2;
+                strlist = MPL_realloc(strlist, capacity * sizeof(char *), MPL_MEM_OTHER);
+                if (!strlist) {
+                    HYDU_ERR_SETANDJUMP(status, HYD_INTERNAL_ERROR,
+                                        "Unable to allocate mem for strlist\n");
+                }
+            }
+        } else {
+            /* happens when line has trailing spaces (including \n) */
+            break;
         }
     }
-    MPL_free(strlist[argc]);
     strlist[argc] = NULL;
 
   fn_exit:

src/pm/hydra/mpiexec/get_parameters.c

@@ -8,6 +8,7 @@
 #include "mpiexec.h"
 #include "uiu.h"
 #include "pmi_util.h"   /* from libpmi, for PMIU_verbose */
+#include "utarray.h"
 
 /* The order of loading options:
  *     * set default sentinel values
@@ -33,7 +34,7 @@ HYD_status HYD_uii_mpx_get_parameters(char **t_argv)
     size_t len;
     char **argv = t_argv;
     char *progname = *argv;
-    char *post, *loc, *tmp[HYD_NUM_TMP_STRINGS], *conf_file;
+    char *post, *loc, *conf_file;
     const char *home, *env_file;
     HYD_status status = HYD_SUCCESS;
 
@@ -91,16 +92,19 @@ HYD_status HYD_uii_mpx_get_parameters(char **t_argv)
 
   config_file_check_exit:
     if (HYD_ui_mpich_info.config_file) {
-        char **config_argv;
-        HYDU_MALLOC_OR_JUMP(config_argv, char **, HYD_NUM_TMP_STRINGS * sizeof(char *), status);
-
-        status =
-            HYDU_parse_hostfile(HYD_ui_mpich_info.config_file, config_argv, process_config_token);
+        UT_array *args = NULL;
+        utarray_new(args, &ut_str_icd, MPL_MEM_OTHER);
+        status = HYDU_parse_hostfile(HYD_ui_mpich_info.config_file, args, process_config_token);
         HYDU_ERR_POP(status, "error parsing config file\n");
 
-        status = parse_args(config_argv, 1);
+        const char *null_ptr = NULL;
+        utarray_push_back(args, &null_ptr, MPL_MEM_OTHER);
+
+        status = parse_args(ut_str_array(args), 1);
         HYDU_ERR_POP(status, "error parsing config args\n");
 
+        utarray_free(args);
+
         MPL_free(HYD_ui_mpich_info.config_file);
         HYD_ui_mpich_info.config_file = NULL;
     }
@@ -119,6 +123,7 @@ HYD_status HYD_uii_mpx_get_parameters(char **t_argv)
 
         /* Check if its absolute or relative */
         if (post[0] != '/') {   /* relative */
+            char *tmp[4];
             tmp[0] = HYDU_getcwd();
             tmp[1] = MPL_strdup("/");
             tmp[2] = MPL_strdup(post);
@@ -277,18 +282,18 @@ static HYD_status check_environment(void)
 static HYD_status process_config_token(char *token, int newline, void *data)
 {
     static int idx = 0;
-    char **config_argv = data;
+    UT_array *args = data;
 
-    if (idx && newline && strcmp(config_argv[idx - 1], ":")) {
+    char **last_arg = (char **) utarray_back(args);
+    if (idx && newline && strcmp(*last_arg, ":")) {
         /* If this is a newline, but not the first one, and the
          * previous token was not a ":", add an executable delimiter
          * ':' */
-        config_argv[idx++] = MPL_strdup(":");
+        static const char *colon = ":";
+        utarray_push_back(args, &colon, MPL_MEM_OTHER);
     }
 
-    config_argv[idx++] = MPL_strdup(token);
-    assert(idx < HYD_NUM_TMP_STRINGS);
-    config_argv[idx] = NULL;
+    utarray_push_back(args, &token, MPL_MEM_OTHER);
 
     return HYD_SUCCESS;
 }
@@ -297,7 +302,6 @@ static HYD_status parse_args(char **t_argv, int reading_config_file)
 {
     char **argv = t_argv;
     struct HYD_exec *exec;
-    int i;
     HYD_status status = HYD_SUCCESS;
 
     HYDU_FUNC_ENTER();
@@ -325,11 +329,8 @@ static HYD_status parse_args(char **t_argv, int reading_config_file)
                 break;
             }
 
-            i = 0;
-            while (exec->exec[i] != NULL)
-                i++;
-            exec->exec[i] = MPL_strdup(*argv);
-            exec->exec[i + 1] = NULL;
+            status = HYDU_exec_add_arg(exec, *argv);
+            HYDU_ERR_POP(status, "unable to add exec arg\n");
         } while (++argv && *argv);
     } while (1);
 

src/pm/hydra/mpiexec/mpiexec.c

@@ -55,7 +55,7 @@ int main(int argc, char **argv)
 
     /* Check if any exec argument is missing */
     for (exec = HYD_uii_mpx_exec_list; exec; exec = exec->next) {
-        if (exec->exec[0] == NULL) {
+        if (exec->exec_len == 0) {
             HYDU_ERR_SETANDJUMP(status, HYD_INVALID_PARAM,
                                 "Missing executable. Try -h for usages.\n");
         }

src/pm/hydra/mpiexec/options.c

@@ -7,6 +7,7 @@
 #include "hydra.h"
 #include "mpiexec.h"
 #include "uiu.h"
+#include "utarray.h"
 
 /* This file defines HYD_mpiexec_match_table, which includes all the
  * argument parsing routines and help messages for command line options.
@@ -327,9 +328,10 @@ static void hostlist_help_fn(void)
 
 static HYD_status hostlist_fn(char *arg, char ***argv)
 {
-    char *hostlist[HYD_NUM_TMP_STRINGS + 1];    /* +1 for null termination of list */
-    int count = 0;
     HYD_status status = HYD_SUCCESS;
+    char *tok;
+    static UT_icd str_icd = { sizeof(char *), NULL, NULL, NULL };
+    UT_array *hosts = NULL;
 
     if (HYD_ui_mpich_info.reading_config_file && HYD_server_info.node_list) {
         /* global variable already set; ignore */
@@ -339,25 +341,28 @@ static HYD_status hostlist_fn(char *arg, char ***argv)
     HYDU_ERR_CHKANDJUMP(status, HYD_server_info.node_list, HYD_INTERNAL_ERROR,
                         "duplicate host file or host list setting\n");
 
-    hostlist[count] = strtok(**argv, ",");
-    while ((count < HYD_NUM_TMP_STRINGS) && hostlist[count])
-        hostlist[++count] = strtok(NULL, ",");
-
-    if (count >= HYD_NUM_TMP_STRINGS)
-        HYDU_ERR_SETANDJUMP(status, HYD_INTERNAL_ERROR, "too many hosts listed\n");
+    utarray_new(hosts, &str_icd, MPL_MEM_OTHER);
+    tok = strtok(**argv, ",");
+    while (tok) {
+        utarray_push_back(hosts, &tok, MPL_MEM_OTHER);
+        tok = strtok(**argv, ",");
+    }
 
-    for (count = 0; hostlist[count]; count++) {
+    char **p = (char **) utarray_front(hosts);
+    while (p) {
         char *h, *procs = NULL;
         int np;
 
-        h = strtok(hostlist[count], ":");
+        h = strtok(*p, ":");
         procs = strtok(NULL, ":");
         np = procs ? atoi(procs) : 1;
 
         status = HYDU_add_to_node_list(h, np, &HYD_server_info.node_list);
         HYDU_ERR_POP(status, "unable to add to node list\n");
     }
 
+    utarray_free(hosts);
+
   fn_exit:
     (*argv)++;
     return status;