From 1659792d85acb1805aad3c9f3d2b8b02ab6d1249 Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Fri, 5 Jan 2024 11:24:33 -0600 Subject: H5Fget_obj_ids/count no longer returns transient types --- fortran/test/tH5F.F90 | 51 +++++----- release_docs/RELEASE.txt | 6 ++ src/H5F.c | 56 ++++++++--- test/tfile.c | 253 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 321 insertions(+), 45 deletions(-) diff --git a/fortran/test/tH5F.F90 b/fortran/test/tH5F.F90 index 569d459..2e38131 100644 --- a/fortran/test/tH5F.F90 +++ b/fortran/test/tH5F.F90 @@ -164,9 +164,9 @@ CONTAINS INTEGER(HID_T) :: file1_id, file2_id ! - ! Group identifier + ! Group identifiers ! - INTEGER(HID_T) :: gid + INTEGER(HID_T) :: gid, g1, g2, g3, g4 ! ! dataset identifier @@ -206,7 +206,6 @@ CONTAINS !number of objects INTEGER(SIZE_T) :: obj_count - INTEGER(HID_T) :: t1, t2, t3, t4 ! File numbers INTEGER :: file_num1 @@ -242,28 +241,28 @@ CONTAINS CALL h5_fixname_f(filename2, fix_filename2, H5P_DEFAULT_F, error) if(error .ne. 0) stop - ! Test object counts - CALL h5tcopy_f(H5T_NATIVE_CHARACTER, t1, error) - CALL check(" h5tcopy_f",error,total_error) - CALL h5tcopy_f(H5T_NATIVE_CHARACTER, t2, error) - CALL check(" h5tcopy_f",error,total_error) - CALL h5tcopy_f(H5T_NATIVE_CHARACTER, t3, error) - CALL check(" h5tcopy_f",error,total_error) - CALL h5tcopy_f(H5T_NATIVE_CHARACTER, t4, error) - CALL check(" h5tcopy_f",error,total_error) + ! + !Create first file "mount1.h5" using default properties. + ! + CALL h5fcreate_f(fix_filename1, H5F_ACC_TRUNC_F, file1_id, error) + CALL check("h5fcreate_f",error,total_error) + ! Test object counts CALL h5fget_obj_count_f(INT(H5F_OBJ_ALL_F,HID_T), H5F_OBJ_ALL_F, obj_count, error) CALL check(" h5fget_obj_count_f",error,total_error) - IF(obj_count.NE.4)THEN + IF(obj_count.NE.1)THEN total_error = total_error + 1 ENDIF - ! - !Create first file "mount1.h5" using default properties. - ! - CALL h5fcreate_f(fix_filename1, H5F_ACC_TRUNC_F, file1_id, error) - CALL check("h5fcreate_f",error,total_error) + CALL h5gcreate_f(file1_id, "/G1", g1, error) + CALL check(" h5gcopy_f",error, total_error) + CALL h5gcreate_f(file1_id, "/G2", g2, error) + CALL check(" h5gcopy_f",error, total_error) + CALL h5gcreate_f(file1_id, "/G3", g3, error) + CALL check(" h5gcopy_f",error, total_error) + CALL h5gcreate_f(file1_id, "/G4", g4, error) + CALL check(" h5gcopy_f",error, total_error) CALL h5fget_obj_count_f(INT(H5F_OBJ_ALL_F,HID_T), H5F_OBJ_ALL_F, obj_count, error) CALL check(" h5fget_obj_count_f",error,total_error) @@ -272,14 +271,14 @@ CONTAINS total_error = total_error + 1 ENDIF - CALL h5tclose_f(t1, error) - CALL check("h5tclose_f",error,total_error) - CALL h5tclose_f(t2, error) - CALL check("h5tclose_f",error,total_error) - CALL h5tclose_f(t3, error) - CALL check("h5tclose_f",error,total_error) - CALL h5tclose_f(t4, error) - CALL check("h5tclose_f",error,total_error) + CALL h5gclose_f(g1, error) + CALL check("h5gclose_f",error,total_error) + CALL h5gclose_f(g2, error) + CALL check("h5gclose_f",error,total_error) + CALL h5gclose_f(g3, error) + CALL check("h5gclose_f",error,total_error) + CALL h5gclose_f(g4, error) + CALL check("h5gclose_f",error,total_error) CALL h5fget_obj_count_f(INT(H5F_OBJ_ALL_F,HID_T), H5F_OBJ_ALL_F, obj_count, error) CALL check(" h5fget_obj_count_f",error,total_error) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index e80f5ba..98939fb 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -320,6 +320,12 @@ New Features Library: -------- + - Fixed an issue where copies of transient datatypes were + considered open objects on a file by H5Fget_obj_count + and H5Fget_obj_ids + + Addresses GitHub issue #3316 + - Implemented optimized support for vector I/O in the Subfiling VFD Previously, the Subfiling VFD would handle vector I/O requests by diff --git a/src/H5F.c b/src/H5F.c index ee4fd71..a472168 100644 --- a/src/H5F.c +++ b/src/H5F.c @@ -196,15 +196,31 @@ done: *------------------------------------------------------------------------- */ static int -H5F__get_all_count_cb(void H5_ATTR_UNUSED *obj_ptr, hid_t H5_ATTR_UNUSED obj_id, void *key) +H5F__get_all_count_cb(void H5_ATTR_UNUSED *obj_ptr, hid_t obj_id, void *key) { - H5F_trav_obj_cnt_t *udata = (H5F_trav_obj_cnt_t *)key; - int ret_value = H5_ITER_CONT; /* Return value */ + H5F_trav_obj_cnt_t *udata = (H5F_trav_obj_cnt_t *)key; + H5I_type_t obj_type = H5I_UNINIT; + htri_t is_committed = FAIL; + int ret_value = H5_ITER_CONT; /* Return value */ - FUNC_ENTER_PACKAGE_NOERR + FUNC_ENTER_PACKAGE + + if ((obj_type = H5Iget_type(obj_id)) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5_ITER_ERROR, "object has invalid id type"); + + if ((obj_type != H5I_DATATYPE)) { + udata->obj_count++; + } + else { + /* Only open committed datatypes should be counted as open datatype objects on the file */ + if ((is_committed = H5Tcommitted(obj_id)) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5_ITER_ERROR, "bad datatype"); - udata->obj_count++; + if (is_committed) + udata->obj_count++; + } +done: FUNC_LEAVE_NOAPI(ret_value) } /* H5F_get_all_count_cb */ @@ -305,17 +321,35 @@ done: static int H5F__get_all_ids_cb(void H5_ATTR_UNUSED *obj_ptr, hid_t obj_id, void *key) { - H5F_trav_obj_ids_t *udata = (H5F_trav_obj_ids_t *)key; - int ret_value = H5_ITER_CONT; /* Return value */ + H5F_trav_obj_ids_t *udata = (H5F_trav_obj_ids_t *)key; + H5I_type_t obj_type = H5I_UNINIT; + htri_t is_committed = FAIL; + int ret_value = H5_ITER_CONT; /* Return value */ - FUNC_ENTER_PACKAGE_NOERR + FUNC_ENTER_PACKAGE if (udata->obj_count >= udata->max_objs) HGOTO_DONE(H5_ITER_STOP); - /* Add the ID to the array */ - udata->oid_list[udata->obj_count] = obj_id; - udata->obj_count++; + if ((obj_type = H5Iget_type(obj_id)) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5_ITER_ERROR, "object has invalid id type"); + + if ((obj_type != H5I_DATATYPE)) { + /* Add the ID to the array */ + udata->oid_list[udata->obj_count] = obj_id; + udata->obj_count++; + } + else { + /* Only open committed datatypes should be counted as open datatype objects on the file */ + if ((is_committed = H5Tcommitted(obj_id)) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5_ITER_ERROR, "bad datatype"); + + if (is_committed) { + /* Add the ID to the array */ + udata->oid_list[udata->obj_count] = obj_id; + udata->obj_count++; + } + } done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/test/tfile.c b/test/tfile.c index 0f5bbd3..14d5433 100644 --- a/test/tfile.c +++ b/test/tfile.c @@ -184,6 +184,19 @@ static const char *FILESPACE_NAME[] = {"tfilespace", NULL}; #define DST_FILE "fill18_copy.h5" #define DSET_DS1 "DS1" +/* Declarations for test_get_datatype_count() */ +#define DATATYPE_OBJ_COUNT_FILENAME_1 "datatype_obj_count_file1" +#define DATATYPE_OBJ_COUNT_FILENAME_2 "datatype_obj_count_file2" +#define DATATYPE_OBJ_COUNT_DATASET_NAME_1 "datatype_obj_count_dset1" +#define DATATYPE_OBJ_COUNT_DATASET_NAME_2 "datatype_obj_count_dset2" +#define DATATYPE_OBJ_COUNT_DATASET_NAME_3 "datatype_obj_count_dset3" +#define DATATYPE_OBJ_COUNT_DATASET_NAME_4 "datatype_obj_count_dset4" +#define DATATYPE_OBJ_COUNT_TRANSIENT_DATATYPE H5T_NATIVE_INT +#define DATATYPE_OBJ_COUNT_DATATYPE_NAME_1 "datatype_obj_count_type1" +#define DATATYPE_OBJ_COUNT_DATATYPE_NAME_2 "datatype_obj_count_type2" +#define DATATYPE_OBJ_COUNT_DSPACE_EXTENT 100 +#define DATATYPE_OBJ_ID_LIST_MAX 16 + /* Local test function declarations for version bounds */ static void test_libver_bounds_low_high(const char *env_h5_drvr); static void test_libver_bounds_super(hid_t fapl, const char *env_h5_drvr); @@ -1432,6 +1445,229 @@ test_get_obj_ids(void) /**************************************************************** ** +** test_get_datatype_count(): Test that H5Fget_obj_ids and +** H5Fget_obj_count do not count transient datatypes as open objects. +** +****************************************************************/ +static void +test_get_datatype_count(void) +{ + hid_t file_id1 = H5I_INVALID_HID; + hid_t file_id2 = H5I_INVALID_HID; + + hid_t dset_id1 = H5I_INVALID_HID; + hid_t dset_id2 = H5I_INVALID_HID; + hid_t dset_id3 = H5I_INVALID_HID; + hid_t dset_id4 = H5I_INVALID_HID; + + hid_t type_id1 = H5I_INVALID_HID; + hid_t type_id2 = H5I_INVALID_HID; + hid_t type_id3 = H5I_INVALID_HID; + + hid_t dspace_id = H5I_INVALID_HID; + hid_t obj_id_list[DATATYPE_OBJ_ID_LIST_MAX]; + hsize_t dims[1] = {DATATYPE_OBJ_COUNT_DSPACE_EXTENT}; + + ssize_t obj_count = 0; + htri_t is_committed = FAIL; + H5I_type_t obj_type = H5I_UNINIT; + herr_t ret = FAIL; + + /* Check that no other files are open at the start of the test */ + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL); + VERIFY(obj_count, 0, "H5Fget_obj_count"); + + file_id1 = H5Fcreate(DATATYPE_OBJ_COUNT_FILENAME_1, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file_id1, H5I_INVALID_HID, "H5Fcreate"); + + /* Create dataset with transient datatype */ + dspace_id = H5Screate_simple(1, dims, NULL); + CHECK(dspace_id, H5I_INVALID_HID, "H5Screate_simple"); + + dset_id1 = H5Dcreate2(file_id1, DATATYPE_OBJ_COUNT_DATASET_NAME_1, DATATYPE_OBJ_COUNT_TRANSIENT_DATATYPE, + dspace_id, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(dset_id1, H5I_INVALID_HID, "H5Dcreate2"); + + /* H5Fget_obj_count should not count transient datatype as open object */ + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 0, "H5Fget_obj_count"); + + type_id1 = H5Tcopy(DATATYPE_OBJ_COUNT_TRANSIENT_DATATYPE); + CHECK(type_id1, H5I_INVALID_HID, "H5Tcopy"); + + ret = H5Tcommit2(file_id1, DATATYPE_OBJ_COUNT_DATATYPE_NAME_1, type_id1, H5P_DEFAULT, H5P_DEFAULT, + H5P_DEFAULT); + CHECK(ret, FAIL, "H5Tcommit2"); + + /* H5Fget_obj_count should count the committed datatype */ + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 1, "H5Fget_obj_count"); + + /* Open file, committed datatype, and dataset */ + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_ALL); + VERIFY(obj_count, 3, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_FILE | H5F_OBJ_DATATYPE); + VERIFY(obj_count, 2, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL); + VERIFY(obj_count, 3, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_FILE | H5F_OBJ_DATATYPE); + VERIFY(obj_count, 2, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 1, "H5Fget_obj_count"); + + /* Create second file to test file_id = H5F_OBJ_ALL with multiple files */ + file_id2 = H5Fcreate(DATATYPE_OBJ_COUNT_FILENAME_2, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file_id2, H5I_INVALID_HID, "H5Fcreate"); + + dset_id2 = H5Dcreate2(file_id2, DATATYPE_OBJ_COUNT_DATASET_NAME_2, DATATYPE_OBJ_COUNT_TRANSIENT_DATATYPE, + dspace_id, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT); + CHECK(dset_id2, H5I_INVALID_HID, "H5Dcreate2"); + + type_id2 = H5Tcopy(DATATYPE_OBJ_COUNT_TRANSIENT_DATATYPE); + CHECK(type_id2, H5I_INVALID_HID, "H5Tcopy"); + + ret = H5Tcommit2(file_id2, DATATYPE_OBJ_COUNT_DATATYPE_NAME_2, type_id2, H5P_DEFAULT, H5P_DEFAULT, + H5P_DEFAULT); + CHECK(ret, FAIL, "H5Tcommit2"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL); + VERIFY(obj_count, 6, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_FILE | H5F_OBJ_DATATYPE); + VERIFY(obj_count, 4, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 2, "H5Fget_obj_count"); + + /* Create a dataset which shares a pre-existing committed datatype */ + dset_id3 = H5Dcreate2(file_id1, DATATYPE_OBJ_COUNT_DATASET_NAME_3, type_id1, dspace_id, H5P_DEFAULT, + H5P_DEFAULT, H5P_DEFAULT); + CHECK(dset_id3, H5I_INVALID_HID, "H5Dcreate2"); + + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 1, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 2, "H5Fget_obj_count"); + + /* Copy transient type */ + type_id3 = H5Tcopy(DATATYPE_OBJ_COUNT_TRANSIENT_DATATYPE); + CHECK(type_id3, H5I_INVALID_HID, "H5Tcopy"); + + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 1, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 2, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_ids(H5F_OBJ_ALL, H5F_OBJ_DATATYPE, DATATYPE_OBJ_ID_LIST_MAX, obj_id_list); + VERIFY(obj_count, 2, "H5Fget_obj_ids"); + + for (ssize_t i = 0; i < obj_count; i++) { + obj_type = H5Iget_type(obj_id_list[i]); + + CHECK(obj_type, H5I_BADID, "H5Iget_type"); + + if (obj_type == H5I_DATATYPE) { + is_committed = H5Tcommitted(obj_id_list[i]); + + CHECK(is_committed, FAIL, "H5Tcommitted"); + + /* Should only return ids of open committed datatypes */ + VERIFY(is_committed, true, "H5Tcommitted"); + } + } + + /* Create dataset with copy of transient datatype */ + dset_id4 = H5Dcreate2(file_id1, DATATYPE_OBJ_COUNT_DATASET_NAME_4, type_id3, dspace_id, H5P_DEFAULT, + H5P_DEFAULT, H5P_DEFAULT); + CHECK(dset_id4, H5I_INVALID_HID, "H5Dcreate2"); + + obj_count = H5Fget_obj_count(file_id1, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 1, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_DATATYPE); + VERIFY(obj_count, 2, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_ids(H5F_OBJ_ALL, H5F_OBJ_DATATYPE, DATATYPE_OBJ_ID_LIST_MAX, obj_id_list); + VERIFY(obj_count, 2, "H5Fget_obj_ids"); + + for (ssize_t i = 0; i < obj_count; i++) { + obj_type = H5Iget_type(obj_id_list[i]); + + CHECK(obj_type, H5I_BADID, "H5Iget_type"); + + if (obj_type == H5I_DATATYPE) { + is_committed = H5Tcommitted(obj_id_list[i]); + + CHECK(is_committed, FAIL, "H5Tcommitted"); + + /* Should only return ids of open committed datatypes */ + VERIFY(is_committed, true, "H5Tcommitted"); + } + } + + obj_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL); + VERIFY(obj_count, 8, "H5Fget_obj_count"); + + obj_count = H5Fget_obj_ids(H5F_OBJ_ALL, H5F_OBJ_ALL, DATATYPE_OBJ_ID_LIST_MAX, obj_id_list); + VERIFY(obj_count, 8, "H5Fget_obj_ids"); + + for (ssize_t i = 0; i < obj_count; i++) { + obj_type = H5Iget_type(obj_id_list[i]); + + CHECK(obj_type, H5I_BADID, "H5Iget_type"); + + if (obj_type == H5I_DATATYPE) { + is_committed = H5Tcommitted(obj_id_list[i]); + + CHECK(is_committed, FAIL, "H5Tcommitted"); + + /* Should only return ids of open committed datatypes */ + VERIFY(is_committed, true, "H5Tcommitted"); + } + } + + ret = H5Tclose(type_id1); + CHECK(ret, FAIL, "H5Tclose"); + + ret = H5Tclose(type_id2); + CHECK(ret, FAIL, "H5Tclose"); + + ret = H5Tclose(type_id3); + CHECK(ret, FAIL, "H5Tclose"); + + ret = H5Dclose(dset_id1); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Dclose(dset_id2); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Dclose(dset_id3); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Dclose(dset_id4); + CHECK(ret, FAIL, "H5Dclose"); + + ret = H5Fclose(file_id1); + CHECK(ret, FAIL, "H5Fclose"); + + ret = H5Fclose(file_id2); + CHECK(ret, FAIL, "H5Fclose"); + + ret = H5Fdelete(DATATYPE_OBJ_COUNT_FILENAME_1, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Fdelete"); + + ret = H5Fdelete(DATATYPE_OBJ_COUNT_FILENAME_2, H5P_DEFAULT); + CHECK(ret, FAIL, "H5Fdelete"); +} + +/**************************************************************** +** ** test_get_file_id(): Test H5Iget_file_id() ** *****************************************************************/ @@ -8361,14 +8597,15 @@ test_file(void) ret = h5_driver_is_default_vfd_compatible(fapl_id, &driver_is_default_compatible); CHECK(ret, FAIL, "h5_driver_is_default_vfd_compatible"); - test_file_create(); /* Test file creation(also creation templates)*/ - test_file_open(env_h5_drvr); /* Test file opening */ - test_file_reopen(); /* Test file reopening */ - test_file_close(); /* Test file close behavior */ - test_get_file_id(); /* Test H5Iget_file_id */ - test_get_obj_ids(); /* Test H5Fget_obj_ids for Jira Issue 8528 */ - test_file_perm(); /* Test file access permissions */ - test_file_perm2(); /* Test file access permission again */ + test_file_create(); /* Test file creation(also creation templates)*/ + test_file_open(env_h5_drvr); /* Test file opening */ + test_file_reopen(); /* Test file reopening */ + test_file_close(); /* Test file close behavior */ + test_get_file_id(); /* Test H5Iget_file_id */ + test_get_obj_ids(); /* Test H5Fget_obj_ids for Jira Issue 8528 */ + test_get_datatype_count(); /* Test for H5Fget_obj_count for committed vs. transient datatypes */ + test_file_perm(); /* Test file access permissions */ + test_file_perm2(); /* Test file access permission again */ test_file_is_accessible(env_h5_drvr); /* Test detecting HDF5 files correctly */ test_file_delete(fapl_id); /* Test H5Fdelete */ test_file_open_dot(); /* Test opening objects with "." for a name */ -- cgit v0.12