Skip to content

Commit

Permalink
Fixed H5Ovisit2() change of behavior between 1.10.11 and v1.14.4.3 (#…
Browse files Browse the repository at this point in the history 
…5022)

H5O__visit() uses the object information to be returned to the
application, so when the application did not request for certain
information, they were not available to H5O__visit.  This lack of
information caused incorrect behavior down the road.

We now call H5O_get_info again providing H5O_INFO_BASIC for "fields",
so we can obtain correct object information for H5O__visit to use.

Fixes #4941
  • Loading branch information
@bmribler
bmribler authored Nov 4, 2024
1 parent 8eb4c0e commit 49935f8
Show file tree
Hide file tree
Showing 3 changed files with 198 additions and 5 deletions.
20 changes: 20 additions & 0 deletions release_docs/RELEASE.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,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

Expand Down
20 changes: 15 additions & 5 deletions src/H5Oint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -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)))
Expand All @@ -2692,7 +2702,7 @@ H5O__visit(H5G_loc_t *loc, const char *obj_name, H5_index_t idx_type, H5_iter_or
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 */

Expand All @@ -2713,18 +2723,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");

Expand Down
163 changes: 163 additions & 0 deletions test/th5o.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1879,6 +1879,164 @@ test_h5o_getinfo_visit(void)

} /* test_h5o_getinfo_visit() */

#define G1 "g1" /* Group /g1 */
#define G1G2 "g1/g2" /* Group /g1/g2 */
#define ATTR1 "Attr1" /* Attribute Attr1 */
#define D1G1G2 "/g1/g2/dset1" /* Dataset /g1/g2/dset1 */
#define NUM_OBJS 4 /* Number of objects including root group */
#define NUM_ATTRS 1 /* Number of attributes belong to root group */
#define VISIT2_FILENAME "th5o_visit2"

typedef struct {
unsigned idx; /* Index in object visit structure */
unsigned fields; /* Fields to verify number of attributes in callback */
} 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 obj_id, const char *name, const H5O_info1_t H5_ATTR_UNUSED *info, void *_op_data)
{
H5O_info1_t oinfo;

memset(&oinfo, 0, sizeof(oinfo));
ovisit2_ud_t *op_data = (ovisit2_ud_t *)_op_data;

if (strcmp(visited_objs[op_data->idx], name) != 0)
return H5_ITER_ERROR;

if (H5Oget_info2(obj_id, &oinfo, op_data->fields) < 0)
return H5_ITER_ERROR;

if (op_data->fields == H5O_INFO_NUM_ATTRS)
if (oinfo.num_attrs != NUM_ATTRS)
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 attid = H5I_INVALID_HID; /* Attribute 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");

/* Add an attribute to verify H5O_get_info with H5O_INFO_NUM_ATTRS works */
attid = H5Acreate2(fid, ATTR1, H5T_NATIVE_INT, sid, H5P_DEFAULT, H5P_DEFAULT);
CHECK(attid, FAIL, "H5Acreate2");

H5Aclose(attid);
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 = udata.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 = udata.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 = udata.fields = H5O_INFO_ALL;
ret = H5Ovisit2(obj_id, idx_type, order, visit2_obj_cb, &udata, fields);
CHECK(ret, FAIL, "H5Ovisit2");

/* Visit root with H5O_INFO_NUM_ATTRS */
udata.idx = 0; /* first object, i.e., root group */
fields = udata.fields = H5O_INFO_NUM_ATTRS;
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 */

/****************************************************************
Expand Down Expand Up @@ -1907,6 +2065,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() */

Expand All @@ -1929,6 +2088,10 @@ cleanup_h5o(void H5_ATTR_UNUSED *params)
{
h5_fixname(TEST_FILENAME, H5P_DEFAULT, filename, sizeof filename);
H5Fdelete(filename, H5P_DEFAULT);
#ifndef H5_NO_DEPRECATED_SYMBOLS
h5_fixname(VISIT2_FILENAME, H5P_DEFAULT, filename, sizeof filename);
H5Fdelete(filename, H5P_DEFAULT);
#endif /* H5_NO_DEPRECATED_SYMBOLS */
}
H5E_END_TRY
}
Expand Down

0 comments on commit 49935f8

Please sign in to comment.