opal/crs: clean up parsing code to fix coverity issues

CID 70622 Dereference before null check (REVERSE_INULL)
CID 70459 Logically dead code (DEADCODE)

Cleanup some cludgy code which (among other things) reimplemented
strcat, strdup, and strchr. In the process this resolved two
outstanding coverity issues.

CID 70631 Dereference before null check (REVERSE_INULL)

best_module can not be NULL in this code path. Remove NULL check and
unnecessary goto statements.

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

opal/mca/crs/base/crs_base_fns.c

@@ -1,3 +1,4 @@
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
  * Copyright (c) 2004-2010 The Trustees of Indiana University.
  *                         All rights reserved.
@@ -11,6 +12,8 @@
  * Copyright (c) 2015      Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
+ * Copyright (c) 2015      Los Alamos National Security, LLC. All rights
+ *                         reserved.
  * $COPYRIGHT$
  * 
  * Additional copyrights may follow
@@ -359,122 +362,98 @@ int opal_crs_base_self_register_continue_callback(opal_crs_base_self_continue_fn
 static int metadata_extract_next_token(FILE *file, char **token, char **value)
 {
     int exit_status = OPAL_SUCCESS;
-    int max_len = 256;
+    const int max_len = 256;
-    char * line = NULL;
+    /* NTH: as long as max_len remains small (256 bytes) there is no need
-    int line_len = 0;
+     * to allocate line on the heap */
-    int c = 0, s = 0, v = 0;
+    char line[max_len];
-    char *local_token = NULL;
+    int line_len = 0, value_len;
     char *local_value = NULL;
     bool end_of_line = false;
+    char *tmp;
 
-    line = (char *) malloc(sizeof(char) * max_len);
-
- try_again:
     /*
      * If we are at the end of the file, then just return
      */
-    if(0 != feof(file) ) {
+    do {
-        exit_status = OPAL_ERROR;
+        /*
-        goto cleanup;
+         * Other wise grab the next token/value pair
-    }
+         */
+        if (NULL == fgets(line, max_len, file) ) {
+            /* the calling code doesn't distinguish error types so
+             * returning OPAL_ERROR on error or EOF is ok. if this
+             * changes re-add the check for EOF. */
+            return OPAL_ERROR;
+        }
 
-    /*
+        line_len = strlen(line);
-     * Other wise grab the next token/value pair
-     */
-    if (NULL == fgets(line, max_len, file) ) {
-        exit_status = OPAL_ERROR;
-        goto cleanup;
-    }
-    line_len = strlen(line);
-    /* Strip off the new line if it it there */
-    if('\n' == line[line_len-1]) {
-        line[line_len-1] = '\0';
-        line_len--;
-        end_of_line = true;
-    }
-    else {
-        end_of_line = false;
-    }
 
-    /* Ignore lines with just '#' too */
+        /* Strip off the new line if it is there */
-    if(2 >= line_len)
+        end_of_line = ('\n' == line[line_len-1]);
-        goto try_again;
+
+        if (end_of_line) {
+            line[--line_len] = '\0';
+        }
+
+        /* Ignore lines with just '#' too */
+    } while (line_len <= 2);
     
     /*
      * Extract the token from the set
      */
-    for(c = 0; 
+    tmp = strchr (line, ':');
-        line[c] != ':' && 
+    if (!tmp) {
-            c < line_len;
+        /* no separator */
-        ++c) {
+        return OPAL_ERROR;
-        ;
-    }
-    c += 2; /* For the ' ' and the '\0' */
-    local_token = (char *)malloc(sizeof(char) * (c + 1));
-
-    for(s = 0; s < c; ++s) {
-        local_token[s] = line[s];
     }
 
-    local_token[s] = '\0';
+    tmp = '\0';
-    *token = strdup(local_token);
 
-    if( NULL != local_token) {
+    *token = strdup (line);
-        free(local_token);
+    local_value = strdup (tmp + 1);
-        local_token = NULL;
+
+    if (NULL == *token || NULL == local_value) {
+        if (local_value) {
+            free (local_value);
+        }
+
+        return OPAL_ERR_OUT_OF_RESOURCE;
     }
 
+    value_len = strlen (local_value) + 1;
+
     /*
      * Extract the value from the set
      */
-    local_value = (char *)malloc(sizeof(char) * (line_len - c + 1));
-    for(v = 0, s = c; 
-        s < line_len;
-        ++s, ++v) {
-        local_value[v] = line[s];
-    }
-
     while(!end_of_line) {
         if (NULL == fgets(line, max_len, file) ) {
             exit_status = OPAL_ERROR;
-            goto cleanup;
+            break;
         }
+
         line_len = strlen(line);
-        /* Strip off the new line if it it there */
+
-        if('\n' == line[line_len-1]) {
+        /* Strip off the new line if it is there */
-            line[line_len-1] = '\0';
+        end_of_line = ('\n' == line[line_len-1]);
-            line_len--;
+
-            end_of_line = true;
+        if (end_of_line) {
-        }
+            line[--line_len] = '\0';
-        else {
-            end_of_line = false;
         }
+
+        value_len += line_len;
         
-        local_value = (char *)realloc(local_value, sizeof(char) * line_len);
+        tmp = (char *) realloc(local_value, value_len);
-        for(s = 0;
+        if (NULL == tmp) {
-            s < line_len;
+            exit_status = OPAL_ERR_OUT_OF_RESOURCE;
-            ++s, ++v) {
+            break;
-            local_value[v] = line[s];
         }
+
+        strcat (local_value, line);
     }
 
-    local_value[v] = '\0';
+    if (OPAL_SUCCESS == exit_status) {
-    *value = strdup(local_value);
+        *value = local_value;
-
+    } else {
- cleanup:
+        free (local_value);
-    if( NULL != local_token) {
-        free(local_token);
-        local_token = NULL;
-    }
-
-    if( NULL != local_value) {
-        free(local_value);
-        local_value = NULL;
-    }
-
-    if( NULL != line) {
-        free(line);
-        line = NULL;
     }
 
     return exit_status;

opal/mca/crs/base/crs_base_select.c

@@ -1,3 +1,4 @@
+/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
  * Copyright (c) 2004-2010 The Trustees of Indiana University.
  *                         All rights reserved.
@@ -8,6 +9,8 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2007      Evergrid, Inc. All rights reserved.
+ * Copyright (c) 2015      Los Alamos National Security, LLC. All rights
+ *                         reserved.
  *
  * $COPYRIGHT$
  * 
@@ -33,9 +36,9 @@ bool opal_crs_base_do_not_select = false;
 
 int opal_crs_base_select(void)
 {
-    int ret, exit_status = OPAL_SUCCESS;
     opal_crs_base_component_t *best_component = NULL;
     opal_crs_base_module_t *best_module = NULL;
+    int ret;
 
     if( !opal_cr_is_enabled ) {
         opal_output_verbose(10, opal_crs_base_framework.framework_output,
@@ -57,22 +60,19 @@ int opal_crs_base_select(void)
                                         (mca_base_module_t **) &best_module,
                                         (mca_base_component_t **) &best_component) ) {
         /* This will only happen if no component was selected */
-        exit_status = OPAL_ERROR;
+        return OPAL_ERROR;
-        goto cleanup;
     }
 
+    /* best_module and best_component should not be NULL here */
+
     /* Save the winner */
     opal_crs_base_selected_component = *best_component;
     opal_crs = *best_module;
 
     /* Initialize the winner */
-    if (NULL != best_module) {
+    if (OPAL_SUCCESS != (ret = opal_crs.crs_init()) ) {
-        if (OPAL_SUCCESS != (ret = opal_crs.crs_init()) ) {
+        return ret;
-            exit_status = ret;
-            goto cleanup;
-        }
     }
 
- cleanup:
+    return OPAL_SUCCESS;
-    return exit_status;
 }