Merge pull request open-mpi#6957 from rhc54/topic/buf

Ensure buffer_unload leaves the buffer in a clean state

Author: ggouaillardet

opal/dss/dss_load_unload.c

@@ -9,7 +9,7 @@
  *                         University of Stuttgart.  All rights reserved.
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
- * Copyright (c) 2014-2018 Intel, Inc. All rights reserved.
+ * Copyright (c) 2014-2019 Intel, Inc.  All rights reserved.
  * Copyright (c) 2015-2017 Research Organization for Information Science
  *                         and Technology (RIST). All rights reserved.
  * Copyright (c) 2018      IBM Corporation.  All rights reserved.
@@ -57,10 +57,8 @@ int opal_dss_unload(opal_buffer_t *buffer, void **payload,
         *payload = buffer->base_ptr;
         *bytes_used = buffer->bytes_used;
         buffer->base_ptr = NULL;
-        buffer->unpack_ptr = NULL;
-        buffer->pack_ptr = NULL;
         buffer->bytes_used = 0;
-        return OPAL_SUCCESS;
+        goto cleanup;
     }
 
     /* okay, we have something to provide - pass it back */
@@ -74,8 +72,10 @@ int opal_dss_unload(opal_buffer_t *buffer, void **payload,
         memcpy(*payload, buffer->unpack_ptr, *bytes_used);
     }
 
-    /* All done */
-
+  cleanup:
+    /* All done - reset the buffer */
+    OBJ_DESTRUCT(buffer);
+    OBJ_CONSTRUCT(buffer, opal_buffer_t);
     return OPAL_SUCCESS;
 }
 
@@ -89,17 +89,11 @@ int opal_dss_load(opal_buffer_t *buffer, void *payload,
     }
 
     /* check if buffer already has payload - free it if so */
-    if (NULL != buffer->base_ptr) {
-        free(buffer->base_ptr);
-    }
+    OBJ_DESTRUCT(buffer);
+    OBJ_CONSTRUCT(buffer, opal_buffer_t);
 
     /* if it's a NULL payload, just set things and return */
     if (NULL == payload) {
-        buffer->base_ptr = NULL;
-        buffer->pack_ptr = buffer->base_ptr;
-        buffer->unpack_ptr = buffer->base_ptr;
-        buffer->bytes_used = 0;
-        buffer->bytes_allocated = 0;
         return OPAL_SUCCESS;
     }
 

orte/util/nidmap.c

@@ -603,7 +603,7 @@ int orte_util_pass_node_info(opal_buffer_t *buffer)
         } else {
             /* mark that this was not compressed */
             compressed = false;
-            bo.bytes = bucket.base_ptr;
+            bo.bytes = (uint8_t*)bucket.base_ptr;
             bo.size = bucket.bytes_used;
         }
         /* indicate compression */

test/class/opal_tree.c

@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2011      Oracle and/or its affiliates.  All rights reserved.
  * Copyright (c) 2014 Cisco Systems, Inc.  All rights reserved.
+ * Copyright (c) 2019      Intel, Inc.  All rights reserved.
  * $COPYRIGHT$
  *
  * Additional copyrights may follow
@@ -214,8 +215,7 @@ int main(int argc, char **argv)
     /* serialize tree */
     serial_tree = OBJ_NEW(opal_buffer_t);
 
-    if (OPAL_SUCCESS == opal_tree_serialize(opal_tree_get_root(&tree),
-					    serial_tree)) {
+    if (OPAL_SUCCESS == opal_tree_serialize(opal_tree_get_root(&tree), serial_tree)) {
         opal_tree_t tmp_tree;
         opal_buffer_t *serial2_tree;
 
@@ -227,29 +227,28 @@ int main(int argc, char **argv)
         /* deserialize tree */
         opal_tree_deserialize(serial_tree, &(tmp_tree.opal_tree_sentinel));
         /* serialize tmp tree */
-	serial2_tree = OBJ_NEW(opal_buffer_t);
-        if (OPAL_SUCCESS == opal_tree_serialize(opal_tree_get_root(&tmp_tree),
-						serial2_tree)) {
-	    void *payload1, *payload2;
-	    int32_t size1, size2;
-
-	    /* compare new with original serialization */
-        serial_tree->unpack_ptr = serial_tree->base_ptr;
-        serial2_tree->unpack_ptr = serial2_tree->unpack_ptr;
-	    opal_dss.unload(serial_tree, &payload1, &size1);
-	    opal_dss.unload(serial2_tree, &payload2, &size2);
-	    if (size1 == size2) {
-		if (0 == memcmp(payload1, payload2, size1)) {
-		    test_success();
-		} else {
-		    test_failure(" failed tree deserialization data compare");
-		}
-	    } else {
-		test_failure(" failed tree deserialization size compare");
-	    }
-	} else {
-	    test_failure(" failed tree second pass serialization");
-	}
+        serial2_tree = OBJ_NEW(opal_buffer_t);
+        if (OPAL_SUCCESS == opal_tree_serialize(opal_tree_get_root(&tmp_tree), serial2_tree)) {
+            void *payload1, *payload2;
+            int32_t size1, size2;
+
+	       /* compare new with original serialization */
+            serial_tree->unpack_ptr = serial_tree->base_ptr;
+            serial2_tree->unpack_ptr = serial2_tree->unpack_ptr;
+            opal_dss.unload(serial_tree, &payload1, &size1);
+            opal_dss.unload(serial2_tree, &payload2, &size2);
+            if (size1 == size2) {
+              if (0 == memcmp(payload1, payload2, size1)) {
+                  test_success();
+              } else {
+                  test_failure(" failed tree deserialization data compare");
+              }
+            } else {
+                test_failure(" failed tree deserialization size compare");
+            }
+    	} else {
+    	    test_failure(" failed tree second pass serialization");
+    	}
     } else {
         test_failure(" failed tree serialization");
     }