Fix issue with copying null new references (#1440)

* Fix issue with copying null new references. Fix assertion failure when
reference copying fails.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

5 files changed, 57 insertions, 49 deletions

diff --git a/src/H5Ocache.c b/src/H5Ocache.c
index ba47da3..c7586cc 100644
--- a/src/H5Ocache.c
+++ b/src/H5Ocache.c
@@ -346,7 +346,7 @@ H5O__cache_deserialize(const void *image, size_t len, void *_udata, hbool_t *dir
 done:
     /* Release the [possibly partially initialized] object header on errors */
     if (!ret_value && oh)
-        if (H5O__free(oh) < 0)
+        if (H5O__free(oh, FALSE) < 0)
             HDONE_ERROR(H5E_OHDR, H5E_CANTRELEASE, NULL, "unable to destroy object header data")
 
     FUNC_LEAVE_NOAPI(ret_value)
@@ -639,7 +639,7 @@ H5O__cache_free_icr(void *_thing)
     HDassert(oh->cache_info.type == H5AC_OHDR);
 
     /* Destroy object header */
-    if (H5O__free(oh) < 0)
+    if (H5O__free(oh, FALSE) < 0)
         HGOTO_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "can't destroy object header")
 
 done:
@@ -1242,7 +1242,7 @@ H5O__prefix_deserialize(const uint8_t *_image, H5O_cache_ud_t *udata)
 
         /* Save the object header for later use in 'deserialize' callback */
         udata->oh = oh;
-        if (H5O__free(saved_oh) < 0)
+        if (H5O__free(saved_oh, FALSE) < 0)
             HGOTO_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "can't destroy object header")
         udata->free_oh = FALSE;
     }
@@ -1255,7 +1255,7 @@ H5O__prefix_deserialize(const uint8_t *_image, H5O_cache_ud_t *udata)
 done:
     /* Release the [possibly partially initialized] object header on errors */
     if (ret_value < 0 && oh)
-        if (H5O__free(oh) < 0)
+        if (H5O__free(oh, FALSE) < 0)
             HDONE_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "unable to destroy object header data")
 
     FUNC_LEAVE_NOAPI(ret_value)
diff --git a/src/H5Ocopy.c b/src/H5Ocopy.c
index 05dfc72..52b1b5c 100644
--- a/src/H5Ocopy.c
+++ b/src/H5Ocopy.c
@@ -772,7 +772,7 @@ done:
     /* Free destination object header on failure */
     if (ret_value < 0) {
         if (oh_dst && !inserted) {
-            if (H5O__free(oh_dst) < 0)
+            if (H5O__free(oh_dst, TRUE) < 0)
                 HDONE_ERROR(H5E_OHDR, H5E_CANTFREE, FAIL, "unable to destroy object header data")
             if (H5O_loc_reset(oloc_dst) < 0)
                 HDONE_ERROR(H5E_OHDR, H5E_CANTFREE, FAIL, "unable to destroy object header data")
diff --git a/src/H5Ocopy_ref.c b/src/H5Ocopy_ref.c
index f1f8aaf..1cda3ea 100644
--- a/src/H5Ocopy_ref.c
+++ b/src/H5Ocopy_ref.c
@@ -288,21 +288,22 @@ H5O__copy_expand_ref_object2(H5O_loc_t *src_oloc, hid_t tid_src, const H5T_t *dt
                              size_t nbytes_src, H5O_loc_t *dst_oloc, H5G_loc_t *dst_root_loc, void *buf_dst,
                              size_t ref_count, H5O_copy_t *cpy_info)
 {
-    H5T_t *     dt_mem        = NULL;                        /* Memory datatype */
-    H5T_t *     dt_dst        = NULL;                        /* Destination datatype */
-    hid_t       tid_mem       = H5I_INVALID_HID;             /* Datatype ID for memory datatype */
-    hid_t       tid_dst       = H5I_INVALID_HID;             /* Datatype ID for memory datatype */
-    H5T_path_t *tpath_src_mem = NULL, *tpath_mem_dst = NULL; /* Datatype conversion paths */
-    size_t      i;                                           /* Local index variable */
-    hbool_t     reg_tid_src   = (tid_src == H5I_INVALID_HID);
-    hid_t       dst_loc_id    = H5I_INVALID_HID;
-    void *      conv_buf      = NULL;        /* Buffer for converting data */
-    size_t      conv_buf_size = 0;           /* Buffer size */
-    void *      reclaim_buf   = NULL;        /* Buffer for reclaiming data */
-    H5S_t *     buf_space     = NULL;        /* Dataspace describing buffer */
-    hsize_t     buf_dim[1]    = {ref_count}; /* Dimension for buffer */
-    size_t      token_size    = H5F_SIZEOF_ADDR(src_oloc->file);
-    herr_t      ret_value     = SUCCEED;
+    H5T_t *             dt_mem        = NULL;                        /* Memory datatype */
+    H5T_t *             dt_dst        = NULL;                        /* Destination datatype */
+    hid_t               tid_mem       = H5I_INVALID_HID;             /* Datatype ID for memory datatype */
+    hid_t               tid_dst       = H5I_INVALID_HID;             /* Datatype ID for memory datatype */
+    H5T_path_t *        tpath_src_mem = NULL, *tpath_mem_dst = NULL; /* Datatype conversion paths */
+    size_t              i;                                           /* Local index variable */
+    hbool_t             reg_tid_src             = (tid_src == H5I_INVALID_HID);
+    hid_t               dst_loc_id              = H5I_INVALID_HID;
+    void *              conv_buf                = NULL;        /* Buffer for converting data */
+    size_t              conv_buf_size           = 0;           /* Buffer size */
+    void *              reclaim_buf             = NULL;        /* Buffer for reclaiming data */
+    H5S_t *             buf_space               = NULL;        /* Dataspace describing buffer */
+    hsize_t             buf_dim[1]              = {ref_count}; /* Dimension for buffer */
+    size_t              token_size              = H5F_SIZEOF_ADDR(src_oloc->file);
+    const unsigned char zeros[H5R_REF_BUF_SIZE] = {0};
+    herr_t              ret_value               = SUCCEED;
 
     FUNC_ENTER_STATIC
 
@@ -353,29 +354,34 @@ H5O__copy_expand_ref_object2(H5O_loc_t *src_oloc, hid_t tid_src, const H5T_t *dt
 
     /* Making equivalent references in the destination file */
     for (i = 0; i < ref_count; i++) {
-        H5R_ref_t *     ref_ptr   = (H5R_ref_t *)conv_buf;
-        H5R_ref_priv_t *ref       = (H5R_ref_priv_t *)&ref_ptr[i];
-        H5O_token_t     tmp_token = {0};
-
-        /* Get src object address */
-        if (H5R__get_obj_token(ref, &tmp_token, &token_size) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to get object token")
-        if (H5VL_native_token_to_addr(src_oloc->file, H5I_FILE, tmp_token, &src_oloc->addr) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTUNSERIALIZE, FAIL, "can't deserialize object token into address")
-
-        /* Attempt to copy object from source to destination file */
-        if (H5O__copy_obj_by_ref(src_oloc, dst_oloc, dst_root_loc, cpy_info) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object")
-
-        /* Set dst object address */
-        if (H5VL_native_addr_to_token(dst_oloc->file, H5I_FILE, dst_oloc->addr, &tmp_token) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTSERIALIZE, FAIL, "can't serialize address into object token")
-        if (H5R__set_obj_token(ref, (const H5O_token_t *)&tmp_token, token_size) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "unable to set object token")
-        /* Do not set app_ref since references are released once the copy is done */
-        if (H5R__set_loc_id(ref, dst_loc_id, TRUE, FALSE) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "unable to set destination loc id")
-    } /* end for */
+        H5R_ref_t *     ref_ptr = (H5R_ref_t *)conv_buf;
+        H5R_ref_priv_t *ref     = (H5R_ref_priv_t *)&ref_ptr[i];
+
+        /* Check for null reference - only expand reference if it is not null */
+        if (HDmemcmp(ref, zeros, H5R_REF_BUF_SIZE)) {
+            H5O_token_t tmp_token = {0};
+
+            /* Get src object address */
+            if (H5R__get_obj_token(ref, &tmp_token, &token_size) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to get object token")
+            if (H5VL_native_token_to_addr(src_oloc->file, H5I_FILE, tmp_token, &src_oloc->addr) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTUNSERIALIZE, FAIL,
+                            "can't deserialize object token into address")
+
+            /* Attempt to copy object from source to destination file */
+            if (H5O__copy_obj_by_ref(src_oloc, dst_oloc, dst_root_loc, cpy_info) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object")
+
+            /* Set dst object address */
+            if (H5VL_native_addr_to_token(dst_oloc->file, H5I_FILE, dst_oloc->addr, &tmp_token) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTSERIALIZE, FAIL, "can't serialize address into object token")
+            if (H5R__set_obj_token(ref, (const H5O_token_t *)&tmp_token, token_size) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "unable to set object token")
+            /* Do not set app_ref since references are released once the copy is done */
+            if (H5R__set_loc_id(ref, dst_loc_id, TRUE, FALSE) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "unable to set destination loc id")
+        } /* end if */
+    }     /* end for */
 
     /* Copy into another buffer, to reclaim memory later */
     if (NULL == (reclaim_buf = H5FL_BLK_MALLOC(type_conv, conv_buf_size)))
diff --git a/src/H5Oint.c b/src/H5Oint.c
index ee79b0c..2348790 100644
--- a/src/H5Oint.c
+++ b/src/H5Oint.c
@@ -289,7 +289,7 @@ H5O_create(H5F_t *f, size_t size_hint, size_t initial_rc, hid_t ocpl_id, H5O_loc
         HGOTO_ERROR(H5E_OHDR, H5E_BADVALUE, FAIL, "Can't apply object header to file")
 
 done:
-    if ((FAIL == ret_value) && (NULL != oh) && (H5O__free(oh) < 0))
+    if ((FAIL == ret_value) && (NULL != oh) && (H5O__free(oh, TRUE) < 0))
         HDONE_ERROR(H5E_OHDR, H5E_CANTFREE, FAIL, "can't delete object header")
 
     FUNC_LEAVE_NOAPI(ret_value)
@@ -353,7 +353,7 @@ H5O_create_ohdr(H5F_t *f, hid_t ocpl_id)
     ret_value = oh;
 
 done:
-    if ((NULL == ret_value) && (NULL != oh) && (H5O__free(oh) < 0))
+    if ((NULL == ret_value) && (NULL != oh) && (H5O__free(oh, TRUE) < 0))
         HDONE_ERROR(H5E_OHDR, H5E_CANTFREE, NULL, "can't delete object header")
 
     FUNC_LEAVE_NOAPI(ret_value)
@@ -3014,7 +3014,7 @@ H5O_get_proxy(const H5O_t *oh)
  *-------------------------------------------------------------------------
  */
 herr_t
-H5O__free(H5O_t *oh)
+H5O__free(H5O_t *oh, hbool_t force)
 {
     unsigned u;                   /* Local index variable */
     herr_t   ret_value = SUCCEED; /* Return value */
@@ -3038,10 +3038,12 @@ H5O__free(H5O_t *oh)
         for (u = 0; u < oh->nmesgs; u++) {
 #ifndef NDEBUG
             /* Verify that message is clean, unless it could have been marked
-             * dirty by decoding */
+             * dirty by decoding, or if this is a forced free (in case of
+             * failure during creation of the object some messages may be dirty)
+             */
             if (oh->ndecode_dirtied && oh->mesg[u].dirty)
                 oh->ndecode_dirtied--;
-            else
+            else if (!force)
                 HDassert(oh->mesg[u].dirty == 0);
 #endif /* NDEBUG */
 
diff --git a/src/H5Opkg.h b/src/H5Opkg.h
index ebfe636..1fe918d 100644
--- a/src/H5Opkg.h
+++ b/src/H5Opkg.h
@@ -551,7 +551,7 @@ H5_DLL herr_t H5O__visit(H5G_loc_t *loc, const char *obj_name, H5_index_t idx_ty
                          H5O_iterate2_t op, void *op_data, unsigned fields);
 H5_DLL herr_t H5O__inc_rc(H5O_t *oh);
 H5_DLL herr_t H5O__dec_rc(H5O_t *oh);
-H5_DLL herr_t H5O__free(H5O_t *oh);
+H5_DLL herr_t H5O__free(H5O_t *oh, hbool_t force);
 
 /* Object header message routines */
 H5_DLL herr_t   H5O__msg_alloc(H5F_t *f, H5O_t *oh, const H5O_msg_class_t *type, unsigned *mesg_flags,