Merge pull request #597 from hjelmn/mca_base_coverity

mca/base: fix coverity issues and enable project name in MCA groups

opal/mca/base/mca_base_component_repository.c

@@ -125,6 +125,7 @@ static int process_repository_item (const char *filename, void *data)
 
     ri = OBJ_NEW(mca_base_component_repository_item_t);
     if (NULL == ri) {
+        free (base);
         return OPAL_ERR_OUT_OF_RESOURCE;
     }
 
@@ -227,14 +228,14 @@ int mca_base_component_repository_init(void)
     OBJ_CONSTRUCT(&mca_base_component_repository, opal_hash_table_t);
     ret = opal_hash_table_init (&mca_base_component_repository, 128);
     if (OPAL_SUCCESS != ret) {
-        mca_base_framework_close (&opal_dl_base_framework);
+        (void) mca_base_framework_close (&opal_dl_base_framework);
         return ret;
     }
 
     ret = mca_base_component_repository_add (mca_base_component_path);
     if (OPAL_SUCCESS != ret) {
         OBJ_DESTRUCT(&mca_base_component_repository);
-        mca_base_framework_close (&opal_dl_base_framework);
+        (void) mca_base_framework_close (&opal_dl_base_framework);
         return ret;
     }
 #endif
@@ -326,6 +327,9 @@ int mca_base_component_repository_open (mca_base_framework_t *framework,
         }
     }
 
+    /* silence coverity issue (invalid free) */
+    mitem = NULL;
+
     if (NULL != ri->ri_dlhandle) {
         opal_output_verbose(40, 0, "mca_base_component_repository_open: already loaded. returning cached component");
         mitem = OBJ_NEW(mca_base_component_list_item_t);

opal/mca/base/mca_base_components_close.c

@@ -37,8 +37,7 @@ void mca_base_component_unload (const mca_base_component_t *component, int outpu
                         "mca: base: close: unloading component %s",
                         component->mca_component_name);
 
-    /* XXX -- TODO -- Replace reserved by mca_project_name for 1.9 */
-    ret = mca_base_var_group_find (component->reserved, component->mca_type_name,
+    ret = mca_base_var_group_find (component->mca_project_name, component->mca_type_name,
                                    component->mca_component_name);
     if (0 <= ret) {
         mca_base_var_group_deregister (ret);

opal/mca/base/mca_base_pvar.c

@@ -260,8 +260,8 @@ int mca_base_pvar_register (const char *project, const char *framework, const ch
 
         do {
             /* generate the variable's full name */
-            mca_base_var_generate_full_name4 (NULL, framework, component, name, &pvar->name);
-            if (NULL == pvar->name) {
+            ret = mca_base_var_generate_full_name4 (NULL, framework, component, name, &pvar->name);
+            if (OPAL_SUCCESS != ret) {
                 ret = OPAL_ERR_OUT_OF_RESOURCE;
                 break;
             }

opal/mca/base/mca_base_var.c

@@ -324,41 +324,46 @@ int mca_base_var_init(void)
 
 static void process_env_list(char *env_list, char ***argv, char sep)
 {
-    int i;
     char** tokens;
-    char* ptr;
-    char* param, *value;
+    char *ptr, *value;
+
     tokens = opal_argv_split(env_list, (int)sep);
-    if (NULL != tokens) {
-        for (i = 0; NULL != tokens[i]; i++) {
-            if (NULL == (ptr = strchr(tokens[i], '='))) {
-                value = getenv(tokens[i]);
-                if (NULL != value) {
-                    if (NULL != strchr(value, '=')) {
-                        param = strdup(value);
-                        value = strchr(param, '=');
-                        *value = '\0';
-                        value++;
-                        opal_setenv(param, value, true, argv);
-                        free(param);
-                    } else {
-                        opal_setenv(tokens[i], value, true, argv);
-                    }
-                } else {
-                    opal_show_help("help-mca-var.txt", "incorrect-env-list-param",
-                            true, tokens[i], env_list);
-                }
-            } else {
-                param = strdup(tokens[i]);
-                value = strchr(param, '=');
-                *value = '\0';
-                value++;
-                opal_setenv(param, value, true, argv);
-                free(param);
-            }
-        }
-        opal_argv_free(tokens);
+    if (NULL == tokens) {
+        return;
     }
+
+    for (int i = 0 ; NULL != tokens[i] ; ++i) {
+        if (NULL == (ptr = strchr(tokens[i], '='))) {
+            value = getenv(tokens[i]);
+            if (NULL == value) {
+                opal_show_help("help-mca-var.txt", "incorrect-env-list-param",
+                               true, tokens[i], env_list);
+                break;
+            }
+
+            /* duplicate the value to silence tainted string coverity issue */
+            value = strdup (value);
+            if (NULL == value) {
+                /* out of memory */
+                break;
+            }
+
+            if (NULL != (ptr = strchr(value, '='))) {
+                *ptr = '\0';
+                opal_setenv(value, ptr + 1, true, argv);
+            } else {
+                opal_setenv(tokens[i], value, true, argv);
+            }
+
+            free (value);
+        } else {
+            *ptr = '\0';
+            opal_setenv(tokens[i], ptr + 1, true, argv);
+            /* NTH: don't bother resetting ptr to = since the string will not be used again */
+        }
+    }
+
+    opal_argv_free(tokens);
 }
 
 int mca_base_var_process_env_list(char ***argv)
@@ -1175,6 +1180,7 @@ static int fixup_files(char **file_list, char * path, bool rel_path_search, char
     char **search_path = NULL;
     char * tmp_file = NULL;
     char **argv = NULL;
+    char *rel_path;
     int mode = R_OK; /* The file exists, and we can read it */
     int count, i, argc = 0;
 
@@ -1182,83 +1188,64 @@ static int fixup_files(char **file_list, char * path, bool rel_path_search, char
     files = opal_argv_split(*file_list, sep);
     count = opal_argv_count(files);
 
+    rel_path = force_agg_path ? force_agg_path : cwd;
+
     /* Read in reverse order, so we can preserve the original ordering */
     for (i = 0 ; i < count; ++i) {
-        /* Absolute paths preserved */
-        if ( opal_path_is_absolute(files[i]) ) {
-            if( NULL == (tmp_file = opal_path_access(files[i], NULL, mode)) ) {
-                opal_show_help("help-mca-var.txt", "missing-param-file",
-                               true, getpid(), files[i], path);
-                exit_status = OPAL_ERROR;
-                goto cleanup;
-            } else {
-                opal_argv_append(&argc, &argv, files[i]);
-            }
+        char *msg_path = path;
+        if (opal_path_is_absolute(files[i])) {
+            /* Absolute paths preserved */
+            tmp_file = opal_path_access(files[i], NULL, mode);
+        } else if (!rel_path_search && NULL != strchr(files[i], OPAL_PATH_SEP[0])) {
+            /* Resolve all relative paths:
+             *  - If filename contains a "/" (e.g., "./foo" or "foo/bar")
+             *    - look for it relative to cwd
+             *    - if exists, use it
+             *    - ow warn/error
+             */
+            msg_path = rel_path;
+            tmp_file = opal_path_access(files[i], rel_path, mode);
+        } else {
+            /* Resolve all relative paths:
+             * - Use path resolution
+             *    - if found and readable, use it
+             *    - otherwise, warn/error
+             */
+            tmp_file = opal_path_find (files[i], search_path, mode, NULL);
         }
-        /* Resolve all relative paths:
-         *  - If filename contains a "/" (e.g., "./foo" or "foo/bar")
-         *    - look for it relative to cwd
-         *    - if exists, use it
-         *    - ow warn/error
-         */
-        else if (!rel_path_search && NULL != strchr(files[i], OPAL_PATH_SEP[0]) ) {
-            if( NULL != force_agg_path ) {
-                tmp_file = opal_path_access(files[i], force_agg_path, mode);
-            }
-            else {
-                tmp_file = opal_path_access(files[i], cwd, mode);
-            }
 
-            if( NULL == tmp_file ) {
-                opal_show_help("help-mca-var.txt", "missing-param-file",
-                               true, getpid(), files[i], cwd);
-                exit_status = OPAL_ERROR;
-                goto cleanup;
-            }
-            else {
-                opal_argv_append(&argc, &argv, tmp_file);
-            }
-        }
-        /* Resolve all relative paths:
-         * - Use path resolution
-         *    - if found and readable, use it
-         *    - otherwise, warn/error
-         */
-        else {
-            if( NULL != (tmp_file = opal_path_find(files[i], search_path, mode, NULL)) ) {
-                opal_argv_append(&argc, &argv, tmp_file);
-            }
-            else {
-                opal_show_help("help-mca-var.txt", "missing-param-file",
-                               true, getpid(), files[i], path);
-                exit_status = OPAL_ERROR;
-                goto cleanup;
-            }
+        if (NULL == tmp_file) {
+            opal_show_help("help-mca-var.txt", "missing-param-file",
+                           true, getpid(), files[i], msg_path);
+            exit_status = OPAL_ERROR;
+            break;
         }
+
+        opal_argv_append(&argc, &argv, tmp_file);
+
         free(tmp_file);
+        tmp_file = NULL;
     }
 
-    tmp_file = NULL;
+    if (OPAL_SUCCESS == exit_status) {
+        free(*file_list);
+        *file_list = opal_argv_join(argv, sep);
+    }
 
-    free(*file_list);
-    *file_list = opal_argv_join(argv, sep);
-
- cleanup:
     if( NULL != files ) {
         opal_argv_free(files);
         files = NULL;
     }
+
     if( NULL != argv ) {
         opal_argv_free(argv);
         argv = NULL;
     }
+
     if( NULL != search_path ) {
         opal_argv_free(search_path);
         search_path = NULL;
     }
-    if( NULL != tmp_file ) {
-        free(tmp_file);
-    }
 
     return exit_status;
 }
@@ -1266,7 +1253,7 @@ static int fixup_files(char **file_list, char * path, bool rel_path_search, char
 static int read_files(char *file_list, opal_list_t *file_values, char sep)
 {
     char **tmp = opal_argv_split(file_list, sep);
-    int i, count, ret;
+    int i, count;
 
     if (!tmp) {
         return OPAL_ERR_OUT_OF_RESOURCE;
@@ -1349,6 +1336,16 @@ static int register_variable (const char *project_name, const char *framework_na
     }
 #endif
 
+    if (flags & MCA_BASE_VAR_FLAG_SYNONYM) {
+        original = opal_pointer_array_get_item (&mca_base_vars, synonym_for);
+        if (NULL == original) {
+            /* Attempting to create a synonym for a non-existent variable. probably a
+             * developer error. */
+            assert (NULL != original);
+            return OPAL_ERR_NOT_FOUND;
+        }
+    }
+
     /* There are data holes in the var struct */
     OPAL_DEBUG_ZERO(var);
 
@@ -1494,17 +1491,16 @@ static int register_variable (const char *project_name, const char *framework_na
 
     var->mbv_enumerator = enumerator;
 
-    if (flags & MCA_BASE_VAR_FLAG_SYNONYM) {
-        original = opal_pointer_array_get_item (&mca_base_vars, synonym_for);
-
-        opal_value_array_append_item(&original->mbv_synonyms, &var_index);
-    } else {
+    if (!original) {
         var->mbv_storage = storage;
 
         /* make a copy of the default string value */
         if ((MCA_BASE_VAR_TYPE_STRING == type || MCA_BASE_VAR_TYPE_VERSION_STRING == type) && NULL != ((char **)storage)[0]) {
             ((char **)storage)[0] = strdup (((char **)storage)[0]);
         }
+    } else {
+        /* synonym variable */
+        opal_value_array_append_item(&original->mbv_synonyms, &var_index);
     }
 
     /* go ahead and mark this variable as valid */
@@ -1718,7 +1714,6 @@ static int var_set_from_file (mca_base_var_t *var, mca_base_var_t *original, opa
     bool deprecated = VAR_IS_DEPRECATED(var[0]);
     bool is_synonym = VAR_IS_SYNONYM(var[0]);
     mca_base_var_file_value_t *fv;
-    int ret;
 
     /* Scan through the list of values read in from files and try to
        find a match.  If we do, cache it on the param (for future

opal/mca/base/mca_base_var_group.c

@@ -144,6 +144,40 @@ static int group_find_by_name (const char *full_name, int *index, bool invalidok
     return OPAL_ERR_NOT_FOUND;
 }
 
+static bool compare_strings (const char *str1, const char *str2) {
+    if ((NULL != str1 && 0 == strcmp (str1, "*")) ||
+        (NULL == str1 && NULL == str2)) {
+        return true;
+    }
+
+    if (NULL != str1 && NULL != str2) {
+        return 0 == strcmp (str1, str2);
+    }
+
+    return false;
+}
+
+static int group_find_linear (const char *project_name, const char *framework_name,
+                              const char *component_name, bool invalidok)
+{
+    for (int i = 0 ; i < mca_base_var_group_count ; ++i) {
+        mca_base_var_group_t *group;
+
+        int rc = mca_base_var_group_get_internal (i, &group, invalidok);
+        if (OPAL_SUCCESS != rc) {
+            continue;
+        }
+
+        if (compare_strings (project_name, group->group_project) &&
+            compare_strings (framework_name, group->group_framework) &&
+            compare_strings (component_name, group->group_component)) {
+            return i;
+        }
+    }
+
+    return OPAL_ERR_NOT_FOUND;
+}
+
 static int group_find (const char *project_name, const char *framework_name,
                        const char *component_name, bool invalidok)
 {
@@ -154,8 +188,11 @@ static int group_find (const char *project_name, const char *framework_name,
         return OPAL_ERR_NOT_FOUND;
     }
 
-    /* TODO -- deal with the project name correctly (including the wildcard '*') */
-    project_name = NULL;
+    /* check for wildcards */
+    if ((project_name && '*' == project_name[0]) || (framework_name && '*' == framework_name[0]) ||
+        (component_name && '*' == component_name[0])) {
+        return group_find_linear (project_name, framework_name, component_name, invalidok);
+    }
 
     ret = mca_base_var_generate_full_name4(project_name, framework_name, component_name,
                                            NULL, &full_name);
@@ -181,11 +218,6 @@ static int group_register (const char *project_name, const char *framework_name,
         return -1;
     }
 
-    /* XXX -- remove this once the project name is available in the component structure */
-    if (framework_name || component_name) {
-        project_name = NULL;
-    }
-
     group_id = group_find (project_name, framework_name, component_name, true);
     if (0 <= group_id) {
         ret = mca_base_var_group_get_internal (group_id, &group, true);