Allow H5Soffset_simple to accept NULL offsets (#4152)

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

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
 

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;