Code cleanup while chasing valgrind complaints.

This commit was SVN r30048.

opal/mca/db/base/db_base_fns.c

@@ -34,7 +34,6 @@ int opal_db_base_store(const opal_identifier_t *proc,
                        const char *key, const void *object,
                        opal_data_type_t type)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc;
 
@@ -43,14 +42,12 @@ int opal_db_base_store(const opal_identifier_t *proc,
     }
 
     /* cycle thru the active modules until one agrees to perform the op */
-    did_op = false;
     OPAL_LIST_FOREACH(mod, &opal_db_base.store_order, opal_db_active_module_t) {
         if (NULL == mod->module->store) {
             continue;
         }
         if (OPAL_SUCCESS == (rc = mod->module->store(proc, scope, key, object, type))) {
-            did_op = true;
+            return OPAL_SUCCESS;
-            break;
         }
         /* modules return "next option" if they didn't perform
          * the operation - anything else is a true error.
@@ -61,18 +58,14 @@ int opal_db_base_store(const opal_identifier_t *proc,
         }
     }
 
-    /* if we get here without performing the operation, that's an error */
+    /* we did not perform any operation, that's an error */
-    if (!did_op) {
+    OPAL_ERROR_LOG(OPAL_ERROR);
-        OPAL_ERROR_LOG(OPAL_ERROR);
+    return OPAL_ERROR;
-        return OPAL_ERROR;
-    }
-    return OPAL_SUCCESS;
 }
 
 int opal_db_base_store_pointer(const opal_identifier_t *proc,
                                opal_value_t *kv)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc;
 
@@ -81,14 +74,12 @@ int opal_db_base_store_pointer(const opal_identifier_t *proc,
     }
 
     /* cycle thru the active modules until one agrees to perform the op */
-    did_op = false;
     OPAL_LIST_FOREACH(mod, &opal_db_base.store_order, opal_db_active_module_t) {
         if (NULL == mod->module->store_pointer) {
             continue;
         }
         if (OPAL_SUCCESS == (rc = mod->module->store_pointer(proc, kv))) {
-            did_op = true;
+            return OPAL_SUCCESS;
-            break;
         }
         /* modules return "next option" if they didn't perform
          * the operation - anything else is a true error.
@@ -99,12 +90,9 @@ int opal_db_base_store_pointer(const opal_identifier_t *proc,
         }
     }
 
-    /* if we get here without performing the operation, that's an error */
+    /* we did not perform any operation, that's an error */
-    if (!did_op) {
+    OPAL_ERROR_LOG(OPAL_ERROR);
-        OPAL_ERROR_LOG(OPAL_ERROR);
+    return OPAL_ERROR;
-        return OPAL_ERROR;
-    }
-    return OPAL_SUCCESS;
 }
 
 void opal_db_base_commit(const opal_identifier_t *proc)
@@ -125,7 +113,6 @@ int opal_db_base_fetch(const opal_identifier_t *proc,
                        const char *key, void **data,
                        opal_data_type_t type)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc, i;
 
@@ -133,20 +120,18 @@ int opal_db_base_fetch(const opal_identifier_t *proc,
         return OPAL_ERR_FATAL;
     }
 
-    /* cycle thru the active modules until one agrees to perform the op */
+    /* cycle thru the active modules until one agrees to perform the op.
-    did_op = false;
+     * we cycle thru the list of modules twice - this allows us to check
-    /* we cycle thru the list of modules twice - this allows us to check
      * a local store first, then attempt to obtain the data from an
      * external store that puts it in the local store
      */
-    for(i=0; i < 2 && !did_op; i++) {
+    for(i = 0; i < 2; i++) {
         OPAL_LIST_FOREACH(mod, &opal_db_base.fetch_order, opal_db_active_module_t) {
             if (NULL == mod->module->fetch) {
                 continue;
             }
             if (OPAL_SUCCESS == (rc = mod->module->fetch(proc, key, data, type))) {
-                did_op = true;
+                return OPAL_SUCCESS;
-                break;
             }
             /* modules return "next option" if they didn't perform
              * the operation - anything else is a true error.
@@ -158,17 +143,13 @@ int opal_db_base_fetch(const opal_identifier_t *proc,
     }
 
     /* if we get here without performing the operation, that's an error */
-    if (!did_op) {
+    return OPAL_ERR_DATA_VALUE_NOT_FOUND;
-        return OPAL_ERR_DATA_VALUE_NOT_FOUND;
-    }
-    return OPAL_SUCCESS;
 }
 
 int opal_db_base_fetch_pointer(const opal_identifier_t *proc,
                                const char *key,
                                void **data, opal_data_type_t type)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc, i;
 
@@ -176,20 +157,18 @@ int opal_db_base_fetch_pointer(const opal_identifier_t *proc,
         return OPAL_ERR_FATAL;
     }
 
-    /* cycle thru the active modules until one agrees to perform the op */
+    /* cycle thru the active modules until one agrees to perform the op.
-    did_op = false;
+     * we cycle thru the list of modules twice - this allows us to check
-    /* we cycle thru the list of modules twice - this allows us to check
      * a local store first, then attempt to obtain the data from an
      * external store that puts it in the local store
      */
-    for(i=0; i < 2 && !did_op; i++) {
+    for(i = 0; i < 2; i++) {
         OPAL_LIST_FOREACH(mod, &opal_db_base.fetch_order, opal_db_active_module_t) {
             if (NULL == mod->module->fetch_pointer) {
                 continue;
             }
             if (OPAL_SUCCESS == (rc = mod->module->fetch_pointer(proc, key, data, type))) {
-                did_op = true;
+                return OPAL_SUCCESS;
-                break;
             }
             /* modules return "next option" if they didn't perform
              * the operation - anything else is a true error.
@@ -201,10 +180,7 @@ int opal_db_base_fetch_pointer(const opal_identifier_t *proc,
     }
 
     /* if we get here without performing the operation, that's an error */
-    if (!did_op) {
+    return OPAL_ERR_DATA_VALUE_NOT_FOUND;
-        return OPAL_ERR_DATA_VALUE_NOT_FOUND;
-    }
-    return OPAL_SUCCESS;
 }
 
 int opal_db_base_fetch_multiple(const opal_identifier_t *proc,
@@ -212,7 +188,6 @@ int opal_db_base_fetch_multiple(const opal_identifier_t *proc,
                                 const char *key,
                                 opal_list_t *kvs)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc;
 
@@ -221,14 +196,12 @@ int opal_db_base_fetch_multiple(const opal_identifier_t *proc,
     }
 
     /* cycle thru the active modules until one agrees to perform the op */
-    did_op = false;
     OPAL_LIST_FOREACH(mod, &opal_db_base.fetch_order, opal_db_active_module_t) {
         if (NULL == mod->module->fetch_multiple) {
             continue;
         }
         if (OPAL_SUCCESS == (rc = mod->module->fetch_multiple(proc, scope, key, kvs))) {
-            did_op = true;
+            return OPAL_SUCCESS;
-            break;
         }
         /* modules return "next option" if they didn't perform
          * the operation - anything else is a true error.
@@ -239,28 +212,22 @@ int opal_db_base_fetch_multiple(const opal_identifier_t *proc,
     }
 
     /* if we get here without performing the operation, that's an error */
-    if (!did_op) {
+    return OPAL_ERR_DATA_VALUE_NOT_FOUND;
-        return OPAL_ERR_DATA_VALUE_NOT_FOUND;
-    }
-    return OPAL_SUCCESS;
 }
 
 int opal_db_base_remove_data(const opal_identifier_t *proc,
                              const char *key)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc;
 
     /* cycle thru the actiove modules until one agrees to perform the op */
-    did_op = false;
     OPAL_LIST_FOREACH(mod, &opal_db_base.store_order, opal_db_active_module_t) {
         if (NULL == mod->module->remove) {
             continue;
         }
         if (OPAL_SUCCESS == (rc = mod->module->remove(proc, key))) {
-            did_op = true;
+            return OPAL_SUCCESS;
-            break;
         }
         /* modules return "next option" if they didn't perform
          * the operation - anything else is a true error.
@@ -272,30 +239,24 @@ int opal_db_base_remove_data(const opal_identifier_t *proc,
     }
 
     /* if we get here without performing the operation, that's an error */
-    if (!did_op) {
+    OPAL_ERROR_LOG(OPAL_ERROR);
-        OPAL_ERROR_LOG(OPAL_ERROR);
+    return OPAL_ERROR;
-        return OPAL_ERROR;
-    }
-    return OPAL_SUCCESS;
 }
 
 
 int opal_db_base_add_log(const char *table,
                          const opal_value_t *kvs, int nkvs)
 {
-    bool did_op;
     opal_db_active_module_t *mod;
     int rc;
 
     /* cycle thru the active modules until one agrees to perform the op */
-    did_op = false;
     OPAL_LIST_FOREACH(mod, &opal_db_base.store_order, opal_db_active_module_t) {
         if (NULL == mod->module->add_log) {
             continue;
         }
         if (OPAL_SUCCESS == (rc = mod->module->add_log(table, kvs, nkvs))) {
-            did_op = true;
+            return OPAL_SUCCESS;
-            break;
         }
         /* modules return "next option" if they didn't perform
          * the operation - anything else is a true error.
@@ -307,9 +268,5 @@ int opal_db_base_add_log(const char *table,
     }
 
     /* if we get here without performing the operation, let the caller know */
-    if (!did_op) {
+    return OPAL_ERROR;
-        /* don't error log it here */
-        return OPAL_ERROR;
-    }
-    return OPAL_SUCCESS;
 }

opal/util/output.c

@@ -344,13 +344,6 @@ void opal_output_close(int output_id)
 #endif
     }
 
-    /* Somewhat of a hack to free up the temp_str */
-
-    if (NULL != temp_str) {
-        free(temp_str);
-        temp_str = NULL;
-        temp_str_len = 0;
-    }
     OPAL_THREAD_UNLOCK(&mutex);
 }
 
@@ -492,6 +485,11 @@ void opal_output_finalize(void)
 
         free (output_prefix);
         free (output_dir);
+        if(NULL != temp_str) {
+	    free(temp_str);
+	    temp_str = NULL;
+	    temp_str_len = 0;
+	}
         OBJ_DESTRUCT(&verbose);
         OBJ_DESTRUCT(&mutex);
     }
@@ -745,14 +743,12 @@ static int open_file(int i)
 
         /* Actually open the file */
         info[i].ldi_fd = open(filename, flags, 0644);
+        free(filename);  /* release the filename in all cases */
         if (-1 == info[i].ldi_fd) {
             info[i].ldi_used = false;
-            free(filename);
             return OPAL_ERR_IN_ERRNO;
         }
 
-        free(filename);
-
         /* Make the file be close-on-exec to prevent child inheritance
          * problems */
         if (-1 == fcntl(info[i].ldi_fd, F_SETFD, 1)) {

orte/mca/oob/tcp/oob_tcp_connection.c

@@ -325,6 +325,7 @@ static int tcp_peer_send_connect_ack(mca_oob_tcp_module_t *mod,
     hdr.dst = peer->name;
     hdr.type = MCA_OOB_TCP_IDENT;
     hdr.tag = 0;
+    hdr.nbytes = 0;
     MCA_OOB_TCP_HDR_HTON(&hdr);
     if (0 > tcp_peer_send_blocking(mod, peer, &hdr, sizeof(hdr))) {
         ORTE_ERROR_LOG(ORTE_ERR_UNREACH);