From 700d1a24df8f49ebda69d7344512c217a81d9d62 Mon Sep 17 00:00:00 2001 From: Dana Robinson <43805+derobins@users.noreply.github.com> Date: Sun, 10 Mar 2024 16:16:41 -0700 Subject: Convert H5B__assert to use error checks (#4109) Switches assert() calls to HGOTO_ERROR in H5B__assert() so it can be used in production mode. Also renames it to H5B__verify_structure() to better reflect what it checks. --- src/H5Bdbg.c | 114 ++++++++++++++++++++++++++++++++--------------------------- src/H5Bpkg.h | 16 +++++---- 2 files changed, 70 insertions(+), 60 deletions(-) diff --git a/src/H5Bdbg.c b/src/H5Bdbg.c index 9b12da1..6bbda05 100644 --- a/src/H5Bdbg.c +++ b/src/H5Bdbg.c @@ -53,9 +53,7 @@ H5B_debug(H5F_t *f, haddr_t addr, FILE *stream, int indent, int fwidth, const H5 FUNC_ENTER_NOAPI(FAIL) - /* - * Check arguments. - */ + /* Check arguments */ assert(f); assert(H5_addr_defined(addr)); assert(stream); @@ -129,26 +127,21 @@ done: } /* end H5B_debug() */ /*------------------------------------------------------------------------- - * Function: H5B__assert + * Function: H5B__verify_structure * * Purpose: Verifies that the tree is structured correctly * - * Relies on assert(), so only built when NDEBUG is not set - * - * Return: Success: SUCCEED - * Failure: assert() + * Return: SUCCEED/FAIL *------------------------------------------------------------------------- */ -#ifndef NDEBUG herr_t -H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata) +H5B__verify_structure(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata) { H5B_t *bt = NULL; H5UC_t *rc_shared; /* Ref-counted shared info */ H5B_shared_t *shared; /* Pointer to shared B-tree info */ H5B_cache_ud_t cache_udata; /* User-data for metadata cache callback */ - int ncell, cmp; - herr_t status; + int cmp; herr_t ret_value = SUCCEED; /* Return value */ /* A queue of child data */ @@ -163,62 +156,76 @@ H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata) /* Get shared info for B-tree */ if (NULL == (rc_shared = (type->get_shared)(f, udata))) HGOTO_ERROR(H5E_BTREE, H5E_CANTGET, FAIL, "can't retrieve B-tree's shared ref. count object"); - shared = (H5B_shared_t *)H5UC_GET_OBJ(rc_shared); - assert(shared); + if (NULL == (shared = (H5B_shared_t *)H5UC_GET_OBJ(rc_shared))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTGET, FAIL, "can't retrieve B-tree's ref counted shared info"); /* Initialize the queue */ cache_udata.f = f; cache_udata.type = type; cache_udata.rc_shared = rc_shared; - bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__READ_ONLY_FLAG); - assert(bt); - shared = (H5B_shared_t *)H5UC_GET_OBJ(bt->rc_shared); - assert(shared); - cur = (struct child_t *)H5MM_calloc(sizeof(struct child_t)); - assert(cur); + + if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, addr, &cache_udata, H5AC__READ_ONLY_FLAG))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "can't protect B-tree node"); + + if (NULL == (shared = (H5B_shared_t *)H5UC_GET_OBJ(bt->rc_shared))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTGET, FAIL, "can't get B-tree shared data"); + + if (NULL == (cur = (struct child_t *)H5MM_calloc(sizeof(struct child_t)))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTALLOC, FAIL, "can't allocate memory for queue"); + cur->addr = addr; cur->level = bt->level; head = tail = cur; - status = H5AC_unprotect(f, H5AC_BT, addr, bt, H5AC__NO_FLAGS_SET); - assert(status >= 0); + if (H5AC_unprotect(f, H5AC_BT, addr, bt, H5AC__NO_FLAGS_SET) < 0) + HGOTO_ERROR(H5E_BTREE, H5E_CANTUNPROTECT, FAIL, "can't unprotect B-tree node"); + bt = NULL; /* Make certain future references will be caught */ - /* - * Do a breadth-first search of the tree. New nodes are added to the end + /* Do a breadth-first search of the tree. New nodes are added to the end * of the queue as the `cur' pointer is advanced toward the end. We don't * remove any nodes from the queue because we need them in the uniqueness * test. */ - for (ncell = 0; cur; ncell++) { - bt = (H5B_t *)H5AC_protect(f, H5AC_BT, cur->addr, &cache_udata, H5AC__READ_ONLY_FLAG); - assert(bt); + while (cur) { + if (NULL == (bt = (H5B_t *)H5AC_protect(f, H5AC_BT, cur->addr, &cache_udata, H5AC__READ_ONLY_FLAG))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTPROTECT, FAIL, "can't protect B-tree node"); /* Check node header */ - assert(bt->level == cur->level); - if (cur->next && cur->next->level == bt->level) - assert(H5_addr_eq(bt->right, cur->next->addr)); - else - assert(!H5_addr_defined(bt->right)); - if (prev && prev->level == bt->level) - assert(H5_addr_eq(bt->left, prev->addr)); - else - assert(!H5_addr_defined(bt->left)); + if (bt->level != cur->level) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "B-tree level incorrect"); + + if (cur->next && cur->next->level == bt->level) { + if (!H5_addr_eq(bt->right, cur->next->addr)) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "right address should not equal next"); + } + else { + if (H5_addr_defined(bt->right)) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "bt->right should be HADDR_UNDEF"); + } + + if (prev && prev->level == bt->level) { + if (!H5_addr_eq(bt->left, prev->addr)) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "left address should not equal previous"); + } + else { + if (H5_addr_defined(bt->left)) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "bt->left should be HADDR_UNDEF"); + } if (cur->level > 0) { - unsigned u; - - for (u = 0; u < bt->nchildren; u++) { - /* - * Check that child nodes haven't already been seen. If they + for (unsigned u = 0; u < bt->nchildren; u++) { + /* Check that child nodes haven't already been seen. If they * have then the tree has a cycle. */ for (tmp = head; tmp; tmp = tmp->next) - assert(H5_addr_ne(tmp->addr, bt->child[u])); + if (!H5_addr_ne(tmp->addr, bt->child[u])) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "cycle detected in tree"); /* Add the child node to the end of the queue */ - tmp = (struct child_t *)H5MM_calloc(sizeof(struct child_t)); - assert(tmp); + if (NULL == (tmp = (struct child_t *)H5MM_calloc(sizeof(struct child_t)))) + HGOTO_ERROR(H5E_BTREE, H5E_CANTALLOC, FAIL, "can't allocate memory for child node"); + tmp->addr = bt->child[u]; tmp->level = bt->level - 1; tail->next = tmp; @@ -226,28 +233,29 @@ H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata) /* Check that the keys are monotonically increasing */ cmp = (type->cmp2)(H5B_NKEY(bt, shared, u), udata, H5B_NKEY(bt, shared, u + 1)); - assert(cmp < 0); - } /* end for */ - } /* end if */ + if (cmp >= 0) + HGOTO_ERROR(H5E_BTREE, H5E_BADVALUE, FAIL, "keys not monotonically increasing"); + } + } /* Release node */ - status = H5AC_unprotect(f, H5AC_BT, cur->addr, bt, H5AC__NO_FLAGS_SET); - assert(status >= 0); + if (H5AC_unprotect(f, H5AC_BT, cur->addr, bt, H5AC__NO_FLAGS_SET) < 0) + HGOTO_ERROR(H5E_BTREE, H5E_CANTUNPROTECT, FAIL, "can't unprotect B-tree node"); + bt = NULL; /* Make certain future references will be caught */ /* Advance current location in queue */ prev = cur; cur = cur->next; - } /* end for */ + } /* Free all entries from queue */ while (head) { tmp = head->next; H5MM_xfree(head); head = tmp; - } /* end while */ + } done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5B__assert() */ -#endif /* !NDEBUG */ +} /* end H5B__verify_structure() */ diff --git a/src/H5Bpkg.h b/src/H5Bpkg.h index d181867..d1ad647 100644 --- a/src/H5Bpkg.h +++ b/src/H5Bpkg.h @@ -26,8 +26,8 @@ #include "H5Bprivate.h" /* Other private headers needed by this file */ -#include "H5ACprivate.h" /* Metadata cache */ -#include "H5FLprivate.h" /* Free Lists */ +#include "H5ACprivate.h" /* Metadata cache */ +#include "H5FLprivate.h" /* Free Lists */ /**************************/ /* Package Private Macros */ @@ -35,13 +35,15 @@ /* Get the native key at a given index */ #define H5B_NKEY(b, shared, idx) ((b)->native + (shared)->nkey[(idx)]) -#define LEVEL_BITS 8 /* # of bits for node level: 1 byte */ + +/* # of bits for node level: 1 byte */ +#define LEVEL_BITS 8 /****************************/ /* Package Private Typedefs */ /****************************/ -/* The B-tree node as stored in memory... */ +/* The B-tree node as stored in memory */ typedef struct H5B_t { H5AC_info_t cache_info; /* Information for H5AC cache functions */ /* MUST be first field in structure */ @@ -78,8 +80,8 @@ H5FL_EXTERN(H5B_t); /* Package Private Prototypes */ /******************************/ H5_DLL herr_t H5B__node_dest(H5B_t *bt); -#ifndef NDEBUG -herr_t H5B__assert(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata); -#endif + +/* For debugging */ +H5_DLL herr_t H5B__verify_structure(H5F_t *f, haddr_t addr, const H5B_class_t *type, void *udata); #endif /*H5Bpkg_H*/ -- cgit v0.12