From 626f6398504208973dbd7ba6aaa24af07543071b Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Wed, 2 Oct 2024 21:55:37 -0500 Subject: [PATCH] Remove H5E_clear_stack() from H5SM code (#4861) Introduces 'try' flag for H5B2_modify() call --- src/H5Adense.c | 6 +++--- src/H5B2.c | 21 +++++++++++++-------- src/H5B2private.h | 2 +- src/H5SM.c | 43 +++++++++++++++++++++++++------------------ src/H5SMpkg.h | 18 ------------------ test/btree2.c | 14 +++++++------- 6 files changed, 49 insertions(+), 55 deletions(-) diff --git a/src/H5Adense.c b/src/H5Adense.c index 52a6244d7be..143fa9b3646 100644 --- a/src/H5Adense.c +++ b/src/H5Adense.c @@ -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 */ @@ -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: diff --git a/src/H5B2.c b/src/H5B2.c index f49689911dc..d3eceff756e 100644 --- a/src/H5B2.c +++ b/src/H5B2.c @@ -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 */ @@ -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; @@ -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 */ diff --git a/src/H5B2private.h b/src/H5B2private.h index f3f1eaf8a65..bf06a8bd846 100644 --- a/src/H5B2private.h +++ b/src/H5B2private.h @@ -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, diff --git a/src/H5SM.c b/src/H5SM.c index 1c2d4e6caa7..59abd24e679 100644 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -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 */ /********************/ @@ -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 @@ -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) @@ -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 @@ -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 */ diff --git a/src/H5SMpkg.h b/src/H5SMpkg.h index 4ff4c7da81f..dbeafc15139 100644 --- a/src/H5SMpkg.h +++ b/src/H5SMpkg.h @@ -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 */ diff --git a/test/btree2.c b/test/btree2.c index 7653da5ec6e..1fb4d882d5e 100644 --- a/test/btree2.c +++ b/test/btree2.c @@ -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 */ @@ -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 */ @@ -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 */ @@ -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 */ @@ -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 */ @@ -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 */ @@ -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 */