Merge pull request #2714 in HDFFV/hdf5 from ~DEROBINS/hdf5_der:h5s_set_extent_none to develop

* commit 'e4603ff8a171e82de6373ba6eddcf59d5eeab5aa':
  Updated the Java tests to deal with H5Sset_extent_none changes
  Fixed additional typos in tselect.c comments.
  Fixes typo in tselect.c
  Updates H5Sset_extent_none() to set H5S_NULL

4 files changed, 99 insertions, 5 deletions

diff --git a/java/test/TestH5S.java b/java/test/TestH5S.java
index 985342b..97c0b68 100644
--- a/java/test/TestH5S.java
+++ b/java/test/TestH5S.java
@@ -171,7 +171,7 @@ public class TestH5S {
         try {
             H5.H5Sset_extent_none(H5sid);
             read_type = H5.H5Sget_simple_extent_type(H5sid);
-            assertTrue("H5.H5Sget_simple_extent_type: "+read_type, HDF5Constants.H5S_NO_CLASS == read_type);
+            assertTrue("H5.H5Sget_simple_extent_type: "+read_type, HDF5Constants.H5S_NULL == read_type);
         }
         catch (Throwable err) {
             err.printStackTrace();
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 42ce5e4..9972288 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -951,6 +951,21 @@ Bug Fixes since HDF5-1.10.3 release
 
       (DER - 2019/12/09, HDFFV-10945)
 
+    - H5Sset_extent_none() sets the dataspace class to H5S_NO_CLASS which
+      causes asserts/errors when passed to other dataspace API calls.
+
+      H5S_NO_CLASS is an internal class value that should not have been
+      exposed via a public API call.
+
+      In debug builds of the library, this can cause asserts to trip. In
+      non-debug builds, it will produce normal library errors.
+
+      The new library behavior is for H5Sset_extent_none() to convert
+      the dataspace into one of type H5S_NULL, which is better handled
+      by the library and easier for developers to reason about.
+
+      (DER - 2020/07/27, HDFFV-11027)
+
 
     Java Library:
     ----------------
diff --git a/src/H5S.c b/src/H5S.c
index bea86ca..7e48076 100644
--- a/src/H5S.c
+++ b/src/H5S.c
@@ -437,6 +437,9 @@ H5S__extent_release(H5S_extent_t *extent)
             extent->max = H5FL_ARR_FREE(hsize_t, extent->max);
     } /* end if */
 
+    extent->rank = 0;
+    extent->nelem = 0;
+
     FUNC_LEAVE_NOAPI(SUCCEED)
 } /* end H5S__extent_release() */
 
@@ -1832,7 +1835,7 @@ done:
  RETURNS
     Non-negative on success/Negative on failure
  DESCRIPTION
-    This function resets the type of a dataspace back to "none" with no
+    This function resets the type of a dataspace to H5S_NULL with no
     extent information stored for the dataspace.
 --------------------------------------------------------------------------*/
 herr_t
@@ -1852,7 +1855,7 @@ H5Sset_extent_none(hid_t space_id)
     if(H5S__extent_release(&space->extent) < 0)
         HGOTO_ERROR(H5E_RESOURCE, H5E_CANTDELETE, FAIL, "can't release previous dataspace")
 
-    space->extent.type = H5S_NO_CLASS;
+    space->extent.type = H5S_NULL;
 
 done:
     FUNC_LEAVE_API(ret_value)
diff --git a/test/tselect.c b/test/tselect.c
index aa0ab11..c98db5d 100644
--- a/test/tselect.c
+++ b/test/tselect.c
@@ -15442,14 +15442,14 @@ test_hyper_io_1d(void)
 
     /* Get the dataset's dataspace */
     sid = H5Dget_space(did);
-    CHECK(sid, H5I_INVALID_HID, "H5Pcreate");
+    CHECK(sid, H5I_INVALID_HID, "H5Dget_space");
     ret = H5Sselect_hyperslab(sid, H5S_SELECT_SET, offset, stride, count, block);
     CHECK(ret, FAIL, "H5Sselect_hyperslab");
 
     /* Set up contiguous memory dataspace for the selected elements */
     dimsm[0] = count[0];
     mid = H5Screate_simple(RANK, dimsm, NULL);
-    CHECK(mid, H5I_INVALID_HID, "H5Screate");
+    CHECK(mid, H5I_INVALID_HID, "H5Screate_simple");
 
     /* Read all the selected 10th elements in the dataset into "rdata" */
     ret = H5Dread(did, H5T_NATIVE_INT, mid, sid, H5P_DEFAULT, rdata);
@@ -15476,6 +15476,77 @@ test_hyper_io_1d(void)
 
 /****************************************************************
 **
+**  test_h5s_set_extent_none:
+**  Test to verify the behavior of dataspace code when passed
+**  a dataspace modified by H5Sset_extent_none().
+**
+****************************************************************/
+static void
+test_h5s_set_extent_none(void)
+{
+    hid_t sid = H5I_INVALID_HID;
+    hid_t dst_sid = H5I_INVALID_HID;
+    hid_t null_sid = H5I_INVALID_HID;
+    int rank = 1;
+    hsize_t current_dims = 123;
+    H5S_class_t cls;
+    int out_rank;
+    hsize_t out_dims;
+    hsize_t out_maxdims;
+    hssize_t out_points;
+    htri_t equal;
+    herr_t ret;
+
+    /* Specific values here don't matter as we're just going to reset */
+    sid = H5Screate_simple(rank, &current_dims, NULL);
+    CHECK(sid, H5I_INVALID_HID, "H5Screate_simple");
+
+    /* Dataspace class will be H5S_NULL after this.
+     * In versions prior to 1.10.7 / 1.12.1 this would produce a
+     * dataspace with the internal H5S_NO_CLASS class.
+     */
+    ret = H5Sset_extent_none(sid);
+    CHECK(ret, FAIL, "H5Sset_extent_none");
+    cls = H5Sget_simple_extent_type(sid);
+    VERIFY(cls, H5S_NULL, "H5Sget_simple_extent_type");
+
+    /* Extent getters should generate normal results and not segfault.
+     */
+    out_rank = H5Sget_simple_extent_dims(sid, &out_dims, &out_maxdims);
+    VERIFY(out_rank, 0, "H5Sget_simple_extent_dims");
+    out_rank = H5Sget_simple_extent_ndims(sid);
+    VERIFY(out_rank, 0, "H5Sget_simple_extent_ndims");
+    out_points = H5Sget_simple_extent_npoints(sid);
+    VERIFY(out_points, 0, "H5Sget_simple_extent_npoints");
+
+    /* Check that copying the new (non-)extent works.
+     */
+    dst_sid = H5Screate_simple(rank, &current_dims, NULL);
+    CHECK(dst_sid, H5I_INVALID_HID, "H5Screate_simple");
+    ret = H5Sextent_copy(dst_sid, sid);
+    CHECK(ret, FAIL, "H5Sextent_copy");
+
+    /* Check that H5Sset_extent_none() produces the same extent as
+     * H5Screate(H5S_NULL).
+     */
+    null_sid = H5Screate(H5S_NULL);
+    CHECK(null_sid, H5I_INVALID_HID, "H5Screate");
+    equal = H5Sextent_equal(sid, null_sid);
+    VERIFY(equal, TRUE, "H5Sextent_equal");
+
+    /* Close */
+    ret = H5Sclose(sid);
+    CHECK(ret, FAIL, "H5Sclose");
+    ret = H5Sclose(dst_sid);
+    CHECK(ret, FAIL, "H5Sclose");
+    ret = H5Sclose(null_sid);
+    CHECK(ret, FAIL, "H5Sclose");
+
+} /* test_h5s_set_extent_none() */
+
+
+/****************************************************************
+**
 **  test_select(): Main H5S selection testing routine.
 **
 ****************************************************************/
@@ -15660,6 +15731,11 @@ test_select(void)
     /* Test reading of 1-d disjoint file space to 1-d single block memory space */
     test_hyper_io_1d();
 
+    /* Test H5Sset_extent_none() functionality after we updated it to set
+     * the class to H5S_NULL instead of H5S_NO_CLASS.
+     */
+    test_h5s_set_extent_none();
+
 }   /* test_select() */