[svn-r24391] Issue 8528 - H5F_get_objects overfilled the list for object IDs. I merged the fix the trunk. I put the safeguard in both H5F_get_objects and H5F_get_objects_cb to prevent overfill the list. I also added test cases in tfile.c.

Tested with h5committest on jam, koala, ostrich, and platypus.

2 files changed, 203 insertions, 66 deletions

diff --git a/src/H5F.c b/src/H5F.c
index 8f71a84..bf79153 100644
--- a/src/H5F.c
+++ b/src/H5F.c
@@ -77,7 +77,7 @@ typedef struct H5F_olist_t {
         } ptr;
     } file_info;
     size_t     list_index;      /* Current index in open ID array */
-    size_t     max_index;            /* Maximum # of IDs to put into array */
+    size_t     max_nobjs;       /* Maximum # of IDs to put into array */
 } H5F_olist_t;
 
 
@@ -520,7 +520,8 @@ H5Fget_obj_ids(hid_t file_id, unsigned types, size_t max_objs, hid_t *oid_list)
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "not a file id")
     if(0 == (types & H5F_OBJ_ALL))
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "not an object type")
-    HDassert(oid_list);
+    if(!oid_list)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "object ID list is NULL")
 
     /* Perform the query */
     if(H5F_get_obj_ids(f, types, max_objs, oid_list, TRUE, &obj_id_count) < 0)
@@ -579,7 +580,7 @@ done:
  *---------------------------------------------------------------------------
  */
 static herr_t
-H5F_get_objects(const H5F_t *f, unsigned types, size_t max_index, hid_t *obj_id_list, hbool_t app_ref, size_t *obj_id_count_ptr)
+H5F_get_objects(const H5F_t *f, unsigned types, size_t max_nobjs, hid_t *obj_id_list, hbool_t app_ref, size_t *obj_id_count_ptr)
 {
     size_t obj_id_count=0;      /* Number of open IDs */
     H5F_olist_t olist;          /* Structure to hold search results */
@@ -591,10 +592,10 @@ H5F_get_objects(const H5F_t *f, unsigned types, size_t max_index, hid_t *obj_id_
     HDassert(obj_id_count_ptr);
 
     /* Set up search information */
-    olist.obj_id_list  = (max_index==0 ? NULL : obj_id_list);
+    olist.obj_id_list  = (max_nobjs==0 ? NULL : obj_id_list);
     olist.obj_id_count = &obj_id_count;
     olist.list_index   = 0;
-    olist.max_index   = max_index;
+    olist.max_nobjs   = max_nobjs;
 
     /* Determine if we are searching for local or global objects */
     if(types & H5F_OBJ_LOCAL) {
@@ -614,38 +615,54 @@ H5F_get_objects(const H5F_t *f, unsigned types, size_t max_index, hid_t *obj_id_
             HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(1)")
     } /* end if */
 
-    /* Search through dataset IDs to count number of datasets, and put their
+    /* If the caller just wants to count the number of objects (OLIST.MAX_NOBJS is zero),
+     * or the caller wants to get the list of IDs and the list isn't full,
+     * search through dataset IDs to count number of datasets, and put their
      * IDs on the object list */
-    if(types & H5F_OBJ_DATASET) {
-        olist.obj_type = H5I_DATASET;
-        if(H5I_iterate(H5I_DATASET, H5F_get_objects_cb, &olist, app_ref) < 0)
-            HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(2)")
-    } /* end if */
+    if(!olist.max_nobjs || (olist.max_nobjs && olist.list_index<olist.max_nobjs)) { 
+        if (types & H5F_OBJ_DATASET) {
+            olist.obj_type = H5I_DATASET;
+            if(H5I_iterate(H5I_DATASET, H5F_get_objects_cb, &olist, app_ref) < 0)
+                HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(2)")
+        } /* end if */
+    } 
 
-    /* Search through group IDs to count number of groups, and put their
+    /* If the caller just wants to count the number of objects (OLIST.MAX_NOBJS is zero),
+     * or the caller wants to get the list of IDs and the list isn't full,
+     * search through group IDs to count number of groups, and put their
      * IDs on the object list */
-    if(types & H5F_OBJ_GROUP) {
-        olist.obj_type = H5I_GROUP;
-        if(H5I_iterate(H5I_GROUP, H5F_get_objects_cb, &olist, app_ref) < 0)
-            HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(3)")
-    } /* end if */
+    if(!olist.max_nobjs || (olist.max_nobjs && olist.list_index<olist.max_nobjs)) { 
+        if(types & H5F_OBJ_GROUP) {
+            olist.obj_type = H5I_GROUP;
+            if(H5I_iterate(H5I_GROUP, H5F_get_objects_cb, &olist, app_ref) < 0)
+                HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(3)")
+        } /* end if */
+    } 
 
-    /* Search through datatype IDs to count number of named datatypes, and put their
+    /* If the caller just wants to count the number of objects (OLIST.MAX_NOBJS is zero),
+     * or the caller wants to get the list of IDs and the list isn't full,
+     * search through datatype IDs to count number of named datatypes, and put their
      * IDs on the object list */
-    if(types & H5F_OBJ_DATATYPE) {
-        olist.obj_type = H5I_DATATYPE;
-        if(H5I_iterate(H5I_DATATYPE, H5F_get_objects_cb, &olist, app_ref) < 0)
-            HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(4)")
-    } /* end if */
+    if(!olist.max_nobjs || (olist.max_nobjs && olist.list_index<olist.max_nobjs)) { 
+        if(types & H5F_OBJ_DATATYPE) {
+            olist.obj_type = H5I_DATATYPE;
+            if(H5I_iterate(H5I_DATATYPE, H5F_get_objects_cb, &olist, app_ref) < 0)
+                HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(4)")
+        } /* end if */
+    } 
 
-    /* Search through attribute IDs to count number of attributes, and put their
+    /* If the caller just wants to count the number of objects (OLIST.MAX_NOBJS is zero),
+     * or the caller wants to get the list of IDs and the list isn't full,
+     * search through attribute IDs to count number of attributes, and put their
      * IDs on the object list */
-    if(types & H5F_OBJ_ATTR) {
-        olist.obj_type = H5I_ATTR;
-        if(H5I_iterate(H5I_ATTR, H5F_get_objects_cb, &olist, app_ref) < 0)
-            HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(5)")
-    } /* end if */
-
+    if(!olist.max_nobjs || (olist.max_nobjs && olist.list_index<olist.max_nobjs)) {
+        if(types & H5F_OBJ_ATTR) {
+            olist.obj_type = H5I_ATTR;
+            if(H5I_iterate(H5I_ATTR, H5F_get_objects_cb, &olist, app_ref) < 0)
+                HGOTO_ERROR(H5E_FILE, H5E_BADITER, FAIL, "iteration failed(5)")
+        } /* end if */
+    }
+ 
     /* Set the number of objects currently open */
     *obj_id_count_ptr = obj_id_count;
 
@@ -661,8 +678,8 @@ done:
  * 		object is in the file, and either count it or put its ID
  *		on the list.
  *
- * Return:      TRUE if the array of object IDs is filled up.
- *              FALSE otherwise.
+ * Return:      H5_ITER_STOP if the array of object IDs is filled up.
+ *              H5_ITER_CONT otherwise.
  *
  * Programmer:  Raymond Lu
  *              Wednesday, Dec 5, 2001
@@ -673,7 +690,8 @@ static int
 H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
 {
     H5F_olist_t *olist = (H5F_olist_t *)key;    /* Alias for search info */
-    int      ret_value = FALSE;    /* Return value */
+    int         ret_value = H5_ITER_CONT;    /* Return value */
+    hbool_t     add_obj = FALSE;
 
     FUNC_ENTER_NOAPI_NOINIT
 
@@ -686,22 +704,7 @@ H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
                         (!olist->file_info.ptr.file || (olist->file_info.ptr.file && (H5F_t*)obj_ptr == olist->file_info.ptr.file) ))
                 ||  (!olist->file_info.local &&
                         ( !olist->file_info.ptr.shared || (olist->file_info.ptr.shared && ((H5F_t*)obj_ptr)->shared == olist->file_info.ptr.shared) ))) {
-            /* Add the object's ID to the ID list, if appropriate */
-            if(olist->obj_id_list) {
-                olist->obj_id_list[olist->list_index] = obj_id;
-		olist->list_index++;
-	    }
-
-            /* 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 */
+            add_obj = TRUE;
 	}
     } /* end if */
     else { /* either count opened object IDs or put the IDs on the list */
@@ -740,7 +743,7 @@ H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
 	    case H5I_ERROR_STACK:
 	    case H5I_NTYPES:
             default:
-                HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "unknown data object")
+                HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, H5_ITER_ERROR, "unknown data object")
 	} /* end switch */
 
         if((olist->file_info.local &&
@@ -751,25 +754,29 @@ H5F_get_objects_cb(void *obj_ptr, hid_t obj_id, void *key)
                     ((!olist->file_info.ptr.shared && olist->obj_type == H5I_DATATYPE && H5T_is_immutable((H5T_t *)obj_ptr) == FALSE)
                             || (!olist->file_info.ptr.shared && olist->obj_type != H5I_DATATYPE)
                             || (oloc && oloc->file && oloc->file->shared == olist->file_info.ptr.shared)))) {
-            /* Add the object's ID to the ID list, if appropriate */
-            if(olist->obj_id_list) {
-            	olist->obj_id_list[olist->list_index] = obj_id;
-		olist->list_index++;
-	    } /* end if */
-
-            /* 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 */
+            add_obj = TRUE;
     	} /* end if */
     } /* end else */
 
+    if(TRUE==add_obj) {
+        /* Add the object's ID to the ID list, if appropriate */
+        if(olist->obj_id_list) {
+            olist->obj_id_list[olist->list_index] = obj_id;
+	    olist->list_index++;
+	} /* end if */
+
+        /* Increment the number of open objects */
+	if(olist->obj_id_count)
+            (*olist->obj_id_count)++;
+
+        /* Check if we've filled up the array.  Return H5_ITER_STOP only if
+         * we have filled up the array. Otherwise return H5_ITER_CONT(RET_VALUE is
+         * preset to H5_ITER_CONT) because H5I_iterate needs the return value of 
+         * H5_ITER_CONT to continue the iteration. */
+        if(olist->max_nobjs>0 && olist->list_index>=olist->max_nobjs)
+            HGOTO_DONE(H5_ITER_STOP)  /* Indicate that the iterator should stop */
+    }
+
 done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5F_get_objects_cb() */
diff --git a/test/tfile.c b/test/tfile.c
index 7a52e2c..012de71 100644
--- a/test/tfile.c
+++ b/test/tfile.c
@@ -102,6 +102,10 @@
 /* Declaration for test_libver_macros2() */
 #define FILE5		"tfile5.h5"	/* Test file */
 
+/* Declaration for test_get_obj_ids() */
+#define FILE6			"tfile6.h5"	/* Test file */
+#define NGROUPS			2
+#define NDSETS			4
 
 static void
 create_objects(hid_t, hid_t, hid_t *, hid_t *, hid_t *, hid_t *);
@@ -924,6 +928,131 @@ 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(FILE6, 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);
+
+    /* Reopen the file to check whether H5Fget_obj_count and H5Fget_obj_ids still works 
+     * when the file is closed first */ 
+    fid = H5Fopen(FILE6, H5F_ACC_RDONLY, H5P_DEFAULT);
+    CHECK(fid, FAIL, "H5Fopen");
+
+    /* Open NDSETS datasets under the root group */
+    for(n = 0; n < NDSETS; n++) {
+         sprintf(dname, "dataset%d", n);
+         dset[n] = H5Dopen2(fid, dname, H5P_DEFAULT);
+         CHECK(dset[n], FAIL, "H5Dcreate2");
+    }
+
+    /* Close the file first */
+    H5Fclose(fid);
+
+    /* Get the number of all opened objects */
+    oid_count = H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL);
+    CHECK(oid_count, FAIL, "H5Fget_obj_count");
+    VERIFY(oid_count, NDSETS, "H5Fget_obj_count");
+
+    oid_list = (hid_t *)HDcalloc((size_t)oid_count, sizeof(hid_t));
+    CHECK(oid_list, NULL, "HDcalloc");
+
+    /* Get the list of all opened objects */
+    ret_count = H5Fget_obj_ids(H5F_OBJ_ALL, H5F_OBJ_ALL, (size_t)oid_count, oid_list);
+    CHECK(ret_count, FAIL, "H5Fget_obj_ids");
+    VERIFY(ret_count, NDSETS, "H5Fget_obj_count");
+
+    /* Close all open objects with H5Oclose */
+    for(n = 0; n < oid_count; n++)
+         H5Oclose(oid_list[n]);
+
+    HDfree(oid_list);
+}
+
+/****************************************************************
+**
 **  test_get_file_id(): Test H5Iget_file_id()
 **
 *****************************************************************/
@@ -2832,6 +2961,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 */