Fixed H5Ovisit2() change of behavior between 1.10.11 and v1.14.4.3

release_docs/RELEASE.txt

@@ -255,6 +255,26 @@ Bug Fixes since HDF5-2.0.0 release
       would cause those routines to return FAIL instead of FALSE when checking
       the existence of a non-existent object with a file ID instead of a
       group ID.
+
+    - Fixed a segfault in h5dump when a B-tree node level is corrupted
+
+      h5dump produced a segfault on a mal-formed file because a B-tree node
+      level was corrupted.
+
+      An internal function was modified to help detecting when a decoded B-tree
+      node level has an unexpected value and an error will be produced.
+
+      Fixes GitHub issue #4432
+
+    - Fixed H5Ovisit2 to recursively visit all objects
+
+      H5Ovisit2 visited only the root group and not all the nested groups.
+
+      This behavior occurred when the fields are not H5O_INFO_BASIC or
+      H5O_INFO_ALL because an internal function did not obtain the basic
+      information needed by its caller.  This problem is now fixed.
+
+      Fixes GitHub issue #4941
 
     - Only clear FE_INVALID when that symbol is present on the system
 

src/H5Oint.c

@@ -2647,7 +2647,9 @@ H5O__visit(H5G_loc_t *loc, const char *obj_name, H5_index_t idx_type, H5_iter_or
     H5O_loc_t           obj_oloc;                    /* Opened object object location */
     bool                loc_found = false;           /* Entry at 'name' found */
     H5O_info2_t         oinfo;                       /* Object info struct */
-    void               *obj = NULL;                  /* Object */
+    H5O_info2_t         int_oinfo;                   /* Internal object info */
+    H5O_info2_t        *oinfop = &oinfo;             /* Object info pointer */
+    void               *obj    = NULL;               /* Object */
     H5I_type_t          opened_type;                 /* ID type of object */
     hid_t               obj_id    = H5I_INVALID_HID; /* ID of object */
     herr_t              ret_value = FAIL;            /* Return value */
@@ -2674,6 +2676,14 @@ H5O__visit(H5G_loc_t *loc, const char *obj_name, H5_index_t idx_type, H5_iter_or
     if (H5O_get_info(&obj_oloc, &oinfo, fields) < 0)
         HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to get object info");
 
+    /* If basic fields are not requested, get object basic info to use here */
+    if (!(fields & H5O_INFO_BASIC)) {
+        oinfop = &int_oinfo;
+        /* Get the object's basic info for local use */
+        if (H5O_get_info(&obj_oloc, &int_oinfo, H5O_INFO_BASIC) < 0)
+            HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to get object's basic info");
+    }
+
     /* Open the object */
     /* (Takes ownership of the obj_loc information) */
     if (NULL == (obj = H5O_open_by_loc(&obj_loc, &opened_type)))
@@ -2684,15 +2694,17 @@ H5O__visit(H5G_loc_t *loc, const char *obj_name, H5_index_t idx_type, H5_iter_or
         HGOTO_ERROR(H5E_ID, H5E_CANTREGISTER, FAIL, "unable to register visited object");
 
     /* Make callback for starting object */
-    if ((ret_value = op(obj_id, ".", &oinfo, op_data)) < 0)
+    /* if ((ret_value = op(obj_id, ".", oinfop, op_data)) < 0)
+     */
+    if ((ret_value = op(obj_id, ".", oinfop, op_data)) < 0)
         HGOTO_ERROR(H5E_OHDR, H5E_BADITER, FAIL, "can't visit objects");
 
     /* Check return value of first callback */
     if (ret_value != H5_ITER_CONT)
         HGOTO_DONE(ret_value);
 
     /* Check for object being a group */
-    if (oinfo.type == H5O_TYPE_GROUP) {
+    if (oinfop->type == H5O_TYPE_GROUP) {
         H5G_loc_t start_loc; /* Location of starting group */
         H5G_loc_t vis_loc;   /* Location of visited group */
 
@@ -2713,18 +2725,18 @@ H5O__visit(H5G_loc_t *loc, const char *obj_name, H5_index_t idx_type, H5_iter_or
 
         /* If its ref count is > 1, we add it to the list of visited objects */
         /* (because it could come up again during traversal) */
-        if (oinfo.rc > 1) {
+        if (oinfop->rc > 1) {
             H5_obj_t *obj_pos; /* New object node for visited list */
 
             /* Allocate new object "position" node */
             if ((obj_pos = H5FL_MALLOC(H5_obj_t)) == NULL)
                 HGOTO_ERROR(H5E_OHDR, H5E_NOSPACE, FAIL, "can't allocate object node");
 
             /* Construct unique "position" for this object */
-            obj_pos->fileno = oinfo.fileno;
+            obj_pos->fileno = oinfop->fileno;
 
             /* De-serialize object token into an object address */
-            if (H5VL_native_token_to_addr(loc->oloc->file, H5I_FILE, oinfo.token, &(obj_pos->addr)) < 0)
+            if (H5VL_native_token_to_addr(loc->oloc->file, H5I_FILE, oinfop->token, &(obj_pos->addr)) < 0)
                 HGOTO_ERROR(H5E_OHDR, H5E_CANTUNSERIALIZE, FAIL,
                             "can't deserialize object token into address");
 

test/th5o.c

@@ -23,7 +23,8 @@
 #include "H5VLprivate.h"
 #include "H5VLnative_private.h"
 
-#define TEST_FILENAME "th5o_file"
+#define TEST_FILENAME   "th5o_file"
+#define VISIT2_FILENAME "th5o_visit2"
 
 #define RANK 2
 #define DIM0 5
@@ -1879,6 +1880,140 @@ test_h5o_getinfo_visit(void)
 
 } /* test_h5o_getinfo_visit() */
 
+#define G1       "g1"           /* Group /g1 */
+#define G1G2     "g1/g2"        /* Group /g1/g2 */
+#define D1G1G2   "/g1/g2/dset1" /* Dataset /g1/g2/dset1 */
+#define NUM_OBJS 4              /* Number of objects including root group */
+
+typedef struct {
+    unsigned idx; /* Index in object visit structure */
+} ovisit2_ud_t;
+
+/* Names of objects being visited, in this order */
+static const char *visited_objs[] = {".", "g1", "g1/g2", "g1/g2/dset1"};
+
+/****************************************************************
+**
+**  visit_obj_cb():
+**      This is the callback function invoked by H5Ovisit1() in
+**      test_h5o_getinfo_visit():
+**      --Verify that the object info returned to the callback
+**        function is the same as H5Oget_info2().
+**
+****************************************************************/
+static int
+visit2_obj_cb(hid_t H5_ATTR_UNUSED obj_id, const char *name, const H5O_info1_t H5_ATTR_UNUSED *info,
+              void *_op_data)
+{
+
+    ovisit2_ud_t *op_data = (ovisit2_ud_t *)_op_data;
+
+    if (strcmp(visited_objs[op_data->idx], name) != 0)
+        return H5_ITER_ERROR;
+
+    /* Advance to next location in expected output */
+    op_data->idx++;
+
+    return H5_ITER_CONT;
+}
+
+/****************************************************************
+**
+**  test_h5o_visit2():
+**    Verify that H5Ovisit2 visits the nested groups and objects
+**    instead of only root group.
+**
+****************************************************************/
+static void
+test_h5o_visit2(void)
+{
+    hid_t           fid  = H5I_INVALID_HID;                         /* HDF5 File ID */
+    hid_t           gid1 = H5I_INVALID_HID, gid2 = H5I_INVALID_HID; /* Group IDs */
+    hid_t           sid = H5I_INVALID_HID;                          /* Dataspace ID */
+    hid_t           did = H5I_INVALID_HID;                          /* Dataset ID */
+    hid_t           obj_id;                                         /* Object ID for root group */
+    char            filename[1024];                                 /* File used in this test */
+    ovisit2_ud_t    udata;                                          /* User-data for visiting */
+    unsigned        fields;                                         /* Fields to return */
+    H5_index_t      idx_type = H5_INDEX_NAME;
+    H5_iter_order_t order    = H5_ITER_DEC;
+    bool            vol_is_native;
+    herr_t          ret; /* Value returned from API calls */
+
+    /* Output message about test being performed */
+    MESSAGE(5, ("Testing H5Ovisit2 visits all nested groups and objects\n"));
+
+    h5_fixname(VISIT2_FILENAME, H5P_DEFAULT, filename, sizeof filename);
+
+    /* Create an HDF5 file */
+    fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(fid, FAIL, "H5Fcreate");
+
+    /* Check if native VOL is being used */
+    CHECK(h5_using_native_vol(H5P_DEFAULT, fid, &vol_is_native), FAIL, "h5_using_native_vol");
+    if (!vol_is_native) {
+        CHECK(H5Fclose(fid), FAIL, "H5Fclose");
+        MESSAGE(5, (" -- SKIPPED --\n"));
+        return;
+    }
+
+    /* Create group G1 in the file */
+    gid1 = H5Gcreate2(fid, G1, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(gid1, FAIL, "H5Gcreate2");
+
+    /* Create group G1G2 in the file */
+    gid2 = H5Gcreate2(fid, G1G2, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(gid2, FAIL, "H5Gcreate2");
+
+    /* Create dataspace */
+    sid = H5Screate(H5S_SCALAR);
+    CHECK(sid, FAIL, "H5Screate");
+
+    did = H5Dcreate2(gid2, D1G1G2, H5T_NATIVE_INT, sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(did, FAIL, "H5Dcreate2");
+
+    H5Dclose(did);
+    H5Gclose(gid2);
+    H5Gclose(gid1);
+
+    /* Open root object */
+    obj_id = H5Oopen(fid, "/", H5P_DEFAULT);
+    CHECK(obj_id, FAIL, "H5Oopen");
+
+    /* Visit root with H5O_INFO_META_SIZE */
+    udata.idx = 0; /* first object, i.e., root group */
+    fields    = H5O_INFO_META_SIZE;
+    ret       = H5Ovisit2(obj_id, idx_type, order, visit2_obj_cb, &udata, fields);
+    CHECK(ret, FAIL, "H5Ovisit2");
+
+    /* Verify that all objects were visitted */
+    VERIFY(udata.idx, NUM_OBJS, "idx should be the same as NUM_OBJS");
+
+    /* Visit root with H5O_INFO_BASIC */
+    udata.idx = 0; /* first object, i.e., root group */
+    fields    = H5O_INFO_BASIC;
+    ret       = H5Ovisit2(obj_id, idx_type, order, visit2_obj_cb, &udata, fields);
+    CHECK(ret, FAIL, "H5Ovisit2");
+
+    /* Verify that all objects were visitted */
+    VERIFY(udata.idx, NUM_OBJS, "idx should be the same as NUM_OBJS");
+
+    /* Visit root with H5O_INFO_ALL */
+    udata.idx = 0; /* first object, i.e., root group */
+    fields    = H5O_INFO_ALL;
+    ret       = H5Ovisit2(obj_id, idx_type, order, visit2_obj_cb, &udata, fields);
+    CHECK(ret, FAIL, "H5Ovisit2");
+
+    /* Verify that all objects were visitted */
+    VERIFY(udata.idx, NUM_OBJS, "idx should be the same as NUM_OBJS");
+
+    /* Close object and file */
+    ret = H5Oclose(obj_id);
+    CHECK(ret, FAIL, "H5Oclose");
+    ret = H5Fclose(fid);
+    CHECK(ret, FAIL, "H5Fclose");
+}
+
 #endif /* H5_NO_DEPRECATED_SYMBOLS */
 
 /****************************************************************
@@ -1907,6 +2042,7 @@ test_h5o(const void H5_ATTR_UNUSED *params)
 #ifndef H5_NO_DEPRECATED_SYMBOLS
     test_h5o_open_by_addr_deprec(); /* Test opening objects by address with H5Lget_info1 */
     test_h5o_getinfo_visit();       /* Test object info for H5Oget_info1/2 and H5Ovisit1 */
+    test_h5o_visit2();              /* Test behavior of H5Ovisit2 */
 #endif                              /* H5_NO_DEPRECATED_SYMBOLS */
 } /* test_h5o() */
 
@@ -1929,6 +2065,8 @@ cleanup_h5o(void H5_ATTR_UNUSED *params)
         {
             h5_fixname(TEST_FILENAME, H5P_DEFAULT, filename, sizeof filename);
             H5Fdelete(filename, H5P_DEFAULT);
+            h5_fixname(TEST_FILENAME, H5P_DEFAULT, filename, sizeof filename);
+            H5Fdelete(filename, H5P_DEFAULT);
         }
         H5E_END_TRY
     }