Propagate group creation properties to intermediate groups #4139
Conversation
Propagate object creation properties to intermediate groups.
vchoi-hdfgroup
requested review from
lrknox,
derobins,
byrnHDF,
fortnern,
jhendersonHDF,
qkoziol,
bmribler,
glennsong09,
mattjala and
brtnfld
as code owners
hyoklee
added
Component - C Library
derobins
changed the title
test/tmisc.c
Outdated
| hid_t sid = H5I_INVALID_HID; | ||
| hsize_t dims[1] = {10}; | ||
| unsigned cr_order = 0; | ||
| hbool_t track_times = false; |
src/H5Gtraverse.c
Outdated
| if (ocrt_info->obj_type == H5O_TYPE_GROUP) | ||
| gcrt_info.gcpl_id = ((H5G_obj_create_t *)ocrt_info->crt_info)->gcpl_id; | ||
| else if (ocrt_info->obj_type == H5O_TYPE_DATASET) | ||
| gcrt_info.gcpl_id = ((H5D_obj_create_t *)ocrt_info->crt_info)->dcpl_id; |
There was a problem hiding this comment.
This seems like a poor reason to include the H5Dpkg header. Please add a private 'getter' routine to the H5D interface instead.
There was a problem hiding this comment.
Actually, it looks like there should be 3 new private routines: a 'getter' in the H5L interface (so the H5L_trav_cr_t struct stays hidden from other packages), and a 'getter' for the H5G and H5D interfaces that retrieve the GCPL & DCPL IDs, respectively.
There was a problem hiding this comment.
I implemented the 3 getters for H5L, H5G, and H5D interfaces.
However, I got the following CI build errors:
/usr/bin/ld: ../src/.libs/libhdf5.so: undefined reference to `H5D_OBJ_ID'
collect2: error: ld returned 1 exit status
I couldn't figure out what went wrong for the macro H5D_OBJ_ID in the H5D interface. Anyone knows what might be the problem?
There was a problem hiding this comment.
Not certain about this, since H5Gtraverse included H5Dprivate.h. Maybe delete your build directory and re-run autogen.sh?
There was a problem hiding this comment.
The failures are from the CI actions. Not from my own build on jelly.
which was a macro defined in src/H5Dprivate.h. The header file "H5Dprivate.h" was removed from src/H5Gtraverse.c in the commit "Remove useless headers (HDFGroup#4145)".
| */ | ||
|
|
||
| /* Create groups that use default creation properties */ | ||
| def_gid = H5Gcreate2(fid, "def_group1/def_group2", lcpl, H5P_DEFAULT, H5P_DEFAULT); |
There was a problem hiding this comment.
Is it better to use constants for these names?
| if (ocrt_info->obj_type == H5O_TYPE_GROUP) | ||
| gcrt_info.gcpl_id = H5G_OBJ_ID(ocrt_info->crt_info); | ||
| else if (ocrt_info->obj_type == H5O_TYPE_DATASET) | ||
| gcrt_info.gcpl_id = H5D_OBJ_ID(ocrt_info->crt_info); |
There was a problem hiding this comment.
Seems like this may be problematic to assign a DCPL ID to a GCPL ID. They're both subclasses of H5P_CLS_OBJECT_CREATE_g, but if the code ever needs to retrieve some group-specific properties (maybe in the future), it seems like that may fail. Should gcrt_info have an ocpl_id field rather than a gcpl_id field to try to make it clear that only generic OCPL properties should be retrieved from it?
There was a problem hiding this comment.
Seems like this may be problematic to assign a DCPL ID to a GCPL ID. They're both subclasses of H5P_CLS_OBJECT_CREATE_g, but if the code ever needs to retrieve some group-specific properties (maybe in the future), it seems like that may fail. Should
gcrt_infohave anocpl_idfield rather than agcpl_idfield to try to make it clear that only generic OCPL properties should be retrieved from it?
Agree
There was a problem hiding this comment.
I have couple questions about changing gcpl_id to ocpl_id:
--Changing gcpl_id field in gcrt_info to ocpl_id would lead to changes in quite a few files which uses gcrt_info.gpcl_id.
--If I change gcpl_id in gcrt_info which is H5G_obj_create_t struct, should I also change dcpl_id field in H5D_obj_create_t struct to ocpl_id?
There was a problem hiding this comment.
--Changing gcpl_id field in gcrt_info to ocpl_id would lead to changes in quite a few files which uses gcrt_info.gpcl_id.
It's probably fine to put this off to a separate focused PR since the code appears to currently only retrieve generic properties.
--If I change gcpl_id in gcrt_info which is H5G_obj_create_t struct, should I also change dcpl_id field in H5D_obj_create_t struct to ocpl_id?
As far as I can tell, it looks like the only place that sets up a H5D_obj_create_t struct sets its dcpl_id field to a DCPL, so that one shouldn't need to be changed.
| CHECK(status, FAIL, "H5Gclose"); | ||
| VERIFY(track_times, true, "H5Pget_obj_track_times"); | ||
|
|
||
| /* Default is false */ |
test/tmisc.c
Outdated
| CHECK(status, FAIL, "H5Gclose"); | ||
|
|
||
| status = H5Pclose(def_gcpl); | ||
| CHECK(status, FAIL, "H5Gclose"); |
test/tmisc.c
Outdated
| * group with intermediate groups | ||
| */ | ||
| gcpl = H5Pcreate(H5P_GROUP_CREATE); | ||
| CHECK(gcpl, FAIL, "H5Fcreate"); |
test/tmisc.c
Outdated
| VERIFY(track_times, false, "H5Pget_obj_track_times"); | ||
|
|
||
| status = H5Gclose(gid); | ||
| CHECK(status, FAIL, "H5Pclose"); |
test/tmisc.c
Outdated
| * dataset with intermediate group | ||
| */ | ||
| dcpl = H5Pcreate(H5P_DATASET_CREATE); | ||
| CHECK(dcpl, FAIL, "H5Fcreate"); |
test/tmisc.c
Outdated
|
|
||
| /* Verify dname */ | ||
| did = H5Dopen2(gid, "dname", H5P_DEFAULT); | ||
| CHECK(did, FAIL, "H5Gopen2"); |
test/tmisc.c
Outdated
| VERIFY(track_times, true, "H5Pget_obj_track_times"); | ||
|
|
||
| status = H5Dclose(did); | ||
| CHECK(status, FAIL, "H5Pclose"); |
test/tmisc.c
Outdated
| CHECK(status, FAIL, "H5Pclose"); | ||
|
|
||
| status = H5Gclose(gid); | ||
| CHECK(status, FAIL, "H5Pclose"); |
* Call memset before stat calls (#4202) The buffers passed to stat-like calls are only partially filled in by the call, leaving ununitialized memory areas when the stat buffers are created on the stack. This change memsets the buffers to 0 before the stat calls, quieting the -fsanitze=memory complaints. * Remove unused CMake configuration checks (#4199) * Update link to Chunking in HDF5 page (#4203) * Fix H5Pset_efile_prefix documentation error (#4206) Fixes GH issue #1759 * Suggested header footer for NEWSLETTER (#4204) * Suggested header footer for NEWSLETTER * Updates * Add NEWSLETTER.txt to h5vers script * Fix grammar in README.md release template (#4201) * Add back snapshot names (#4198) * Use tar.gz extension for ABI reports (#4205) * Fix issue with Subfiling VFD and multiple opens of same file (#4194) * Fix issue with Subfiling VFD and multiple opens of same file * Update H5_subfile_fid_to_context to return error value instead of ID * Add helper routine to initialize open file mapping * Reverts AC_SYS_LARGEFILE change (#4213) We previously replaced local macros with AC_SYS_LARGEFILE, which is unfortunately buggy on some systems and does not correctly set the necessary defines, despite successfully detecting them. This restores the previous macro hacks to acsite.m4 * Propagate group creation properties to intermediate groups (#4139) * Add clarification for current behavior of H5Get_objtype_by_idx() (#4120) * Addressed Fortran issues with promoted integers and reals via compilation flags (#4209) * addressed issue wit promoted integers and reals * added option to use mpi_f08 * Summarize the library version table (#4212) Fixes GH-3773 * Fix URLs (#4210) Also removed Copyright.html context because it's no longer valid. * Fix 'make check-vfd' target for Autotools (#4211) Changes Autotools testing to use HDF5_TEST_DRIVER environment variable to avoid running tests that don't work well with several VFDs Restores old h5_get_vfd_fapl() testing function to setup a FAPL with a particular VFD Adds a macro for the default VFD name * Revert "Addressed Fortran issues with promoted integers and reals via compil…" (#4220) This reverts commit 06c42ff. * Backup and clear CMAKE_C_FLAGS before performing _Float16 configure checks (#4217) * Fix broken links (#4224) * Fix broken URLs in documentation (#4214) Fixes GH-3881 partially. There are pages that need to be recreated. * Avoid file size checks in C++ testhdf5 for certain VFDs (#4226) * Fix an issue with type size assumptions in h5dumpgentest (#4222) * Fix issue with -Werror cleanup sed command in configure.ac (#4223) * Fix Java JNI warnings (#4229) * Rework snapshots/release workflows for consistent args (#4227) * Fixed a cache assert with too-large metadata objects (#4231) If the library tries to load a metadata object that is above the library's hard-coded limits, the size will trip an assert in debug builds. In HDF5 1.14.4, this can happen if you create a very large number of links in an old-style group that uses local heaps. The library will now emit a normal error when it tries to load a metadata object that is too large. Partially addresses GitHub #3762 * Set DXPL in API context for native VOL attribute I/O calls (#4228) * Initialize a variable in C++ testhdf5's tattr.cpp (#4232) * Addressed Fortran issues with promoted integers and reals via compilation flags, part 2 (#4221) * addressed issue wit promoted integers and reals * fixed h5fcreate_f * added option to use mpi_f08 * change the kind of logical in the parallel tests * addressed missing return value from callback * Use cp -rp in test_plugin.sh (#4233) When building with debug symbols on MacOS, the cp -p commands in test_plugin.sh will attempt to copy the .dSYM directories with debugging info, which will fail since -r is missing. Using cp -rp is harmless and allows the test to run Fixes HDFFV-10542 * Clean up types in h5test.c (#4235) Reduces warnings on 32-bit and LLP64 systems * Fix example links (#4237) * Fix links md files (#4239) * Add markdown link checker action (#4219) * Match minimum CMake version to 3.18 (#4215)
Propagate object creation properties to intermediate groups.
Fixes #3945