From 8e0e86812567cd6dbe413e3794a63d109c6263de Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 22 Jan 2021 12:15:53 -0600 Subject: [PATCH 1/4] Fixed problems with vlens and refs inside compound using H5VLget_file_type() --- release_docs/RELEASE.txt | 9 +++++++ src/H5Odtype.c | 4 ++- src/H5T.c | 57 +++++++++++++++++++++++++++++++++++++--- src/H5Tcommit.c | 7 ++++- src/H5Tconv.c | 2 +- src/H5Tdeprec.c | 1 + src/H5Tpkg.h | 4 +-- src/H5Tprivate.h | 3 +++ src/H5Tref.c | 13 ++++++++- src/H5Tvlen.c | 13 ++++++++- src/H5VL.c | 15 ++++++----- src/H5VLint.c | 12 ++++++--- src/H5VLprivate.h | 1 + 13 files changed, 120 insertions(+), 21 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 3bd3e011c1d..5f48b61946b 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -669,6 +669,15 @@ Bug Fixes since HDF5-1.12.0 release ================================== Library ------- + - Fixed problems with vlens and refs inside compound using + H5VLget_file_type() + + Modified library to properly ref count H5VL_object_t structs and only + consider file vlen and reference types to be equal if their files are + the same. + + (NAF - 2021/01/19) + - Creation of dataset with optional filter When the combination of type, space, etc doesn't work for filter diff --git a/src/H5Odtype.c b/src/H5Odtype.c index b6e986dbfc7..c1effad749f 100644 --- a/src/H5Odtype.c +++ b/src/H5Odtype.c @@ -579,8 +579,10 @@ H5O__dtype_decode_helper(unsigned *ioflags /*in,out*/, const uint8_t **pp, H5T_t done: if (ret_value < 0) if (dt != NULL) { - if (dt->shared != NULL) + if (dt->shared != NULL) { + HDassert(!dt->shared->owned_vol_obj); dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); + } /* end if */ dt = H5FL_FREE(H5T_t, dt); } /* end if */ diff --git a/src/H5T.c b/src/H5T.c index 59f0224bb3c..8f31246791f 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -1479,6 +1479,8 @@ H5T__init_package(void) if (copied_dtype) (void)H5T_close_real(dt); else { + if (dt->shared->owned_vol_obj && H5VL_free_object(dt->shared->owned_vol_obj) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, FAIL, "unable to close owned VOL object") dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); dt = H5FL_FREE(H5T_t, dt); } /* end else */ @@ -3447,6 +3449,8 @@ H5T__create(H5T_class_t type, size_t size) done: if (NULL == ret_value) { if (dt) { + if (dt->shared->owned_vol_obj && H5VL_free_object(dt->shared->owned_vol_obj) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close owned VOL object") dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); dt = H5FL_FREE(H5T_t, dt); } @@ -3489,9 +3493,12 @@ H5T__initiate_copy(const H5T_t *old_dt) /* Copy shared information */ *(new_dt->shared) = *(old_dt->shared); - /* Reset VOL fields */ + /* Increment ref count on owned VOL object */ + if (new_dt->shared->owned_vol_obj) + new_dt->shared->owned_vol_obj->rc++; + + /* Reset vol_obj field */ new_dt->vol_obj = NULL; - new_dt->shared->owned_vol_obj = NULL; /* Set return value */ ret_value = new_dt; @@ -3499,8 +3506,11 @@ H5T__initiate_copy(const H5T_t *old_dt) done: if (ret_value == NULL) if (new_dt) { - if (new_dt->shared) + if (new_dt->shared) { + if (new_dt->shared->owned_vol_obj && H5VL_free_object(new_dt->shared->owned_vol_obj) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close owned VOL object") new_dt->shared = H5FL_FREE(H5T_shared_t, new_dt->shared); + } /* end if */ new_dt = H5FL_FREE(H5T_t, new_dt); } /* end if */ @@ -3829,6 +3839,8 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) if (ret_value == NULL) if (new_dt) { HDassert(new_dt->shared); + if (new_dt->shared->owned_vol_obj && H5VL_free_object(new_dt->shared->owned_vol_obj) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close owned VOL object") new_dt->shared = H5FL_FREE(H5T_shared_t, new_dt->shared); new_dt = H5FL_FREE(H5T_t, new_dt); } /* end if */ @@ -3896,6 +3908,8 @@ H5T_copy_reopen(H5T_t *old_dt) /* The object is already open. Free the H5T_shared_t struct * we had been using and use the one that already exists. * Not terribly efficient. */ + if (new_dt->shared->owned_vol_obj && H5VL_free_object(new_dt->shared->owned_vol_obj) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close owned VOL object") new_dt->shared = H5FL_FREE(H5T_shared_t, new_dt->shared); new_dt->shared = reopened_fo; @@ -3932,6 +3946,8 @@ H5T_copy_reopen(H5T_t *old_dt) if (ret_value == NULL) if (new_dt) { HDassert(new_dt->shared); + if (new_dt->shared->owned_vol_obj && H5VL_free_object(new_dt->shared->owned_vol_obj) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close owned VOL object") new_dt->shared = H5FL_FREE(H5T_shared_t, new_dt->shared); new_dt = H5FL_FREE(H5T_t, new_dt); } /* end if */ @@ -4026,8 +4042,10 @@ H5T__alloc(void) done: if (ret_value == NULL) if (dt) { - if (dt->shared) + if (dt->shared) { + HDassert(!dt->shared->owned_vol_obj); dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); + } /* end if */ dt = H5FL_FREE(H5T_t, dt); } /* end if */ @@ -4148,6 +4166,7 @@ H5T_close_real(H5T_t *dt) if (H5T__free(dt) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "unable to free datatype"); + HDassert(!dt->shared->owned_vol_obj); dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); } /* end if */ else @@ -4473,6 +4492,31 @@ H5T_get_size(const H5T_t *dt) FUNC_LEAVE_NOAPI(dt->shared->size) } /* end H5T_get_size() */ +/*------------------------------------------------------------------------- + * Function: H5T_get_force_conv + * + * Purpose: Determines if the type has forced conversion. This will be + * true if and only if the type keeps a pointer to a file VOL + * object internally. + * + * Return: TRUE/FALSE (never fails) + * + * Programmer: Neil Fortner + * Thursday, January 21, 2021 + *------------------------------------------------------------------------- + */ +hbool_t +H5T_get_force_conv(const H5T_t *dt) +{ + /* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */ + FUNC_ENTER_NOAPI_NOINIT_NOERR + + /* check args */ + HDassert(dt); + + FUNC_LEAVE_NOAPI(dt->shared->force_conv) +} /* end H5T_get_force_conv() */ + /*------------------------------------------------------------------------- * Function: H5T_cmp * @@ -4914,6 +4958,10 @@ H5T_cmp(const H5T_t *dt1, const H5T_t *dt2, hbool_t superset) HGOTO_DONE(-1); if (dt1->shared->u.atomic.u.r.loc > dt2->shared->u.atomic.u.r.loc) HGOTO_DONE(1); + if (dt1->shared->u.atomic.u.r.file < dt2->shared->u.atomic.u.r.file) + HGOTO_DONE(-1); + if (dt1->shared->u.atomic.u.r.file > dt2->shared->u.atomic.u.r.file) + HGOTO_DONE(1); break; case H5T_NO_CLASS: @@ -6295,6 +6343,7 @@ H5T_own_vol_obj(H5T_t *dt, H5VL_object_t *vol_obj) /* Take ownership */ dt->shared->owned_vol_obj = vol_obj; + vol_obj->rc++; done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Tcommit.c b/src/H5Tcommit.c index 41191fa60c6..57e690c9c81 100644 --- a/src/H5Tcommit.c +++ b/src/H5Tcommit.c @@ -148,6 +148,7 @@ H5T__commit_api_common(hid_t loc_id, const char *name, hid_t type_id, hid_t lcpl new_obj->connector = (*vol_obj_ptr)->connector; new_obj->connector->nrefs++; new_obj->data = data; + new_obj->rc = 1; /* Set the committed type object to the VOL connector pointer in the H5T_t struct */ dt->vol_obj = new_obj; @@ -375,6 +376,7 @@ H5Tcommit_anon(hid_t loc_id, hid_t type_id, hid_t tcpl_id, hid_t tapl_id) new_obj->connector = vol_obj->connector; new_obj->connector->nrefs++; new_obj->data = dt; + new_obj->rc = 1; /* Set the committed type object to the VOL connector pointer in the H5T_t struct */ type->vol_obj = new_obj; @@ -1114,8 +1116,11 @@ H5T_open(const H5G_loc_t *loc) done: if (ret_value == NULL) { if (dt) { - if (shared_fo == NULL) /* Need to free shared fo */ + if (shared_fo == NULL) { /* Need to free shared fo */ + if (dt->shared->owned_vol_obj && H5VL_free_object(dt->shared->owned_vol_obj) < 0) + HDONE_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, NULL, "unable to close owned VOL object") dt->shared = H5FL_FREE(H5T_shared_t, dt->shared); + } /* end if */ H5O_loc_free(&(dt->oloc)); H5G_name_free(&(dt->path)); diff --git a/src/H5Tconv.c b/src/H5Tconv.c index d8e47592335..92b8ae137b3 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -2010,7 +2010,7 @@ H5T__conv_struct_init(H5T_t *src, H5T_t *dst, H5T_cdata_t *cdata) priv->subset_info.copy_size = 0; /* - * Insure that members are sorted. + * Ensure that members are sorted. */ H5T__sort_value(src, NULL); H5T__sort_value(dst, NULL); diff --git a/src/H5Tdeprec.c b/src/H5Tdeprec.c index a6a6631db0d..4dee09c347b 100644 --- a/src/H5Tdeprec.c +++ b/src/H5Tdeprec.c @@ -139,6 +139,7 @@ H5Tcommit1(hid_t loc_id, const char *name, hid_t type_id) new_obj->connector = vol_obj->connector; new_obj->connector->nrefs++; new_obj->data = data; + new_obj->rc = 1; /* Set the committed type object to the VOL connector pointer in the H5T_t struct */ dt->vol_obj = new_obj; diff --git a/src/H5Tpkg.h b/src/H5Tpkg.h index e96921fae14..e9991d82780 100644 --- a/src/H5Tpkg.h +++ b/src/H5Tpkg.h @@ -860,7 +860,7 @@ H5_DLL void H5T__bit_neg(uint8_t *buf, size_t start, size_t size); /* VL functions */ H5_DLL H5T_t *H5T__vlen_create(const H5T_t *base); H5_DLL herr_t H5T__vlen_reclaim(void *elem, const H5T_t *dt, H5T_vlen_alloc_info_t *alloc_info); -H5_DLL htri_t H5T__vlen_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc); +H5_DLL htri_t H5T__vlen_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc); /* Array functions */ H5_DLL H5T_t *H5T__array_create(H5T_t *base, unsigned ndims, const hsize_t dim[/* ndims */]); @@ -869,7 +869,7 @@ H5_DLL int H5T__get_array_dims(const H5T_t *dt, hsize_t dims[]); /* Reference functions */ H5_DLL herr_t H5T__ref_reclaim(void *elem, const H5T_t *dt); -H5_DLL htri_t H5T__ref_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc); +H5_DLL htri_t H5T__ref_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc); /* Compound functions */ H5_DLL herr_t H5T__insert(H5T_t *parent, const char *name, size_t offset, const H5T_t *member); diff --git a/src/H5Tprivate.h b/src/H5Tprivate.h index 52f1c1441a0..862ee1fd2cf 100644 --- a/src/H5Tprivate.h +++ b/src/H5Tprivate.h @@ -41,11 +41,13 @@ typedef struct H5T_t H5T_t; #define H5T_GET_SHARED(T) ((T)->shared) #define H5T_GET_MEMBER_OFFSET(T, I) ((T)->u.compnd.memb[I].offset) #define H5T_GET_MEMBER_SIZE(T, I) ((T)->u.compnd.memb[I].shared->size) +#define H5T_GET_FORCE_CONV(T) ((T)>shared->force_conv) #else /* H5T_MODULE */ #define H5T_GET_SIZE(T) (H5T_get_size(T)) #define H5T_GET_SHARED(T) (H5T_get_shared(T)) #define H5T_GET_MEMBER_OFFSET(T, I) (H5T_get_member_offset((T), (I))) #define H5T_GET_MEMBER_SIZE(T, I) (H5T_get_member_size((T), (I))) +#define H5T_GET_FORCE_CONV(T) (H5T_get_force_conv(T)) #endif /* H5T_MODULE */ /* Forward references of package typedefs (declared in H5Tpkg.h) */ @@ -114,6 +116,7 @@ H5_DLL H5T_t * H5T_get_super(const H5T_t *dt); H5_DLL H5T_class_t H5T_get_class(const H5T_t *dt, htri_t internal); H5_DLL htri_t H5T_detect_class(const H5T_t *dt, H5T_class_t cls, hbool_t from_api); H5_DLL size_t H5T_get_size(const H5T_t *dt); +H5_DLL hbool_t H5T_get_force_conv(const H5T_t *dt); H5_DLL int H5T_cmp(const H5T_t *dt1, const H5T_t *dt2, hbool_t superset); H5_DLL herr_t H5T_encode(H5T_t *obj, unsigned char *buf, size_t *nalloc); H5_DLL H5T_t * H5T_decode(size_t buf_size, const unsigned char *buf); diff --git a/src/H5Tref.c b/src/H5Tref.c index 8d7dca2c394..9c6477b5b41 100644 --- a/src/H5Tref.c +++ b/src/H5Tref.c @@ -156,7 +156,7 @@ static const H5T_ref_class_t H5T_ref_dsetreg_disk_g = { *------------------------------------------------------------------------- */ htri_t -H5T__ref_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) +H5T__ref_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) { htri_t ret_value = FALSE; /* Indicate success, but no location change */ @@ -180,6 +180,13 @@ H5T__ref_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) /* Mark this type as being stored in memory */ dt->shared->u.atomic.u.r.loc = H5T_LOC_MEMORY; + /* Release owned file */ + if (dt->shared->owned_vol_obj) { + if(H5VL_free_object(dt->shared->owned_vol_obj) < 0) + HGOTO_ERROR(H5E_REFERENCE, H5E_CANTCLOSEOBJ, FAIL, "unable to close owned VOL object") + dt->shared->owned_vol_obj = NULL; + } /* end if */ + /* Reset file ID (since this reference is in memory) */ dt->shared->u.atomic.u.r.file = file; /* file is NULL */ @@ -220,6 +227,10 @@ H5T__ref_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) /* Set file pointer (since this reference is on disk) */ dt->shared->u.atomic.u.r.file = file; + /* dt now owns a reference to file */ + if (H5T_own_vol_obj(dt, file) < 0) + HGOTO_ERROR(H5E_REFERENCE, H5E_CANTINIT, FAIL, "can't give ownership of VOL object") + if (dt->shared->u.atomic.u.r.rtype == H5R_OBJECT1) { H5F_t *f; diff --git a/src/H5Tvlen.c b/src/H5Tvlen.c index e2d8d2aaac2..20dab84f36d 100644 --- a/src/H5Tvlen.c +++ b/src/H5Tvlen.c @@ -245,7 +245,7 @@ H5T__vlen_create(const H5T_t *base) *------------------------------------------------------------------------- */ htri_t -H5T__vlen_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) +H5T__vlen_set_loc(H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) { H5VL_file_cont_info_t cont_info = {H5VL_CONTAINER_INFO_VERSION, 0, 0, 0}; htri_t ret_value = FALSE; /* Indicate success, but no location change */ @@ -282,6 +282,13 @@ H5T__vlen_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) else HDassert(0 && "Invalid VL type"); + /* Release owned file */ + if (dt->shared->owned_vol_obj) { + if(H5VL_free_object(dt->shared->owned_vol_obj) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTCLOSEOBJ, FAIL, "unable to close owned VOL object") + dt->shared->owned_vol_obj = NULL; + } /* end if */ + /* Reset file pointer (since this VL is in memory) */ dt->shared->u.vlen.file = NULL; break; @@ -307,6 +314,10 @@ H5T__vlen_set_loc(const H5T_t *dt, H5VL_object_t *file, H5T_loc_t loc) /* Set file ID (since this VL is on disk) */ dt->shared->u.vlen.file = file; + + /* dt now owns a reference to file */ + if (H5T_own_vol_obj(dt, file) < 0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "can't give ownership of VOL object") break; case H5T_LOC_BADLOC: diff --git a/src/H5VL.c b/src/H5VL.c index c1f8adb6a85..628d6bba6de 100644 --- a/src/H5VL.c +++ b/src/H5VL.c @@ -683,8 +683,9 @@ H5VLget_file_type(void *file_obj, hid_t connector_id, hid_t dtype_id) if (NULL == (dtype = (H5T_t *)H5I_object_verify(dtype_id, H5I_DATATYPE))) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a data type") - /* Create VOL object for file */ - if (NULL == (file_vol_obj = H5VL_create_object_using_vol_id(H5I_FILE, file_obj, connector_id))) + /* Create VOL object for file if necessary (force_conv will be TRUE if and + * only if file needs to be passed to H5T_set_loc) */ + if (H5T_GET_FORCE_CONV(dtype) && (NULL == (file_vol_obj = H5VL_create_object_using_vol_id(H5I_FILE, file_obj, connector_id)))) HGOTO_ERROR(H5E_VOL, H5E_CANTCREATE, FAIL, "can't create VOL object") /* Copy the datatype */ @@ -701,10 +702,12 @@ H5VLget_file_type(void *file_obj, hid_t connector_id, hid_t dtype_id) if (H5T_set_loc(file_type, file_vol_obj, H5T_LOC_DISK) < 0) HGOTO_ERROR(H5E_VOL, H5E_CANTINIT, FAIL, "can't set datatype location") - /* file_type now owns file_vol_obj */ - if (H5T_own_vol_obj(file_type, file_vol_obj) < 0) - HGOTO_ERROR(H5E_VOL, H5E_CANTINIT, FAIL, "can't give ownership of VOL object") - file_vol_obj = NULL; + /* Release our reference to file_type */ + if (file_vol_obj) { + if (H5VL_free_object(file_vol_obj) < 0) + HGOTO_ERROR(H5E_VOL, H5E_CANTDEC, FAIL, "unable to free VOL object") + file_vol_obj = NULL; + } /* end if */ /* Set return value */ ret_value = file_type_id; diff --git a/src/H5VLint.c b/src/H5VLint.c index b7823fbf880..79eda97fcf9 100644 --- a/src/H5VLint.c +++ b/src/H5VLint.c @@ -568,6 +568,7 @@ H5VL__new_vol_obj(H5I_type_t type, void *object, H5VL_t *vol_connector, hbool_t } /* end if */ else new_vol_obj->data = object; + new_vol_obj->rc = 1; /* Bump the reference count on the VOL connector */ H5VL_conn_inc_rc(vol_connector); @@ -844,6 +845,7 @@ H5VL_create_object(void *object, H5VL_t *vol_connector) HGOTO_ERROR(H5E_VOL, H5E_CANTALLOC, NULL, "can't allocate memory for VOL object") ret_value->connector = vol_connector; ret_value->data = object; + ret_value->rc = 1; /* Bump the reference count on the VOL connector */ H5VL_conn_inc_rc(vol_connector); @@ -1000,11 +1002,13 @@ H5VL_free_object(H5VL_object_t *vol_obj) /* Check arguments */ HDassert(vol_obj); - /* Decrement refcount on connector */ - if (H5VL_conn_dec_rc(vol_obj->connector) < 0) - HGOTO_ERROR(H5E_VOL, H5E_CANTDEC, FAIL, "unable to decrement ref count on VOL connector") + if(--vol_obj->rc == 0) { + /* Decrement refcount on connector */ + if (H5VL_conn_dec_rc(vol_obj->connector) < 0) + HGOTO_ERROR(H5E_VOL, H5E_CANTDEC, FAIL, "unable to decrement ref count on VOL connector") - vol_obj = H5FL_FREE(H5VL_object_t, vol_obj); + vol_obj = H5FL_FREE(H5VL_object_t, vol_obj); + } /* end if */ done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5VLprivate.h b/src/H5VLprivate.h index a224a14c6b0..2eda0b959cb 100644 --- a/src/H5VLprivate.h +++ b/src/H5VLprivate.h @@ -37,6 +37,7 @@ typedef struct H5VL_t { typedef struct H5VL_object_t { void * data; /* Pointer to connector-managed data for this object */ H5VL_t *connector; /* Pointer to VOL connector struct */ + hsize_t rc; /* Reference count */ } H5VL_object_t; /* Internal structure to hold the connector ID & info for FAPLs */ From 84c88b1c57c86029d09ec03cb7bd4836d8a96c42 Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 22 Jan 2021 12:19:58 -0600 Subject: [PATCH 2/4] Fix date in RELEASE.txt --- release_docs/RELEASE.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 5f48b61946b..404c944f1a4 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -676,7 +676,7 @@ Bug Fixes since HDF5-1.12.0 release consider file vlen and reference types to be equal if their files are the same. - (NAF - 2021/01/19) + (NAF - 2021/01/22) - Creation of dataset with optional filter From 003ff76efae1e7bdcae6dc80bd3645f83ee3e15a Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 22 Jan 2021 13:04:01 -0600 Subject: [PATCH 3/4] Add assertions --- src/H5T.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/H5T.c b/src/H5T.c index 8f31246791f..0ec901be557 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -4301,6 +4301,7 @@ H5T__set_size(H5T_t *dt, size_t size) /* Check args */ HDassert(dt); + HDassert(dt->shared); HDassert(size != 0); HDassert(H5T_REFERENCE != dt->shared->type); HDassert(!(H5T_ENUM == dt->shared->type && 0 == dt->shared->u.enumer.nmembs)); @@ -4488,6 +4489,7 @@ H5T_get_size(const H5T_t *dt) /* check args */ HDassert(dt); + HDassert(dt->shared); FUNC_LEAVE_NOAPI(dt->shared->size) } /* end H5T_get_size() */ @@ -4513,6 +4515,7 @@ H5T_get_force_conv(const H5T_t *dt) /* check args */ HDassert(dt); + HDassert(dt->shared); FUNC_LEAVE_NOAPI(dt->shared->force_conv) } /* end H5T_get_force_conv() */ @@ -4553,6 +4556,9 @@ H5T_cmp(const H5T_t *dt1, const H5T_t *dt2, hbool_t superset) if (dt1 == dt2) HGOTO_DONE(0); + HDassert(dt1->shared); + HDassert(dt2->shared); + /* compare */ if (dt1->shared->type < dt2->shared->type) HGOTO_DONE(-1); From 389fb22d0f30f2a49a1ac178a386344898f27fb2 Mon Sep 17 00:00:00 2001 From: Neil Fortner Date: Fri, 22 Jan 2021 13:49:25 -0600 Subject: [PATCH 4/4] Move some manipulation of H5VL_object_t struct fields into the H5VL package. --- src/H5T.c | 4 ++-- src/H5Tcommit.c | 16 ++++------------ src/H5Tdeprec.c | 8 ++------ src/H5VLint.c | 21 +++++++++++++++++++++ src/H5VLprivate.h | 3 ++- 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/src/H5T.c b/src/H5T.c index 0ec901be557..33a75c9ec5c 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -3495,7 +3495,7 @@ H5T__initiate_copy(const H5T_t *old_dt) /* Increment ref count on owned VOL object */ if (new_dt->shared->owned_vol_obj) - new_dt->shared->owned_vol_obj->rc++; + (void)H5VL_object_inc_rc(new_dt->shared->owned_vol_obj); /* Reset vol_obj field */ new_dt->vol_obj = NULL; @@ -6349,7 +6349,7 @@ H5T_own_vol_obj(H5T_t *dt, H5VL_object_t *vol_obj) /* Take ownership */ dt->shared->owned_vol_obj = vol_obj; - vol_obj->rc++; + (void)H5VL_object_inc_rc(vol_obj); done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Tcommit.c b/src/H5Tcommit.c index 57e690c9c81..5ce6f3a1b08 100644 --- a/src/H5Tcommit.c +++ b/src/H5Tcommit.c @@ -143,12 +143,8 @@ H5T__commit_api_common(hid_t loc_id, const char *name, hid_t type_id, hid_t lcpl HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "unable to commit datatype") /* Set up VOL object */ - if (NULL == (new_obj = H5FL_CALLOC(H5VL_object_t))) - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "can't allocate top object structure") - new_obj->connector = (*vol_obj_ptr)->connector; - new_obj->connector->nrefs++; - new_obj->data = data; - new_obj->rc = 1; + if (NULL == (new_obj = H5VL_create_object(data, (*vol_obj_ptr)->connector))) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "can't create VOL object for committed datatype") /* Set the committed type object to the VOL connector pointer in the H5T_t struct */ dt->vol_obj = new_obj; @@ -371,12 +367,8 @@ H5Tcommit_anon(hid_t loc_id, hid_t type_id, hid_t tcpl_id, hid_t tapl_id) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "unable to commit datatype") /* Setup VOL object */ - if (NULL == (new_obj = H5FL_CALLOC(H5VL_object_t))) - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "can't allocate top object structure") - new_obj->connector = vol_obj->connector; - new_obj->connector->nrefs++; - new_obj->data = dt; - new_obj->rc = 1; + if (NULL == (new_obj = H5VL_create_object(dt, vol_obj->connector))) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "can't create VOL object for committed datatype") /* Set the committed type object to the VOL connector pointer in the H5T_t struct */ type->vol_obj = new_obj; diff --git a/src/H5Tdeprec.c b/src/H5Tdeprec.c index 4dee09c347b..fa141519a58 100644 --- a/src/H5Tdeprec.c +++ b/src/H5Tdeprec.c @@ -134,12 +134,8 @@ H5Tcommit1(hid_t loc_id, const char *name, hid_t type_id) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "unable to commit datatype") /* Set up VOL object */ - if (NULL == (new_obj = H5FL_CALLOC(H5VL_object_t))) - HGOTO_ERROR(H5E_DATATYPE, H5E_NOSPACE, FAIL, "can't allocate top object structure") - new_obj->connector = vol_obj->connector; - new_obj->connector->nrefs++; - new_obj->data = data; - new_obj->rc = 1; + if (NULL == (new_obj = H5VL_create_object(data, vol_obj->connector))) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, FAIL, "can't create VOL object for committed datatype") /* Set the committed type object to the VOL connector pointer in the H5T_t struct */ dt->vol_obj = new_obj; diff --git a/src/H5VLint.c b/src/H5VLint.c index 79eda97fcf9..033327db88c 100644 --- a/src/H5VLint.c +++ b/src/H5VLint.c @@ -982,6 +982,27 @@ H5VL_conn_dec_rc(H5VL_t *connector) FUNC_LEAVE_NOAPI(ret_value) } /* end H5VL_conn_dec_rc() */ +/*------------------------------------------------------------------------- + * Function: H5VL_object_inc_rc + * + * Purpose: Wrapper to increment the ref count on a VOL object. + * + * Return: SUCCEED/FAIL + * + *------------------------------------------------------------------------- + */ +hsize_t +H5VL_object_inc_rc(H5VL_object_t *vol_obj) +{ + FUNC_ENTER_NOAPI_NOERR_NOFS + + /* Check arguments */ + HDassert(vol_obj); + + /* Increment refcount for object and return */ + FUNC_LEAVE_NOAPI(++vol_obj->rc) +} /* end H5VL_object_inc_rc() */ + /*------------------------------------------------------------------------- * Function: H5VL_free_object * diff --git a/src/H5VLprivate.h b/src/H5VLprivate.h index 2eda0b959cb..880efd3bd44 100644 --- a/src/H5VLprivate.h +++ b/src/H5VLprivate.h @@ -37,7 +37,7 @@ typedef struct H5VL_t { typedef struct H5VL_object_t { void * data; /* Pointer to connector-managed data for this object */ H5VL_t *connector; /* Pointer to VOL connector struct */ - hsize_t rc; /* Reference count */ + size_t rc; /* Reference count */ } H5VL_object_t; /* Internal structure to hold the connector ID & info for FAPLs */ @@ -91,6 +91,7 @@ H5_DLL void *H5VL_object_verify(hid_t id, H5I_type_t obj_type); H5_DLL H5VL_object_t *H5VL_vol_object(hid_t id); H5_DLL H5VL_object_t *H5VL_create_object(void *object, H5VL_t *vol_connector); H5_DLL H5VL_object_t *H5VL_create_object_using_vol_id(H5I_type_t type, void *obj, hid_t connector_id); +H5_DLL hsize_t H5VL_object_inc_rc(H5VL_object_t *obj); H5_DLL herr_t H5VL_free_object(H5VL_object_t *obj); H5_DLL herr_t H5VL_object_is_native(const H5VL_object_t *obj, hbool_t *is_native); H5_DLL herr_t H5VL_file_is_same(const H5VL_object_t *vol_obj1, const H5VL_object_t *vol_obj2,