[svn-r24210] Jira issue 8528: H5Fget_obj_ids overfilled the list of object IDs by one. I moved the safeguard in H5F_get_objects_cb

to the beginning of the function to prevent overfill the list.  I added a new test case for this problem and fix in tfile.c.

Tested with h5committest. 
  

2 files changed, 104 insertions, 14 deletions

diff --git a/src/H5F.c b/src/H5F.c
index 676daef..04e627b 100644
--- a/src/H5F.c
+++ b/src/H5F.c
@@ -667,6 +667,13 @@ H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
     HDassert(obj_ptr);
     HDassert(olist);
 
+    /* Check if we've filled up the array.  Return TRUE only if
+     * we have filled up the array. Otherwise return FALSE(RET_VALUE is
+     * preset to FALSE) because H5I_iterate needs the return value of 
+     * FALSE to continue the iteration. */
+    if(olist->max_index>0 && olist->list_index>=olist->max_index)
+        HGOTO_DONE(TRUE)  /* Indicate that the iterator should stop */
+
     /* Count file IDs */
     if(olist->obj_type == H5I_FILE) {
         if((olist->file_info.local &&
@@ -682,13 +689,6 @@ H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
             /* Increment the number of open objects */
 	    if(olist->obj_id_count)
 	    	(*olist->obj_id_count)++;
-
-            /* Check if we've filled up the array.  Return TRUE only if
-             * we have filled up the array. Otherwise return FALSE(RET_VALUE is
-             * preset to FALSE) because H5I_iterate needs the return value of 
- 	     * FALSE to continue the iteration. */
-            if(olist->max_index>0 && olist->list_index>=olist->max_index)
-                HGOTO_DONE(TRUE)  /* Indicate that the iterator should stop */
 	}
     } /* end if */
     else { /* either count opened object IDs or put the IDs on the list */
@@ -747,13 +747,6 @@ H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
             /* Increment the number of open objects */
 	    if(olist->obj_id_count)
             	(*olist->obj_id_count)++;
-
-            /* Check if we've filled up the array.  Return TRUE only if
-             * we have filled up the array. Otherwise return FALSE(RET_VALUE is
-             * preset to FALSE) because H5I_iterate needs the return value of 
-	     * FALSE to continue iterating. */
-            if(olist->max_index>0 && olist->list_index>=olist->max_index)
-                HGOTO_DONE(TRUE)  /* Indicate that the iterator should stop */
     	} /* end if */
     } /* end else */
 
diff --git a/test/tfile.c b/test/tfile.c
index ac2ef82..556ccd3 100644
--- a/test/tfile.c
+++ b/test/tfile.c
@@ -112,6 +112,11 @@
 /* Declaration for test_libver_macros2() */
 #define FILE6			"tfile6.h5"	/* Test file */
 
+/* Declaration for test_get_obj_ids() */
+#define FILE7			"tfile7.h5"	/* Test file */
+#define NGROUPS			2
+#define NDSETS			4
+
 const char *OLD_FILENAME[] = {  /* Files created under 1.6 branch and 1.8 branch */
     "filespace_1_6.h5",	/* 1.6 HDF5 file */
     "filespace_1_8.h5"	/* 1.8 HDF5 file */
@@ -952,6 +957,97 @@ create_objects(hid_t fid1, hid_t fid2, hid_t *ret_did, hid_t *ret_gid1,
 
 /****************************************************************
 **
+**  test_get_obj_ids(): Test the bug and the fix for Jira 8528.
+**                      H5Fget_obj_ids overfilled the list of 
+**                      object IDs by one.  This is an enhancement
+**                      for test_obj_count_and_id().
+**
+****************************************************************/
+static void
+test_get_obj_ids(void)
+{
+    hid_t    fid, gid[NGROUPS], dset[NDSETS];
+    hid_t    filespace;
+    hsize_t  file_dims[F2_RANK] = {F2_DIM0, F2_DIM1};
+    ssize_t  oid_count, ret_count;
+    hid_t *oid_list = NULL;
+    herr_t   ret;
+    int i, m, n;
+    ssize_t oid_list_size = NDSETS;
+    char gname[64], dname[64];
+
+    /* Create a new file */
+    fid = H5Fcreate(FILE7, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(fid, FAIL, "H5Fcreate");
+
+    filespace = H5Screate_simple(F2_RANK, file_dims,  NULL);
+    CHECK(filespace, FAIL, "H5Screate_simple");
+
+    /* creates NGROUPS groups under the root group */
+    for(m = 0; m < NGROUPS; m++) {
+        sprintf(gname, "group%d", m);
+        gid[m] = H5Gcreate2(fid, gname, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+        CHECK(gid[m], FAIL, "H5Gcreate2");
+    }
+
+    /* create NDSETS datasets under the root group */
+    for(n = 0; n < NDSETS; n++) {
+         sprintf(dname, "dataset%d", n);
+         dset[n] = H5Dcreate2(fid, dname, H5T_NATIVE_INT, filespace, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+         CHECK(dset[m], FAIL, "H5Dcreate2");
+    }
+
+    /* The number of opened objects should be NGROUPS + NDSETS + 1.  One is opened file. */
+    oid_count = H5Fget_obj_count(fid, H5F_OBJ_ALL);
+    CHECK(oid_count, FAIL, "H5Fget_obj_count");
+    VERIFY(oid_count, (NGROUPS + NDSETS + 1), "H5Fget_obj_count");
+
+    oid_list = (hid_t *)HDcalloc((size_t)oid_list_size, sizeof(hid_t));
+    CHECK(oid_list, NULL, "HDcalloc");
+
+    /* Call the public function H5F_get_obj_ids to use H5F_get_objects.  User reported having problem here. 
+     * that the returned size (ret_count) from H5Fget_obj_ids is one greater than the size passed in 
+     * (oid_list_size) */
+    ret_count = H5Fget_obj_ids(fid, H5F_OBJ_ALL, (size_t)oid_list_size, oid_list);
+    CHECK(ret_count, FAIL, "H5Fget_obj_ids");
+    VERIFY(ret_count, oid_list_size, "H5Fget_obj_count");
+
+    /* Close all object IDs on the list except the file ID. The first ID is supposed to be file ID according 
+     * to the library design */
+    for(i = 0; i< ret_count; i++) {
+        if(fid != oid_list[i]) {
+            ret = H5Oclose(oid_list[i]);
+            CHECK(ret, FAIL, "H5Oclose");
+        }
+    }
+
+    /* The number of opened objects should be NGROUPS + 1 + 1.  The first one is opened file. The second one
+     * is the dataset ID left open from the previous around of H5Fget_obj_ids */
+    oid_count = H5Fget_obj_count(fid, H5F_OBJ_ALL);
+    CHECK(oid_count, FAIL, "H5Fget_obj_count");
+    VERIFY(oid_count, NGROUPS + 2, "H5Fget_obj_count");
+
+    /* Get the IDs of the left opend objects */ 
+    ret_count = H5Fget_obj_ids(fid, H5F_OBJ_ALL, (size_t)oid_list_size, oid_list);
+    CHECK(ret_count, FAIL, "H5Fget_obj_ids");
+    VERIFY(ret_count, oid_list_size, "H5Fget_obj_count");
+
+    /* Close all object IDs on the list except the file ID. The first ID is still the file ID */
+    for(i = 0; i< ret_count; i++) {
+        if(fid != oid_list[i]) {
+            ret = H5Oclose(oid_list[i]);
+            CHECK(ret, FAIL, "H5Oclose");
+        }
+    }
+  
+    H5Sclose(filespace);
+    H5Fclose(fid);
+
+    HDfree(oid_list);
+}
+
+/****************************************************************
+**
 **  test_get_file_id(): Test H5Iget_file_id()
 **
 *****************************************************************/
@@ -3526,6 +3622,7 @@ test_file(void)
     test_file_close();          /* Test file close behavior */
 #endif /* H5_NO_SHARED_WRITING */
     test_get_file_id();         /* Test H5Iget_file_id */
+    test_get_obj_ids();         /* Test H5Fget_obj_ids for Jira Issue 8528 */
     test_file_perm();           /* Test file access permissions */
     test_file_perm2();          /* Test file access permission again */
     test_file_freespace();      /* Test file free space information */