Skip to content

Commit

Permalink
Start refactoring H5E code to avoid using IDs internally (HDFGroup#4427)
Browse files Browse the repository at this point in the history
  • Loading branch information
qkoziol authored Apr 25, 2024
1 parent 0ce1a96 commit 603f8b1
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 112 deletions.
35 changes: 31 additions & 4 deletions bin/make_err
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ sub create_init ($) {
my $desc; # Description of error message
my $sect_name; # Section of minor error messages
my $sect_desc; # Description of section
my $first_major = 0; # Whether the first major error code was saved
my $first_minor = 0; # Whether the first minor error code was saved

# Rename previous file
# rename "${prefix}${file}", "${prefix}${file}~" or die "unable to make backup";
Expand All @@ -241,12 +243,22 @@ sub create_init ($) {
print HEADER "/* Major error codes */\n";
print HEADER "/*********************/\n\n";
foreach $name (keys %major) {
print HEADER " "x(0*$indent),"assert(${name}_g==(-1));\n";
print HEADER " "x(0*$indent),"assert(${name}_g==H5I_INVALID_HID);\n";
print HEADER " "x(0*$indent),"if((msg = H5E__create_msg(cls, H5E_MAJOR, \"${major{$name}}\"))==NULL)\n";
print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTINIT, FAIL, \"error message initialization failed\");\n";
print HEADER " "x(0*$indent),"if((${name}_g = H5I_register(H5I_ERROR_MSG, msg, false))<0)\n";
print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTREGISTER, FAIL, \"can't register error message\");\n";
if ($first_major == 0) {
print HEADER " "x(0*$indent),"\n/* Remember first major error code ID */\n";
print HEADER " "x(0*$indent),"assert(H5E_first_maj_id_g==H5I_INVALID_HID);\n";
print HEADER " "x(0*$indent),"H5E_first_maj_id_g = ${name}_g;\n\n";
$first_major = 1;
}
$last_name = $name;
}
print HEADER " "x(0*$indent),"\n/* Remember last major error code ID */\n";
print HEADER " "x(0*$indent),"assert(H5E_last_maj_id_g==H5I_INVALID_HID);\n";
print HEADER " "x(0*$indent),"H5E_last_maj_id_g = ${last_name}_g;\n\n";

# Iterate over all the minor error sections
print HEADER "\n/*********************/\n";
Expand All @@ -257,13 +269,24 @@ sub create_init ($) {

# Iterate over all the minor errors in each section
for $name ( @{$section_list{$sect_name}}) {
print HEADER " "x(0*$indent),"assert(${name}_g==(-1));\n";
print HEADER " "x(0*$indent),"assert(${name}_g==H5I_INVALID_HID);\n";
print HEADER " "x(0*$indent),"if((msg = H5E__create_msg(cls, H5E_MINOR, \"${minor{$name}}\"))==NULL)\n";
print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTINIT, FAIL, \"error message initialization failed\");\n";
print HEADER " "x(0*$indent),"if((${name}_g = H5I_register(H5I_ERROR_MSG, msg, true))<0)\n";
print HEADER " "x(1*$indent),"HGOTO_ERROR(H5E_ERROR, H5E_CANTREGISTER, FAIL, \"can't register error message\");\n";

if ($first_minor == 0) {
print HEADER " "x(0*$indent),"\n/* Remember first minor error code ID */\n";
print HEADER " "x(0*$indent),"assert(H5E_first_min_id_g==H5I_INVALID_HID);\n";
print HEADER " "x(0*$indent),"H5E_first_min_id_g = ${name}_g;\n\n";
$first_minor = 1;
}
$last_name = $name;
}
}
print HEADER " "x(0*$indent),"\n/* Remember last minor error code ID */\n";
print HEADER " "x(0*$indent),"assert(H5E_last_min_id_g==H5I_INVALID_HID);\n";
print HEADER " "x(0*$indent),"H5E_last_min_id_g = ${last_name}_g;\n";

print_endprotect(*HEADER, $file);

Expand Down Expand Up @@ -299,7 +322,9 @@ sub create_term ($) {
foreach $name (keys %major) {
print HEADER " "x($indent),"\n${name}_g=";
}
print HEADER " (-1);\n";
print HEADER " H5I_INVALID_HID;\n";
print HEADER " "x(0*$indent),"H5E_first_maj_id_g = H5I_INVALID_HID;\n\n";
print HEADER " "x(0*$indent),"H5E_last_maj_id_g = H5I_INVALID_HID;\n\n";

# Iterate over all the minor error sections
print HEADER "\n/* Reset minor error IDs */\n";
Expand All @@ -311,7 +336,9 @@ sub create_term ($) {
print HEADER " "x($indent),"\n${name}_g=";
}
}
print HEADER " (-1);\n";
print HEADER " H5I_INVALID_HID;\n";
print HEADER " "x(0*$indent),"H5E_first_min_id_g = H5I_INVALID_HID;\n\n";
print HEADER " "x(0*$indent),"H5E_last_min_id_g = H5I_INVALID_HID;\n\n";

print_endprotect(*HEADER, $file);

Expand Down
83 changes: 25 additions & 58 deletions src/H5E.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ static herr_t H5E__append_stack(H5E_t *dst_estack, const H5E_t *src_stack);
/* Package Variables */
/*********************/

/* First & last major and minor error codes registered by the library */
hid_t H5E_first_maj_id_g = H5I_INVALID_HID;
hid_t H5E_last_maj_id_g = H5I_INVALID_HID;
hid_t H5E_first_min_id_g = H5I_INVALID_HID;
hid_t H5E_last_min_id_g = H5I_INVALID_HID;

/*****************************/
/* Library Private Variables */
/*****************************/
Expand Down Expand Up @@ -435,11 +441,11 @@ H5E__register_class(const char *cls_name, const char *lib_name, const char *vers
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");

/* Duplicate string information */
if (NULL == (cls->cls_name = H5MM_xstrdup(cls_name)))
if (NULL == (cls->cls_name = strdup(cls_name)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");
if (NULL == (cls->lib_name = H5MM_xstrdup(lib_name)))
if (NULL == (cls->lib_name = strdup(lib_name)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");
if (NULL == (cls->lib_vers = H5MM_xstrdup(version)))
if (NULL == (cls->lib_vers = strdup(version)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");

/* Set the return value */
Expand Down Expand Up @@ -738,7 +744,7 @@ H5E__create_msg(H5E_cls_t *cls, H5E_type_t msg_type, const char *msg_str)
/* Fill new message object */
msg->cls = cls;
msg->type = msg_type;
if (NULL == (msg->msg = H5MM_xstrdup(msg_str)))
if (NULL == (msg->msg = strdup(msg_str)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");

/* Set return value */
Expand Down Expand Up @@ -884,24 +890,11 @@ H5E__get_current_stack(void)
current_error = &(current_stack->slot[u]);
new_error = &(estack_copy->slot[u]);

/* Increment the IDs to indicate that they are used in this stack */
if (H5I_inc_ref(current_error->cls_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, NULL, "unable to increment ref count on error class");
new_error->cls_id = current_error->cls_id;
if (H5I_inc_ref(current_error->maj_num, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, NULL, "unable to increment ref count on error message");
new_error->maj_num = current_error->maj_num;
if (H5I_inc_ref(current_error->min_num, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, NULL, "unable to increment ref count on error message");
new_error->min_num = current_error->min_num;
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
new_error->func_name = current_error->func_name;
new_error->file_name = current_error->file_name;
new_error->line = current_error->line;
if (NULL == (new_error->desc = H5MM_xstrdup(current_error->desc)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed");
/* Set error stack entry */
if (H5E__set_stack_entry(new_error, current_error->file_name, current_error->func_name,
current_error->line, current_error->cls_id, current_error->maj_num,
current_error->min_num, current_error->desc) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, NULL, "can't set error entry");
} /* end for */

/* Copy the "automatic" error reporting information */
Expand Down Expand Up @@ -997,24 +990,11 @@ H5E__set_current_stack(H5E_t *estack)
current_error = &(current_stack->slot[u]);
new_error = &(estack->slot[u]);

/* Increment the IDs to indicate that they are used in this stack */
if (H5I_inc_ref(new_error->cls_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "unable to increment ref count on error class");
current_error->cls_id = new_error->cls_id;
if (H5I_inc_ref(new_error->maj_num, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "unable to increment ref count on error class");
current_error->maj_num = new_error->maj_num;
if (H5I_inc_ref(new_error->min_num, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "unable to increment ref count on error class");
current_error->min_num = new_error->min_num;
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
current_error->func_name = new_error->func_name;
current_error->file_name = new_error->file_name;
current_error->line = new_error->line;
if (NULL == (current_error->desc = H5MM_xstrdup(new_error->desc)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed");
/* Set error stack entry */
if (H5E__set_stack_entry(current_error, new_error->file_name, new_error->func_name, new_error->line,
new_error->cls_id, new_error->maj_num, new_error->min_num,
new_error->desc) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't set error entry");
} /* end for */

done:
Expand Down Expand Up @@ -1635,24 +1615,11 @@ H5E__append_stack(H5E_t *dst_stack, const H5E_t *src_stack)
src_error = &(src_stack->slot[u]);
dst_error = &(dst_stack->slot[dst_stack->nused]);

/* Increment the IDs to indicate that they are used in this stack */
if (H5I_inc_ref(src_error->cls_id, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "unable to increment ref count on error class");
dst_error->cls_id = src_error->cls_id;
if (H5I_inc_ref(src_error->maj_num, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "unable to increment ref count on error message");
dst_error->maj_num = src_error->maj_num;
if (H5I_inc_ref(src_error->min_num, false) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTINC, FAIL, "unable to increment ref count on error message");
dst_error->min_num = src_error->min_num;
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
dst_error->func_name = src_error->func_name;
dst_error->file_name = src_error->file_name;
dst_error->line = src_error->line;
if (NULL == (dst_error->desc = H5MM_xstrdup(src_error->desc)))
HGOTO_ERROR(H5E_ERROR, H5E_CANTALLOC, FAIL, "memory allocation failed");
/* Set error stack entry */
if (H5E__set_stack_entry(dst_error, src_error->file_name, src_error->func_name, src_error->line,
src_error->cls_id, src_error->maj_num, src_error->min_num,
src_error->desc) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTSET, FAIL, "can't set error entry");

/* Increment # of errors in destination stack */
dst_stack->nused++;
Expand Down
126 changes: 83 additions & 43 deletions src/H5Eint.c
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,50 @@ H5E__push_stack(H5E_t *estack, const char *file, const char *func, unsigned line
if (NULL == (estack = H5E__get_my_stack()))
HGOTO_DONE(FAIL);

/*
* Push the error if there's room. Otherwise just forget it.
*/
if (estack->nused < H5E_NSLOTS) {
if (H5E__set_stack_entry(&estack->slot[estack->nused], file, func, line, cls_id, maj_id, min_id,
desc) < 0)
HGOTO_DONE(FAIL);
estack->nused++;
} /* end if */

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5E__push_stack() */

/*-------------------------------------------------------------------------
* Function: H5E__set_stack_entry
*
* Purpose: Sets the information for a given stack entry.
*
* Return: SUCCEED/FAIL
*
*-------------------------------------------------------------------------
*/
herr_t
H5E__set_stack_entry(H5E_error2_t *err_entry, const char *file, const char *func, unsigned line, hid_t cls_id,
hid_t maj_id, hid_t min_id, const char *desc)
{
herr_t ret_value = SUCCEED; /* Return value */

/*
* WARNING: We cannot call HERROR() from within this function or else we
* could enter infinite recursion. Furthermore, we also cannot
* call any other HDF5 macro or function which might call
* HERROR(). HERROR() is called by HRETURN_ERROR() which could
* be called by FUNC_ENTER().
*/
FUNC_ENTER_PACKAGE_NOERR

/* Sanity check */
assert(err_entry);
assert(cls_id > 0);
assert(maj_id > 0);
assert(min_id > 0);

/*
* Don't fail if arguments are bad. Instead, substitute some default
* value.
Expand All @@ -731,36 +775,32 @@ H5E__push_stack(H5E_t *estack, const char *file, const char *func, unsigned line
if (!desc)
desc = "No description given";

/*
* Push the error if there's room. Otherwise just forget it.
*/
assert(estack);

if (estack->nused < H5E_NSLOTS) {
/* Increment the IDs to indicate that they are used in this stack */
/* Increment the IDs to indicate that they are used in this stack */
/* Note: don't waste time incrementing library internal error IDs */
if (cls_id != H5E_ERR_CLS_g)
if (H5I_inc_ref_noherr(cls_id, false) < 0)
HGOTO_DONE(FAIL);
estack->slot[estack->nused].cls_id = cls_id;
err_entry->cls_id = cls_id;
if (maj_id < H5E_first_maj_id_g || maj_id > H5E_last_maj_id_g)
if (H5I_inc_ref_noherr(maj_id, false) < 0)
HGOTO_DONE(FAIL);
estack->slot[estack->nused].maj_num = maj_id;
err_entry->maj_num = maj_id;
if (min_id < H5E_first_min_id_g || min_id > H5E_last_min_id_g)
if (H5I_inc_ref_noherr(min_id, false) < 0)
HGOTO_DONE(FAIL);
estack->slot[estack->nused].min_num = min_id;
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
estack->slot[estack->nused].func_name = func;
estack->slot[estack->nused].file_name = file;
estack->slot[estack->nused].line = line;
if (NULL == (estack->slot[estack->nused].desc = H5MM_xstrdup(desc)))
HGOTO_DONE(FAIL);
estack->nused++;
} /* end if */
err_entry->min_num = min_id;
/* The 'func' & 'file' strings are statically allocated (by the compiler)
* there's no need to duplicate them.
*/
err_entry->func_name = func;
err_entry->file_name = file;
err_entry->line = line;
if (NULL == (err_entry->desc = strdup(desc)))
HGOTO_DONE(FAIL);

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5E__push_stack() */
} /* end H5E__set_stack_entry() */

/*-------------------------------------------------------------------------
* Function: H5E__clear_entries
Expand Down Expand Up @@ -791,12 +831,16 @@ H5E__clear_entries(H5E_t *estack, size_t nentries)

/* Decrement the IDs to indicate that they are no longer used by this stack */
/* (In reverse order that they were incremented, so that reference counts work well) */
if (H5I_dec_ref(error->min_num) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error message");
if (H5I_dec_ref(error->maj_num) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error message");
if (H5I_dec_ref(error->cls_id) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error class");
/* Note: don't decrement library internal error IDs, since they weren't incremented */
if (error->min_num < H5E_first_min_id_g || error->min_num > H5E_last_min_id_g)
if (H5I_dec_ref(error->min_num) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error message");
if (error->maj_num < H5E_first_maj_id_g || error->maj_num > H5E_last_maj_id_g)
if (H5I_dec_ref(error->maj_num) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error message");
if (error->cls_id != H5E_ERR_CLS_g)
if (H5I_dec_ref(error->cls_id) < 0)
HGOTO_ERROR(H5E_ERROR, H5E_CANTDEC, FAIL, "unable to decrement ref count on error class");

/* Release strings */
/* The 'func' & 'file' strings are statically allocated (by the compiler)
Expand Down Expand Up @@ -888,32 +932,28 @@ H5E__pop(H5E_t *estack, size_t count)
*-------------------------------------------------------------------------
*/
herr_t
H5E_dump_api_stack(bool is_api)
H5E_dump_api_stack(void)
{
H5E_t *estack = H5E__get_my_stack();
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_NOAPI_NOERR

/* Only dump the error stack during an API call */
if (is_api) {
H5E_t *estack = H5E__get_my_stack();

assert(estack);
assert(estack);

#ifdef H5_NO_DEPRECATED_SYMBOLS
if (estack->auto_op.func2)
(void)((estack->auto_op.func2)(H5E_DEFAULT, estack->auto_data));
#else /* H5_NO_DEPRECATED_SYMBOLS */
if (estack->auto_op.vers == 1) {
if (estack->auto_op.func1)
(void)((estack->auto_op.func1)(estack->auto_data));
} /* end if */
else {
if (estack->auto_op.func2)
(void)((estack->auto_op.func2)(H5E_DEFAULT, estack->auto_data));
#else /* H5_NO_DEPRECATED_SYMBOLS */
if (estack->auto_op.vers == 1) {
if (estack->auto_op.func1)
(void)((estack->auto_op.func1)(estack->auto_data));
} /* end if */
else {
if (estack->auto_op.func2)
(void)((estack->auto_op.func2)(H5E_DEFAULT, estack->auto_data));
} /* end else */
} /* end else */
#endif /* H5_NO_DEPRECATED_SYMBOLS */
} /* end if */

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5E_dump_api_stack() */
Loading

0 comments on commit 603f8b1

Please sign in to comment.