Skip to content

Commit

Permalink
Remove H5E_clear_stack() from H5SM code (#4861)
Browse files Browse the repository at this point in the history
Introduces 'try' flag for H5B2_modify() call
  • Loading branch information
qkoziol authored Oct 3, 2024
1 parent e8f6bb6 commit 626f639
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 55 deletions.
6 changes: 3 additions & 3 deletions src/H5Adense.c
Original file line number Diff line number Diff line change
Expand Up @@ -633,8 +633,8 @@ H5A__dense_write_bt2_cb(void *_record, void *_op_data, bool *changed)
udata.found_op_data = NULL;

/* Modify record for creation order index */
if (H5B2_modify(bt2_corder, &udata, H5A__dense_write_bt2_cb2, &op_data->attr->sh_loc.u.heap_id) <
0)
if (H5B2_modify(bt2_corder, &udata, false, H5A__dense_write_bt2_cb2,
&op_data->attr->sh_loc.u.heap_id) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTINSERT, FAIL, "unable to modify record in v2 B-tree");
} /* end if */

Expand Down Expand Up @@ -763,7 +763,7 @@ H5A__dense_write(H5F_t *f, const H5O_ainfo_t *ainfo, H5A_t *attr)
op_data.corder_bt2_addr = ainfo->corder_bt2_addr;

/* Modify attribute through 'name' tracking v2 B-tree */
if (H5B2_modify(bt2_name, &udata, H5A__dense_write_bt2_cb, &op_data) < 0)
if (H5B2_modify(bt2_name, &udata, false, H5A__dense_write_bt2_cb, &op_data) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTINSERT, FAIL, "unable to modify record in v2 B-tree");

done:
Expand Down
21 changes: 13 additions & 8 deletions src/H5B2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1115,7 +1115,7 @@ H5B2_neighbor(H5B2_t *bt2, H5B2_compare_t range, void *udata, H5B2_found_t op, v
*-------------------------------------------------------------------------
*/
herr_t
H5B2_modify(H5B2_t *bt2, void *udata, H5B2_modify_t op, void *op_data)
H5B2_modify(H5B2_t *bt2, void *udata, bool try, H5B2_modify_t op, void *op_data)
{
H5B2_hdr_t *hdr; /* Pointer to the B-tree header */
H5B2_node_ptr_t curr_node_ptr; /* Node pointer info for current node */
Expand All @@ -1142,8 +1142,13 @@ H5B2_modify(H5B2_t *bt2, void *udata, H5B2_modify_t op, void *op_data)
curr_node_ptr = hdr->root;

/* Check for empty tree */
if (0 == curr_node_ptr.node_nrec)
HGOTO_ERROR(H5E_BTREE, H5E_NOTFOUND, FAIL, "B-tree has no records");
if (0 == curr_node_ptr.node_nrec) {
/* Don't fail if the caller set the 'try' flag */
if (try)
HGOTO_DONE(SUCCEED);
else
HGOTO_ERROR(H5E_BTREE, H5E_NOTFOUND, FAIL, "B-tree has no records");
} /* end if */

/* Current depth of the tree */
depth = hdr->depth;
Expand Down Expand Up @@ -1277,11 +1282,11 @@ H5B2_modify(H5B2_t *bt2, void *udata, H5B2_modify_t op, void *op_data)
if (H5AC_unprotect(hdr->f, H5AC_BT2_LEAF, curr_node_ptr.addr, leaf, H5AC__NO_FLAGS_SET) < 0)
HGOTO_ERROR(H5E_BTREE, H5E_CANTUNPROTECT, FAIL, "unable to release B-tree node");

/* Note: don't push error on stack, leave that to next higher level,
* since many times the B-tree is searched in order to determine
* if an object exists in the B-tree or not.
*/
HGOTO_DONE(FAIL);
/* Don't fail if the caller set the 'try' flag */
if (try)
HGOTO_DONE(SUCCEED);
else
HGOTO_ERROR(H5E_BTREE, H5E_NOTFOUND, FAIL, "record not found");
}
else {
/* Make callback for current record */
Expand Down
2 changes: 1 addition & 1 deletion src/H5B2private.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ H5_DLL herr_t H5B2_iterate(H5B2_t *bt2, H5B2_operator_t op, void *op_data);
H5_DLL herr_t H5B2_find(H5B2_t *bt2, void *udata, bool *found, H5B2_found_t op, void *op_data);
H5_DLL herr_t H5B2_index(H5B2_t *bt2, H5_iter_order_t order, hsize_t idx, H5B2_found_t op, void *op_data);
H5_DLL herr_t H5B2_neighbor(H5B2_t *bt2, H5B2_compare_t range, void *udata, H5B2_found_t op, void *op_data);
H5_DLL herr_t H5B2_modify(H5B2_t *bt2, void *udata, H5B2_modify_t op, void *op_data);
H5_DLL herr_t H5B2_modify(H5B2_t *bt2, void *udata, bool try, H5B2_modify_t op, void *op_data);
H5_DLL herr_t H5B2_update(H5B2_t *bt2, void *udata, H5B2_modify_t op, void *op_data);
H5_DLL herr_t H5B2_remove(H5B2_t *b2, void *udata, H5B2_remove_t op, void *op_data);
H5_DLL herr_t H5B2_remove_by_idx(H5B2_t *bt2, H5_iter_order_t order, hsize_t idx, H5B2_remove_t op,
Expand Down
43 changes: 25 additions & 18 deletions src/H5SM.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,13 @@ typedef struct H5SM_read_udata_t {
void *encoding_buf; /* The encoded message (out) */
} H5SM_read_udata_t;

/* Typedef to increment a reference count in the B-tree */
typedef struct {
H5SM_mesg_key_t *key; /* IN: key for message being incremented */
bool found; /* OUT: if message was found */
H5O_fheap_id_t fheap_id; /* OUT: fheap ID of record */
} H5SM_incr_ref_opdata_t;

/********************/
/* Local Prototypes */
/********************/
Expand Down Expand Up @@ -1141,9 +1148,9 @@ H5SM_try_share(H5F_t *f, H5O_t *open_oh, unsigned defer_flags, unsigned type_id,
static herr_t
H5SM__incr_ref(void *record, void *_op_data, bool *changed)
{
H5SM_sohm_t *message = (H5SM_sohm_t *)record;
H5SM_incr_ref_opdata *op_data = (H5SM_incr_ref_opdata *)_op_data;
herr_t ret_value = SUCCEED;
H5SM_sohm_t *message = (H5SM_sohm_t *)record;
H5SM_incr_ref_opdata_t *op_data = (H5SM_incr_ref_opdata_t *)_op_data;
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE

Expand Down Expand Up @@ -1174,9 +1181,9 @@ H5SM__incr_ref(void *record, void *_op_data, bool *changed)
/* If we got here, the message has changed */
*changed = true;

/* Check for retrieving the heap ID */
if (op_data)
op_data->fheap_id = message->u.heap_loc.fheap_id;
/* Set the heap ID and indicate it was found */
op_data->fheap_id = message->u.heap_loc.fheap_id;
op_data->found = true;

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -1326,24 +1333,24 @@ H5SM__write_mesg(H5F_t *f, H5O_t *open_oh, H5SM_index_header_t *header, bool def
HGOTO_ERROR(H5E_SOHM, H5E_NOTFOUND, FAIL, "can't search for message in index");
} /* end if */
else {
H5SM_incr_ref_opdata op_data;
H5SM_incr_ref_opdata_t op_data;

/* Set up callback info */
op_data.key = &key;
op_data.key = &key;
op_data.found = false;

/* If this returns failure, it means that the message wasn't found. */
/* If it succeeds, set the heap_id in the shared struct. It will
* return a heap ID, since a message with a reference count greater
* than 1 is always shared in the heap.
/* Set the heap_id in the shared struct, if the message was found.
* It will return a heap ID, since a message with a reference count
* greater than 1 is always shared in the heap.
*/
if (H5B2_modify(bt2, &key, H5SM__incr_ref, &op_data) >= 0) {
if (H5B2_modify(bt2, &key, true, H5SM__incr_ref, &op_data) < 0)
HGOTO_ERROR(H5E_SOHM, H5E_CANTMODIFY, FAIL, "B-tree modification failed");
if (op_data.found) {
shared.u.heap_id = op_data.fheap_id;
found = true;
} /* end if */
else
H5E_clear_stack(); /*ignore error*/
} /* end else */
} /* end else */
} /* end else */
} /* end else */

if (found) {
/* If the message was found, it's shared in the heap (now). Set up a
Expand Down Expand Up @@ -1807,7 +1814,7 @@ H5SM__delete_from_index(H5F_t *f, H5O_t *open_oh, H5SM_index_header_t *header, c
/* If this returns failure, it means that the message wasn't found.
* If it succeeds, a copy of the modified message will be returned.
*/
if (H5B2_modify(bt2, &key, H5SM__decr_ref, &message) < 0)
if (H5B2_modify(bt2, &key, false, H5SM__decr_ref, &message) < 0)
HGOTO_ERROR(H5E_SOHM, H5E_NOTFOUND, FAIL, "message not in index");

/* Point to the message */
Expand Down
18 changes: 0 additions & 18 deletions src/H5SMpkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -201,24 +201,6 @@ typedef struct {
* heap ID, the heap ID will be 0. */
} H5SM_mesg_key_t;

/*
* Data exchange structure to pass through the fractal heap layer for the
* H5HF_op function when computing a hash value for a message.
*/
typedef struct {
/* downward (internal) */
unsigned type_id; /* Message type */

/* upward */
uint32_t hash; /* Hash value */
} H5SM_fh_ud_gh_t;

/* Typedef to increment a reference count in the B-tree */
typedef struct {
H5SM_mesg_key_t *key; /* IN: key for message being incremented */
H5O_fheap_id_t fheap_id; /* OUT: fheap ID of record */
} H5SM_incr_ref_opdata;

/* v2 B-tree client callback context */
typedef struct H5SM_bt2_ctx_t {
uint8_t sizeof_addr; /* Size of file addresses */
Expand Down
14 changes: 7 additions & 7 deletions test/btree2.c
Original file line number Diff line number Diff line change
Expand Up @@ -9575,7 +9575,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
modify = 4;
H5E_BEGIN_TRY
{
ret = H5B2_modify(bt2, &record, modify_cb, &modify);
ret = H5B2_modify(bt2, &record, false, modify_cb, &modify);
}
H5E_END_TRY
/* Should fail */
Expand All @@ -9600,7 +9600,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
/* Attempt to modify a record in a leaf node */
record = 4330;
modify = 4331;
if (H5B2_modify(bt2, &record, modify_cb, &modify) < 0)
if (H5B2_modify(bt2, &record, false, modify_cb, &modify) < 0)
FAIL_STACK_ERROR;

/* Check status of B-tree */
Expand All @@ -9626,7 +9626,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
found = HSIZET_MAX;
H5E_BEGIN_TRY
{
ret = H5B2_modify(bt2, &record, modify_cb, &modify);
ret = H5B2_modify(bt2, &record, false, modify_cb, &modify);
}
H5E_END_TRY
/* Should fail */
Expand All @@ -9651,7 +9651,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
/* Attempt to modify a record in an internal node */
record = 5350;
modify = 5352;
if (H5B2_modify(bt2, &record, modify_cb, &modify) < 0)
if (H5B2_modify(bt2, &record, false, modify_cb, &modify) < 0)
FAIL_STACK_ERROR;

/* Check status of B-tree */
Expand All @@ -9677,7 +9677,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
found = 5350;
H5E_BEGIN_TRY
{
ret = H5B2_modify(bt2, &record, modify_cb, &modify);
ret = H5B2_modify(bt2, &record, false, modify_cb, &modify);
}
H5E_END_TRY
/* Should fail */
Expand All @@ -9702,7 +9702,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
/* Attempt to modify a record in a root node */
record = 9445;
modify = 9448;
if (H5B2_modify(bt2, &record, modify_cb, &modify) < 0)
if (H5B2_modify(bt2, &record, false, modify_cb, &modify) < 0)
FAIL_STACK_ERROR;

/* Check status of B-tree */
Expand All @@ -9728,7 +9728,7 @@ test_modify(hid_t fapl, const H5B2_create_t *cparam, const bt2_test_param_t *tpa
found = 9445;
H5E_BEGIN_TRY
{
ret = H5B2_modify(bt2, &record, modify_cb, &modify);
ret = H5B2_modify(bt2, &record, false, modify_cb, &modify);
}
H5E_END_TRY
/* Should fail */
Expand Down

0 comments on commit 626f639

Please sign in to comment.