Allow H5Soffset_simple to accept NULL offsets

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 error when overwriting certain nested variable length types
 
       Previously, when using a datatype that included a variable length type

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*/

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
  *

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)

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() */

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;