Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix H5Otoken_to_str call in h5dump and other minor cleanup #3314

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Fix H5Otoken_to_str call in h5dump and other minor cleanup #3314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/H5Dint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3540,11 +3540,11 @@ H5D_flush_all(H5F_t *f)
hid_t
H5D_get_create_plist(const H5D_t *dset)
{
H5P_genplist_t *dcpl_plist; /* Dataset's DCPL */
H5P_genplist_t *new_plist; /* Copy of dataset's DCPL */
H5O_layout_t copied_layout; /* Layout to tweak */
H5O_fill_t copied_fill; /* Fill value to tweak */
H5O_efl_t copied_efl; /* External file list to tweak */
H5P_genplist_t *dcpl_plist; /* Dataset's DCPL */
H5P_genplist_t *new_plist; /* Copy of dataset's DCPL */
H5O_layout_t copied_layout; /* Layout to tweak */
H5O_fill_t copied_fill = {0}; /* Fill value to tweak */
H5O_efl_t copied_efl; /* External file list to tweak */
hid_t new_dcpl_id = FAIL;
hid_t ret_value = H5I_INVALID_HID; /* Return value */

Expand Down Expand Up @@ -3697,11 +3697,15 @@ H5D_get_create_plist(const H5D_t *dset)
ret_value = new_dcpl_id;

done:
if (ret_value < 0)
if (ret_value < 0) {
if (new_dcpl_id > 0)
if (H5I_dec_app_ref(new_dcpl_id) < 0)
HDONE_ERROR(H5E_DATASET, H5E_CANTDEC, FAIL, "unable to close temporary object");

if (copied_fill.type && (H5T_close_real(copied_fill.type) < 0))
HDONE_ERROR(H5E_DATASET, H5E_CANTFREE, FAIL, "Can't free temporary datatype");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this error report redundant, since the call has already failed at this point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One could make the argument that this pattern of reporting errors after the done: section in the library when a function has already failed is redundant, but sometimes the error messages can be useful and at least it's uniform so developers don't need to remember special rules about when to report an error or not.

}

FUNC_LEAVE_NOAPI(ret_value)
} /* end H5D_get_create_plist() */

Expand Down
73 changes: 50 additions & 23 deletions tools/lib/h5tools_dump.c
View file
Open in desktop
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in this file should be refactored so it's easy to skip certain things when API calls fail while still printing as much as possible, but for now the changes in this file work around H5Dget_create_plist failed and also address a memory leak and three assertion failures.

Original file line number Diff line number Diff line change
Expand Up @@ -3067,11 +3067,15 @@ h5tools_print_fill_value(h5tools_str_t *buffer /*in,out*/, const h5tool_format_t
h5tools_context_t *ctx /*in,out*/, hid_t dcpl, hid_t type_id, hid_t obj_id)
{
size_t size;
hid_t n_type = H5I_INVALID_HID;
void *buf = NULL;
hid_t n_type = H5I_INVALID_HID;
void *buf = NULL;
bool vl_data = false;

n_type = H5Tget_native_type(type_id, H5T_DIR_DEFAULT);

if (h5tools_detect_vlen(type_id) == TRUE)
vl_data = true;

size = H5Tget_size(n_type);
buf = malloc(size);

Expand All @@ -3081,6 +3085,17 @@ h5tools_print_fill_value(h5tools_str_t *buffer /*in,out*/, const h5tool_format_t

H5Tclose(n_type);

if (vl_data) {
hsize_t dims[1] = {1};
hid_t space_id = H5I_INVALID_HID;

space_id = H5Screate_simple(1, dims, NULL);

H5Treclaim(type_id, space_id, H5P_DEFAULT, buf);

H5Sclose(space_id);
}

if (buf)
free(buf);
}
Expand All @@ -3097,26 +3112,26 @@ void
h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *ctx, hid_t dcpl_id,
hid_t type_id, hid_t dset_id)
{
int nfilters; /* number of filters */
int rank; /* rank */
int nfilters = -1; /* number of filters */
int rank; /* rank */
int i;
unsigned j;
unsigned filt_flags; /* filter flags */
unsigned cd_values[20]; /* filter client data values */
unsigned szip_options_mask;
unsigned szip_pixels_per_block;
H5Z_filter_t filtn; /* filter identification number */
H5D_fill_value_t fvstatus;
H5D_alloc_time_t at;
H5D_fill_time_t ft;
H5D_layout_t stl;
size_t ncols = 80; /* available output width */
size_t cd_nelmts; /* filter client number of values */
off_t offset; /* offset of external file */
char f_name[256]; /* filter name */
char name[256]; /* external or virtual file name */
hsize_t chsize[64]; /* chunk size in elements */
hsize_t size; /* size of external file */
H5D_fill_value_t fvstatus = H5D_FILL_VALUE_ERROR;
H5D_alloc_time_t at = H5D_ALLOC_TIME_ERROR;
H5D_fill_time_t ft = H5D_FILL_TIME_ERROR;
H5D_layout_t stl = H5D_LAYOUT_ERROR;
size_t ncols = 80; /* available output width */
size_t cd_nelmts; /* filter client number of values */
off_t offset; /* offset of external file */
char f_name[256]; /* filter name */
char name[256]; /* external or virtual file name */
hsize_t chsize[64]; /* chunk size in elements */
hsize_t size; /* size of external file */
hsize_t storage_size;
hsize_t curr_pos = 0; /* total data element position */
h5tools_str_t buffer; /* string into which to render */
Expand All @@ -3127,7 +3142,9 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
ncols = info->line_ncols;

storage_size = H5Dget_storage_size(dset_id);
nfilters = H5Pget_nfilters(dcpl_id);
if (dcpl_id >= 0)
nfilters = H5Pget_nfilters(dcpl_id);

HDstrcpy(f_name, "\0");

/*-------------------------------------------------------------------------
Expand All @@ -3140,7 +3157,9 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
h5tools_str_append(&buffer, "%s %s", STORAGE_LAYOUT, BEGIN);
h5tools_render_element(stream, info, ctx, &buffer, &curr_pos, (size_t)ncols, (hsize_t)0, (hsize_t)0);

stl = H5Pget_layout(dcpl_id);
if (dcpl_id >= 0)
stl = H5Pget_layout(dcpl_id);

switch (stl) {
case H5D_CHUNKED:
ctx->indent_level++;
Expand Down Expand Up @@ -3639,7 +3658,9 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
h5tools_str_reset(&buffer);
h5tools_str_append(&buffer, "FILL_TIME ");

H5Pget_fill_time(dcpl_id, &ft);
if (dcpl_id >= 0)
H5Pget_fill_time(dcpl_id, &ft);

switch (ft) {
case H5D_FILL_TIME_ALLOC:
h5tools_str_append(&buffer, "%s", "H5D_FILL_TIME_ALLOC");
Expand All @@ -3652,7 +3673,7 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
break;
case H5D_FILL_TIME_ERROR:
default:
assert(0);
h5tools_str_append(&buffer, "%s", "INVALID");
break;
}
h5tools_render_element(stream, info, ctx, &buffer, &curr_pos, (size_t)ncols, (hsize_t)0, (hsize_t)0);
Expand All @@ -3661,7 +3682,10 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *

h5tools_str_reset(&buffer);
h5tools_str_append(&buffer, "%s ", "VALUE ");
H5Pfill_value_defined(dcpl_id, &fvstatus);

if (dcpl_id >= 0)
H5Pfill_value_defined(dcpl_id, &fvstatus);

switch (fvstatus) {
case H5D_FILL_VALUE_UNDEFINED:
h5tools_str_append(&buffer, "%s", "H5D_FILL_VALUE_UNDEFINED");
Expand All @@ -3676,7 +3700,7 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
break;
case H5D_FILL_VALUE_ERROR:
default:
assert(0);
h5tools_str_append(&buffer, "%s", "INVALID");
break;
}
h5tools_render_element(stream, info, ctx, &buffer, &curr_pos, (size_t)ncols, (hsize_t)0, (hsize_t)0);
Expand Down Expand Up @@ -3704,7 +3728,10 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
ctx->need_prefix = TRUE;

h5tools_str_reset(&buffer);
H5Pget_alloc_time(dcpl_id, &at);

if (dcpl_id >= 0)
H5Pget_alloc_time(dcpl_id, &at);

switch (at) {
case H5D_ALLOC_TIME_EARLY:
h5tools_str_append(&buffer, "%s", "H5D_ALLOC_TIME_EARLY");
Expand All @@ -3718,7 +3745,7 @@ h5tools_dump_dcpl(FILE *stream, const h5tool_format_t *info, h5tools_context_t *
case H5D_ALLOC_TIME_ERROR:
case H5D_ALLOC_TIME_DEFAULT:
default:
assert(0);
h5tools_str_append(&buffer, "%s", "INVALID");
break;
}
h5tools_render_element(stream, info, ctx, &buffer, &curr_pos, (size_t)ncols, (hsize_t)0, (hsize_t)0);
Expand Down
3 changes: 2 additions & 1 deletion tools/lib/h5tools_str.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1228,11 +1228,12 @@ h5tools_str_sprint(h5tools_str_t *str, const h5tool_format_t *info, hid_t contai
h5tools_str_append(str, "%u-", (unsigned)oi.type);
break;
}
H5Oclose(obj);

/* Print OID */
H5Otoken_to_str(obj, &oi.token, &obj_tok_str);

H5Oclose(obj);

if (info->obj_hidefileno)
h5tools_str_append(str, info->obj_format, obj_tok_str);
else
Expand Down
16 changes: 10 additions & 6 deletions tools/src/h5dump/h5dump_ddl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,7 @@ dump_dataset(hid_t did, const char *name, struct subset_t *sset)
h5tool_format_t *outputformat = &h5tools_dataformat;
h5tool_format_t string_dataformat;
hid_t type, space;
unsigned attr_crt_order_flags;
unsigned attr_crt_order_flags = 0;
hid_t dcpl_id; /* dataset creation property list ID */
h5tools_str_t buffer; /* string into which to render */
hsize_t curr_pos = 0; /* total data element position */
Expand All @@ -947,14 +947,16 @@ dump_dataset(hid_t did, const char *name, struct subset_t *sset)
outputformat = &string_dataformat;

if ((dcpl_id = H5Dget_create_plist(did)) < 0) {
error_msg("error in getting creation property list ID\n");
error_msg("error in getting creation property list ID for dataset '%s'\n", name);
h5tools_setstatus(EXIT_FAILURE);
}

/* query the creation properties for attributes */
if (H5Pget_attr_creation_order(dcpl_id, &attr_crt_order_flags) < 0) {
error_msg("error in getting creation properties\n");
h5tools_setstatus(EXIT_FAILURE);
if (dcpl_id >= 0) {
if (H5Pget_attr_creation_order(dcpl_id, &attr_crt_order_flags) < 0) {
error_msg("error in getting creation properties for dataset '%s'\n", name);
h5tools_setstatus(EXIT_FAILURE);
}
}

/* setup */
Expand Down Expand Up @@ -993,7 +995,9 @@ dump_dataset(hid_t did, const char *name, struct subset_t *sset)
h5tools_dump_dcpl(rawoutstream, outputformat, &ctx, dcpl_id, type, did);
h5dump_type_table = NULL;
}
H5Pclose(dcpl_id);

if (dcpl_id >= 0)
H5Pclose(dcpl_id);

ctx.sset = sset;
ctx.display_index = dump_opts.display_ai;
Expand Down