Skip to content

Commit

Permalink
Remove a call to H5E_clear_stack() in H5O code (HDFGroup#4875)
Browse files Browse the repository at this point in the history 
* Remove H5E_clear_stack() call

* Add note to H5F_clear_stack() to dissuade use

* Committing clang-format changes

* Eliminate "unknown" case from H5O__obj_type_real()

Also minor optimizations and warning cleanups
  • Loading branch information
@qkoziol
qkoziol authored Oct 1, 2024
1 parent 62e4777 commit e080498
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 52 deletions.
9 changes: 9 additions & 0 deletions src/H5Eint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1673,6 +1673,15 @@ H5E__clear_entries(H5E_stack_t *estack, size_t nentries)
*
* Purpose: Clear the default error stack
*
* Note: This routine should _not_ be used inside general library
* code in general. It creates complex locking issues for
* threadsafe code. Generally, using a 'try' parameter or
* an 'exists' parameter should be used if an operation is
* being used to probe for information. Remember: failing
* to locate a record is not an error for a data structure,
* although it could be an error for the user of the data
* structure.
*
* Return: SUCCEED/FAIL
*
*-------------------------------------------------------------------------
Expand Down
109 changes: 57 additions & 52 deletions src/H5Oint.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ static herr_t H5O__obj_type_real(const H5O_t *oh, H5O_type_t *obj_type);
static herr_t H5O__get_hdr_info_real(const H5O_t *oh, H5O_hdr_info_t *hdr);
static herr_t H5O__free_visit_visited(void *item, void *key, void *operator_data /*in,out*/);
static herr_t H5O__visit_cb(hid_t group, const char *name, const H5L_info2_t *linfo, void *_udata);
static const H5O_obj_class_t *H5O__obj_class_real(const H5O_t *oh);
static herr_t H5O__reset_info2(H5O_info2_t *oinfo);
static herr_t H5O__obj_class_real(const H5O_t *oh, const H5O_obj_class_t **cls);
static herr_t H5O__reset_info2(H5O_info2_t *oinfo);

/*********************/
/* Package Variables */
Expand Down Expand Up @@ -185,7 +185,7 @@ static const H5O_obj_class_t *const H5O_obj_class_g[] = {
* Failure: negative
*-------------------------------------------------------------------------
*/
herr_t
H5_ATTR_CONST herr_t
H5O_init(void)
{
herr_t ret_value = SUCCEED; /* Return value */
Expand Down Expand Up @@ -1594,37 +1594,36 @@ H5O_obj_type(const H5O_loc_t *loc, H5O_type_t *obj_type)
/*-------------------------------------------------------------------------
* Function: H5O__obj_type_real
*
* Purpose: Returns the type of object pointed to by `oh'.
* Purpose: On success, returns the type of object pointed to by `oh' or
* NULL in *obj_type. *obj_type not defined on failure.
*
* Return: Success: Non-negative
* Failure: Negative
* Return: Success: Non-negative
* Failure: Negative
*
*-------------------------------------------------------------------------
*/
static herr_t
H5O__obj_type_real(const H5O_t *oh, H5O_type_t *obj_type)
{
const H5O_obj_class_t *obj_class; /* Class of object for header */
const H5O_obj_class_t *obj_class = NULL; /* Class of object for header */
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE_NOERR
FUNC_ENTER_PACKAGE

/* Sanity check */
assert(oh);
assert(obj_type);

/* Look up class for object header */
if (NULL == (obj_class = H5O__obj_class_real(oh))) {
/* Clear error stack from "failed" class lookup */
H5E_clear_stack();
if (H5O__obj_class_real(oh, &obj_class) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object class");
assert(obj_class);

/* Set type to "unknown" */
*obj_type = H5O_TYPE_UNKNOWN;
}
else
/* Set object type */
*obj_type = obj_class->type;
/* Set object type */
*obj_type = obj_class->type;

FUNC_LEAVE_NOAPI(SUCCEED)
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5O__obj_type_real() */

/*-------------------------------------------------------------------------
Expand All @@ -1650,7 +1649,7 @@ H5O__obj_class(const H5O_loc_t *loc)
HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, NULL, "unable to load object header");

/* Test whether entry qualifies as a particular type of object */
if (NULL == (ret_value = H5O__obj_class_real(oh)))
if (H5O__obj_class_real(oh, &ret_value) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, NULL, "unable to determine object type");

done:
Expand All @@ -1663,37 +1662,41 @@ H5O__obj_class(const H5O_loc_t *loc)
/*-------------------------------------------------------------------------
* Function: H5O__obj_class_real
*
* Purpose: Returns the class of object pointed to by `oh'.
* Purpose: On success returns the class of object pointed to by `oh' or
* NULL in *cls. *cls not defined on failure.
*
* Return: Success: An object class
* Failure: NULL
* Return: Success: Non-negative
* Failure: Negative
*
*-------------------------------------------------------------------------
*/
static const H5O_obj_class_t *
H5O__obj_class_real(const H5O_t *oh)
static herr_t
H5O__obj_class_real(const H5O_t *oh, const H5O_obj_class_t **cls)
{
size_t i; /* Local index variable */
const H5O_obj_class_t *ret_value = NULL; /* Return value */
size_t i; /* Local index variable */
herr_t ret_value = SUCCEED;

FUNC_ENTER_PACKAGE

/* Sanity check */
assert(oh);
assert(cls);

/* Test whether entry qualifies as a particular type of object */
/* (Note: loop is in reverse order, to test specific objects first) */
for (i = NELMTS(H5O_obj_class_g); i > 0; --i) {
htri_t isa; /* Is entry a particular type? */

if ((isa = (H5O_obj_class_g[i - 1]->isa)(oh)) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, NULL, "unable to determine object type");
else if (isa)
HGOTO_DONE(H5O_obj_class_g[i - 1]);
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object type");
else if (isa) {
*cls = H5O_obj_class_g[i - 1];
break;
}
}

if (0 == i)
HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, NULL, "unable to determine object type");
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object type");

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -2071,9 +2074,8 @@ H5O__get_hdr_info_real(const H5O_t *oh, H5O_hdr_info_t *hdr)
herr_t
H5O_get_info(const H5O_loc_t *loc, H5O_info2_t *oinfo, unsigned fields)
{
const H5O_obj_class_t *obj_class; /* Class of object for header */
H5O_t *oh = NULL; /* Object header */
herr_t ret_value = SUCCEED; /* Return value */
H5O_t *oh = NULL; /* Object header */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_NOAPI_TAG(loc->addr, FAIL)

Expand All @@ -2085,25 +2087,27 @@ H5O_get_info(const H5O_loc_t *loc, H5O_info2_t *oinfo, unsigned fields)
if (NULL == (oh = H5O_protect(loc, H5AC__READ_ONLY_FLAG, false)))
HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, FAIL, "unable to load object header");

/* Get class for object */
if (NULL == (obj_class = H5O__obj_class_real(oh)))
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object class");

/* Reset the object info structure */
if (H5O__reset_info2(oinfo) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTSET, FAIL, "can't reset object data struct");

/* Get basic information, if requested */
if (fields & H5O_INFO_BASIC) {
H5O_type_t obj_type = H5O_TYPE_UNKNOWN; /* Type of object */

/* Retrieve the file's fileno */
H5F_GET_FILENO(loc->file, oinfo->fileno);

/* Set the object's address into the token */
if (H5VL_native_addr_to_token(loc->file, H5I_FILE, loc->addr, &oinfo->token) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTSERIALIZE, FAIL, "can't serialize address into object token");

/* Retrieve the type of the object */
oinfo->type = obj_class->type;
/* Get type of object */
if (H5O__obj_type_real(oh, &obj_type) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object type");

/* Set the type of the object */
oinfo->type = obj_type;

/* Set the object's reference count */
oinfo->rc = oh->nlink;
Expand Down Expand Up @@ -2177,9 +2181,8 @@ H5O_get_info(const H5O_loc_t *loc, H5O_info2_t *oinfo, unsigned fields)
herr_t
H5O_get_native_info(const H5O_loc_t *loc, H5O_native_info_t *oinfo, unsigned fields)
{
const H5O_obj_class_t *obj_class; /* Class of object for header */
H5O_t *oh = NULL; /* Object header */
herr_t ret_value = SUCCEED; /* Return value */
H5O_t *oh = NULL; /* Object header */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_NOAPI_TAG(loc->addr, FAIL)

Expand All @@ -2191,10 +2194,6 @@ H5O_get_native_info(const H5O_loc_t *loc, H5O_native_info_t *oinfo, unsigned fie
if (NULL == (oh = H5O_protect(loc, H5AC__READ_ONLY_FLAG, false)))
HGOTO_ERROR(H5E_OHDR, H5E_CANTPROTECT, FAIL, "unable to load object header");

/* Get class for object */
if (NULL == (obj_class = H5O__obj_class_real(oh)))
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object class");

/* Reset the object info structure */
memset(oinfo, 0, sizeof(*oinfo));

Expand All @@ -2205,6 +2204,12 @@ H5O_get_native_info(const H5O_loc_t *loc, H5O_native_info_t *oinfo, unsigned fie

/* Get B-tree & heap metadata storage size, if requested */
if (fields & H5O_NATIVE_INFO_META_SIZE) {
const H5O_obj_class_t *obj_class = NULL; /* Class of object for header */

/* Get class for object */
if (H5O__obj_class_real(oh, &obj_class) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to determine object class");

/* Check for 'bh_info' callback for this type of object */
if (obj_class->bh_info)
/* Call the object's class 'bh_info' routine */
Expand Down Expand Up @@ -2371,7 +2376,7 @@ H5O_obj_create(H5F_t *f, H5O_type_t obj_type, void *crt_info, H5G_loc_t *obj_loc
*
*-------------------------------------------------------------------------
*/
haddr_t
H5_ATTR_PURE haddr_t
H5O_get_oh_addr(const H5O_t *oh)
{
/* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */
Expand All @@ -2388,7 +2393,7 @@ H5O_get_oh_addr(const H5O_t *oh)
*
*-------------------------------------------------------------------------
*/
uint8_t
H5_ATTR_PURE uint8_t
H5O_get_oh_flags(const H5O_t *oh)
{
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand All @@ -2405,7 +2410,7 @@ H5O_get_oh_flags(const H5O_t *oh)
*
*-------------------------------------------------------------------------
*/
time_t
H5_ATTR_PURE time_t
H5O_get_oh_mtime(const H5O_t *oh)
{
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand All @@ -2419,7 +2424,7 @@ H5O_get_oh_mtime(const H5O_t *oh)
*
*-------------------------------------------------------------------------
*/
uint8_t
H5_ATTR_PURE uint8_t
H5O_get_oh_version(const H5O_t *oh)
{
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand Down Expand Up @@ -2837,7 +2842,7 @@ H5O_dec_rc_by_loc(const H5O_loc_t *loc)
*
*-------------------------------------------------------------------------
*/
H5AC_proxy_entry_t *
H5_ATTR_PURE H5AC_proxy_entry_t *
H5O_get_proxy(const H5O_t *oh)
{
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand Down Expand Up @@ -2943,7 +2948,7 @@ H5O__reset_info2(H5O_info2_t *oinfo)
*
*-------------------------------------------------------------------------
*/
bool
H5_ATTR_PURE bool
H5O_has_chksum(const H5O_t *oh)
{
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand Down

0 comments on commit e080498

Please sign in to comment.