From 42c419e2661f67a80cef445084d28ac25572244e Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 18 Feb 2022 09:28:21 -0600 Subject: 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> --- src/H5Ocache.c | 8 +++--- src/H5Ocopy.c | 2 +- src/H5Ocopy_ref.c | 82 +++++++++++++++++++++++++++++-------------------------- src/H5Oint.c | 12 ++++---- src/H5Opkg.h | 2 +- 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, -- cgit v0.12