diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index d82167e1516..dd568f3eb70 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -591,6 +591,17 @@ Bug Fixes since HDF5-1.14.0 release Library ------- + - Corrected H5Soffset_simple() when offset is NULL + + The reference manual states that the offset parameter of H5Soffset_simple() + can be set to NULL to reset the offset of a simple dataspace to 0. This + has never been true, and passing NULL was regarded as an error. + + The library will now accept NULL for the offset parameter and will + correctly set the offset to zero. + + Fixes HDFFV-9299 + - Fixed an issue where the Subfiling VFD's context object cache could grow too large diff --git a/src/H5Spkg.h b/src/H5Spkg.h index 3ae63a50964..fddd5e3c5fa 100644 --- a/src/H5Spkg.h +++ b/src/H5Spkg.h @@ -413,6 +413,7 @@ H5_DLL herr_t H5S__get_rebuild_status_test(hid_t space_id, H5S_diminfo_valid_t * H5S_diminfo_valid_t *status2); H5_DLL herr_t H5S__get_diminfo_status_test(hid_t space_id, H5S_diminfo_valid_t *status); H5_DLL htri_t H5S__internal_consistency_test(hid_t space_id); +H5_DLL herr_t H5S__verify_offsets(hid_t space_id, const hssize_t *offset); #endif /* H5S_TESTING */ #endif /*H5Spkg_H*/ diff --git a/src/H5Spublic.h b/src/H5Spublic.h index 5422d96bcbf..bf9e9118f64 100644 --- a/src/H5Spublic.h +++ b/src/H5Spublic.h @@ -832,12 +832,16 @@ H5_DLL herr_t H5Smodify_select(hid_t space1_id, H5S_seloper_t op, hid_t space2_i * \p space_id. The offset array must be the same number of * elements as the number of dimensions for the dataspace. If the * \p offset array is set to NULL, the offset for the dataspace is - * reset to 0. + * reset to 0 in all dimensions. * * This function allows the same shaped selection to be moved to * different locations within a dataspace without requiring it to * be redefined. * + * \note Until 1.14.4, setting the offset parameter to NULL was considered + * an error, despite the reference manual stating that it had the + * behavior described above. + * * \version 1.4.0 Fortran subroutine was introduced. * \since 1.0.0 * diff --git a/src/H5Sselect.c b/src/H5Sselect.c index 477e0f840c1..7a032b26e85 100644 --- a/src/H5Sselect.c +++ b/src/H5Sselect.c @@ -81,11 +81,6 @@ H5FL_SEQ_EXTERN(hsize_t); Non-negative on success/Negative on failure DESCRIPTION Sets the selection offset for the dataspace - GLOBAL VARIABLES - COMMENTS, BUGS, ASSUMPTIONS - Only works for simple dataspaces currently - EXAMPLES - REVISION LOG --------------------------------------------------------------------------*/ herr_t H5S_select_offset(H5S_t *space, const hssize_t *offset) @@ -95,10 +90,14 @@ H5S_select_offset(H5S_t *space, const hssize_t *offset) /* Check args */ assert(space); assert(0 < space->extent.rank && space->extent.rank <= H5S_MAX_RANK); - assert(offset); - /* Copy the offset over */ - H5MM_memcpy(space->select.offset, offset, sizeof(hssize_t) * space->extent.rank); + /* Copy the offset over. As a special case, when offset is NULL, we + * reset all dimensions to zero. + */ + if (offset) + H5MM_memcpy(space->select.offset, offset, sizeof(hssize_t) * space->extent.rank); + else + memset(space->select.offset, 0, sizeof(hssize_t) * space->extent.rank); /* Indicate that the offset was changed */ space->select.offset_changed = true; @@ -133,12 +132,12 @@ H5Soffset_simple(hid_t space_id, const hssize_t *offset) /* Check args */ if (NULL == (space = (H5S_t *)H5I_object_verify(space_id, H5I_DATASPACE))) - HGOTO_ERROR(H5E_ID, H5E_BADID, FAIL, "not a dataspace"); + HGOTO_ERROR(H5E_DATASPACE, H5E_BADID, FAIL, "not a dataspace"); if (space->extent.rank == 0 || (H5S_GET_EXTENT_TYPE(space) == H5S_SCALAR || H5S_GET_EXTENT_TYPE(space) == H5S_NULL)) - HGOTO_ERROR(H5E_ID, H5E_UNSUPPORTED, FAIL, "can't set offset on scalar or null dataspace"); - if (offset == NULL) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no offset specified"); + HGOTO_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, FAIL, "can't set offset on scalar or null dataspace"); + + /* offset can be NULL (resets all dims to zero) */ /* Set the selection offset */ if (H5S_select_offset(space, offset) < 0) diff --git a/src/H5Stest.c b/src/H5Stest.c index 4978de0558e..422f7c1d20f 100644 --- a/src/H5Stest.c +++ b/src/H5Stest.c @@ -341,11 +341,6 @@ H5S__check_internal_consistency(const H5S_t *space) DESCRIPTION Check the states of internal data structures of the hyperslab, and see whether they are consistent or not - GLOBAL VARIABLES - COMMENTS, BUGS, ASSUMPTIONS - DO NOT USE THIS FUNCTION FOR ANYTHING EXCEPT TESTING - EXAMPLES - REVISION LOG --------------------------------------------------------------------------*/ htri_t H5S__internal_consistency_test(hid_t space_id) @@ -367,3 +362,40 @@ H5S__internal_consistency_test(hid_t space_id) done: FUNC_LEAVE_NOAPI(ret_value) } /* H5S__internal_consistency_test() */ + +/*-------------------------------------------------------------------------- + NAME + H5S__verify_offsets + PURPOSE + Verify that internal offsets match an array of offsets + USAGE + herr_t H5S__verify_offsets(hid_t space_id) + hid_t space_id; IN: dataspace id + const hssize_t *offset; IN: Offset to position the selection at + RETURNS + Non-negative true/false on success, negative on failure + DESCRIPTION + This function is necessary because there is no public API call + that lets you get the offsets +--------------------------------------------------------------------------*/ +herr_t +H5S__verify_offsets(hid_t space_id, const hssize_t *offset) +{ + H5S_t *space; /* Dataspace to modify */ + herr_t ret_value = SUCCEED; /* Return value */ + + FUNC_ENTER_PACKAGE + + if (NULL == (space = (H5S_t *)H5I_object_verify(space_id, H5I_DATASPACE))) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADID, FAIL, "not a dataspace"); + if (space->extent.rank == 0 || + (H5S_GET_EXTENT_TYPE(space) == H5S_SCALAR || H5S_GET_EXTENT_TYPE(space) == H5S_NULL)) + HGOTO_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, FAIL, "can't set offset on scalar or null dataspace"); + + /* Check that the internal and passed-in offset data are the same */ + if (0 != memcmp(space->select.offset, offset, sizeof(hssize_t) * space->extent.rank)) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADVALUE, FAIL, "internal offsets don't match parameters"); + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5S__verify_offsets() */ diff --git a/test/tselect.c b/test/tselect.c index 55430f24ad5..20b85916739 100644 --- a/test/tselect.c +++ b/test/tselect.c @@ -4118,6 +4118,8 @@ test_select_hyper_offset(void) CHECK(ret, FAIL, "H5Soffset_simple"); valid = H5Sselect_valid(sid1); VERIFY(valid, true, "H5Sselect_valid"); + ret = H5S__verify_offsets(sid1, offset); + CHECK(ret, FAIL, "H5S__verify_offsets"); /* Check an invalid offset */ offset[0] = 10; @@ -4127,6 +4129,8 @@ test_select_hyper_offset(void) CHECK(ret, FAIL, "H5Soffset_simple"); valid = H5Sselect_valid(sid1); VERIFY(valid, false, "H5Sselect_valid"); + ret = H5S__verify_offsets(sid1, offset); + CHECK(ret, FAIL, "H5S__verify_offsets"); /* Reset offset */ offset[0] = 0; @@ -4136,6 +4140,28 @@ test_select_hyper_offset(void) CHECK(ret, FAIL, "H5Soffset_simple"); valid = H5Sselect_valid(sid1); VERIFY(valid, true, "H5Sselect_valid"); + ret = H5S__verify_offsets(sid1, offset); + CHECK(ret, FAIL, "H5S__verify_offsets"); + + /* Check behavior of NULL offset parameter */ + + /* Set a valid offset */ + offset[0] = -1; + offset[1] = 0; + offset[2] = 0; + ret = H5Soffset_simple(sid1, offset); + CHECK(ret, FAIL, "H5Soffset_simple"); + valid = H5Sselect_valid(sid1); + VERIFY(valid, true, "H5Sselect_valid"); + /* Reset using NULL */ + ret = H5Soffset_simple(sid1, NULL); + CHECK(ret, FAIL, "H5Soffset_simple"); + valid = H5Sselect_valid(sid1); + VERIFY(valid, true, "H5Sselect_valid"); + /* Validate offset */ + offset[0] = 0; + ret = H5S__verify_offsets(sid1, offset); + CHECK(ret, FAIL, "H5S__verify_offsets"); /* Select 15x26 hyperslab for memory dataset */ start[0] = 15;