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

CID 1047278 Unchecked return value

Updated check for mca_base_var_generate_full_name4 to match other
checks. Logically equivalent to the old check. Not a bug.

CID 1196685 Dereference null return

Added check for NULL when looking up the original variable for a
synonym.

CID 1269705 Logically dead code

Removed code that set the project to NULL. Code was intended to be
removed with an earlier commit that added the project name into the
component structure. Added code to actually support searching for a
group with a wildcard ('*').

CID 1292739 Dereference null return
CID 1269819 Dereference null return

Removed unnecessary string duplication and strchr.

CID 1287030 Logically dead code

Refactored fixup_files code and confirmed that the code in question is
not reachable. Removed the dead code.

CID 1292740 Use of untrusted string

Use strdup to silence coverity warning.

CID 1294413 Free of address-of expression

Reset mitem to NULL after the OPAL_LIST_FOREACH loop to ensure we
never try to free the list sentinel.

CID 1294414 Unchecked return value

Use (void) to indicate we do not care about the return code in this
instance.

CID 1294415 Resource leak

On error free all the base pointer.

Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>

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);