Skip to content

Commit

Permalink
Hdf5_1_14_5 updates (#4892)
Browse files Browse the repository at this point in the history
* Fix issues with large external data files (#4843) (#4847)

* Fixed a memory leak from H5FL_blk_malloc (#4882)

In H5F__accum_reset(), when H5F__accum_flush() failed, the freeing of
f_sh->accum.buf was never reached, causing resource leak.

@fortnern added the third argument to H5F__accum_reset() so we can free
f_sh->accum.buf when we close the file, that is, when H5F__accum_reset()
is called from the H5F__dest() route, and can leave the accumulator in place
otherwise.

* Added an entry for the GH-4585 fix (#4889)

* Fix an incorrect returned value by H5LTfind_dataset() (#4869)

H5LTfind_dataset() returns true for non-existing datasets because it only compares up to the length of the searched string, such as "Day" vs "DayNight" (issue GH-4780).

This PR applied the user's patch and added tests.

* Fix minor spelling in documentation (#4870)

---------

Co-authored-by: Neil Fortner <[email protected]>
Co-authored-by: bmribler <[email protected]>
  • Loading branch information
3 people authored Sep 28, 2024
1 parent 306df30 commit cb13cf2
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 19 deletions.
2 changes: 1 addition & 1 deletion hl/src/H5LT.c
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ find_dataset(H5_ATTR_UNUSED hid_t loc_id, const char *name, H5_ATTR_UNUSED const
* cause the iterator to immediately return that positive value,
* indicating short-circuit success
*/
if (strncmp(name, (char *)op_data, strlen((char *)op_data)) == 0)
if (strcmp(name, (char *)op_data) == 0)
ret = 1;

return ret;
Expand Down
17 changes: 13 additions & 4 deletions hl/test/test_ds.c
Original file line number Diff line number Diff line change
Expand Up @@ -1341,10 +1341,11 @@ test_detachscales(void)
static int
test_char_attachscales(const char *fileext)
{
hid_t fid = -1;
hid_t did = -1;
char dsname[32];
char scalename[32];
hid_t fid = -1;
hid_t did = -1;
char dsname[32];
char scalename[32];
herr_t ds_existed = 0;

snprintf(dsname, sizeof(dsname), "%s%s", DATASET_NAME, "ac");

Expand All @@ -1357,6 +1358,14 @@ test_char_attachscales(const char *fileext)
if (create_char_dataset(fid, "ac", 0) < 0)
goto out;

/* test finding dataset dsname */
if ((ds_existed = H5LTfind_dataset(fid, dsname)) < 0)
goto out;
if (ds_existed == 0) {
printf("Unexpected result: Dataset \"%s\" does exist\n", dsname);
goto out;
}

if ((did = H5Dopen2(fid, dsname, H5P_DEFAULT)) >= 0) {
snprintf(scalename, sizeof(scalename), "%s%s", DS_1_NAME, "ac");
if (test_attach_scale(fid, did, scalename, DIM0) < 0)
Expand Down
24 changes: 23 additions & 1 deletion hl/test/test_lite.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
#define DSET6_NAME "dataset double"
#define DSET7_NAME "dataset string"

/* Name of a non-existing dataset, do not create a dataset with this name */
#define NODS_NAME "dataset"

#define DIM 6

#define ATTR_NAME_SUB "att"
Expand Down Expand Up @@ -60,6 +63,7 @@ test_dsets(void)
hsize_t dims[2] = {2, 3};
hid_t file_id;
hid_t dataset_id;
herr_t ds_existed = 0; /* whether searched ds exists */
char data_char_in[DIM] = {1, 2, 3, 4, 5, 6};
char data_char_out[DIM];
short data_short_in[DIM] = {1, 2, 3, 4, 5, 6};
Expand Down Expand Up @@ -348,6 +352,23 @@ test_dsets(void)
if (strcmp(data_string_in, data_string_out) != 0)
goto out;

PASSED();

/*-------------------------------------------------------------------------
* H5LTfind_dataset test
*-------------------------------------------------------------------------
*/

HL_TESTING2("H5LTfind_dataset");

/* Try to find a non-existing ds whose name matches existing datasets partially */
if ((ds_existed = H5LTfind_dataset(file_id, NODS_NAME)) < 0)
goto out;
if (ds_existed > 0) {
printf("Dataset \"%s\" does not exist.\n", NODS_NAME);
goto out;
}

/*-------------------------------------------------------------------------
* end tests
*-------------------------------------------------------------------------
Expand Down Expand Up @@ -1075,7 +1096,7 @@ test_integers(void)
char *dt_str;
size_t str_len;

HL_TESTING3("\n text for integer types");
HL_TESTING3(" text for integer types");

if ((dtype = H5LTtext_to_dtype("H5T_NATIVE_INT\n", H5LT_DDL)) < 0)
goto out;
Expand Down Expand Up @@ -1881,6 +1902,7 @@ test_text_dtype(void)
{
HL_TESTING2("H5LTtext_to_dtype");

printf("\n");
if (test_integers() < 0)
goto out;

Expand Down
69 changes: 69 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,63 @@ Bug Fixes since HDF5-1.14.4 release
===================================
Library
-------
- Fixed a memory leak in H5F__accum_write()

The memory was allocated in H5F__accum_write() and was to be freed in
H5F__accum_reset() during the closing process but a failure occurred just
before the deallocation, leaving the memory un-freed. The problem is
now fixed.

Fixes GitHub #4585

- Fixed an incorrect returned value by H5LTfind_dataset()

H5LTfind_dataset() returned true for non-existing datasets because it only
compared up to the length of the searched string, such as "Day" vs "DayNight".
Applied the user's patch to correct this behavior.

Fixes GitHub #4780

- Fixed a segfault by H5Gmove2, extending to H5Lcopy and H5Lmove

A user's application segfaulted when it passed in an invalid location ID
to H5Gmove2. The src and dst location IDs must be either a file or a group
ID. The fix was also applied to H5Lcopy and H5Lmove. Now, all these
three functions will fail if either the src or dst location ID is not a file
or a group ID.

Fixes GitHub #4737

- Fixed a segfault by H5Lget_info()

A user's program generated a segfault when the ID passed into H5Lget_info()
was a datatype ID. This was caused by non-VOL functions being used internally
where VOL functions should have been. This correction was extended to many
other functions to prevent potential issue in the future.

Fixes GitHub #4730

- Fixed a segfault by H5Fget_intent(), extending to several other functions

A user's program generated a segfault when the ID passed into H5Fget_intent()
was not a file ID. In addition to H5Fget_intent(), a number of APIs also failed
to detect an incorrect ID being passed in, which can potentially cause various
failures, including segfault. The affected functions are listed below and now
properly detect incorrect ID parameters:

H5Fget_intent()
H5Fget_fileno()
H5Fget_freespace()
H5Fget_create_plist()
H5Fget_access_plist()
H5Fget_vfd_handle()
H5Dvlen_get_buf_size()
H5Fget_mdc_config()
H5Fset_mdc_config()
H5Freset_mdc_hit_rate_stats()

Fixes GitHub #4656 and GitHub #4662

- Fixed a bug with large external datasets

When performing a large I/O on an external dataset, the library would only
Expand Down Expand Up @@ -228,6 +285,18 @@ Bug Fixes since HDF5-1.14.4 release
APIs in HDF5 will be modified/written in this manner, regarding the
length of a character string.

Fixes GitHub #4447

- Fixed heap-buffer-overflow in h5dump

h5dump aborted when provided with a malformed input file. The was because
the buffer size for checksum was smaller than H5_SIZEOF_CHKSUM, causing
an overflow while calculating the offset to the checksum in the buffer.
A check was added so H5F_get_checksums would fail appropriately in all
of its occurrences.

Fixes GitHub #4434

- Fixed library to allow usage of page buffering feature for serial file
access with parallel builds of HDF5

Expand Down
13 changes: 8 additions & 5 deletions src/H5Faccum.c
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ H5F__accum_write(H5F_shared_t *f_sh, H5FD_mem_t map_type, haddr_t addr, size_t s
/* Make certain that data in accumulator is visible before new write */
if ((H5F_SHARED_INTENT(f_sh) & H5F_ACC_SWMR_WRITE) > 0)
/* Flush if dirty and reset accumulator */
if (H5F__accum_reset(f_sh, true) < 0)
if (H5F__accum_reset(f_sh, true, false) < 0)
HGOTO_ERROR(H5E_IO, H5E_CANTRESET, FAIL, "can't reset accumulator");

/* Write the data */
Expand Down Expand Up @@ -776,7 +776,7 @@ H5F__accum_write(H5F_shared_t *f_sh, H5FD_mem_t map_type, haddr_t addr, size_t s
} /* end if */
else { /* Access covers whole accumulator */
/* Reset accumulator, but don't flush */
if (H5F__accum_reset(f_sh, false) < 0)
if (H5F__accum_reset(f_sh, false, false) < 0)
HGOTO_ERROR(H5E_IO, H5E_CANTRESET, FAIL, "can't reset accumulator");
} /* end else */
} /* end if */
Expand Down Expand Up @@ -1039,7 +1039,7 @@ H5F__accum_flush(H5F_shared_t *f_sh)
*-------------------------------------------------------------------------
*/
herr_t
H5F__accum_reset(H5F_shared_t *f_sh, bool flush)
H5F__accum_reset(H5F_shared_t *f_sh, bool flush, bool force)
{
herr_t ret_value = SUCCEED; /* Return value */

Expand All @@ -1050,8 +1050,11 @@ H5F__accum_reset(H5F_shared_t *f_sh, bool flush)

/* Flush any dirty data in accumulator, if requested */
if (flush)
if (H5F__accum_flush(f_sh) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTFLUSH, FAIL, "can't flush metadata accumulator");
if (H5F__accum_flush(f_sh) < 0) {
HDONE_ERROR(H5E_FILE, H5E_CANTFLUSH, FAIL, "can't flush metadata accumulator");
if (!force)
HGOTO_DONE(FAIL);
}

/* Check if we need to reset the metadata accumulator information */
if (f_sh->feature_flags & H5FD_FEAT_ACCUMULATE_METADATA) {
Expand Down
4 changes: 2 additions & 2 deletions src/H5Fint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ H5F__dest(H5F_t *f, bool flush, bool free_on_failure)
} /* end if */

/* Destroy other components of the file */
if (H5F__accum_reset(f->shared, true) < 0)
if (H5F__accum_reset(f->shared, true, true) < 0)
/* Push error, but keep going*/
HDONE_ERROR(H5E_FILE, H5E_CANTRELEASE, FAIL, "problems closing file");
if (H5FO_dest(f) < 0)
Expand Down Expand Up @@ -3893,7 +3893,7 @@ H5F__start_swmr_write(H5F_t *f)
} /* end if */

/* Flush and reset the accumulator */
if (H5F__accum_reset(f->shared, true) < 0)
if (H5F__accum_reset(f->shared, true, false) < 0)
HGOTO_ERROR(H5E_IO, H5E_CANTRESET, FAIL, "can't reset accumulator");

/* Turn on SWMR write in shared file open flags */
Expand Down
2 changes: 1 addition & 1 deletion src/H5Fio.c
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ H5F_flush_tagged_metadata(H5F_t *f, haddr_t tag)
HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush tagged metadata");

/* Flush and reset the accumulator */
if (H5F__accum_reset(f->shared, true) < 0)
if (H5F__accum_reset(f->shared, true, false) < 0)
HGOTO_ERROR(H5E_IO, H5E_CANTRESET, FAIL, "can't reset accumulator");

/* Flush file buffers to disk. */
Expand Down
2 changes: 1 addition & 1 deletion src/H5Fpkg.h
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,7 @@ H5_DLL herr_t H5F__accum_write(H5F_shared_t *f_sh, H5FD_mem_t type, haddr_t addr
const void *buf);
H5_DLL herr_t H5F__accum_free(H5F_shared_t *f, H5FD_mem_t type, haddr_t addr, hsize_t size);
H5_DLL herr_t H5F__accum_flush(H5F_shared_t *f_sh);
H5_DLL herr_t H5F__accum_reset(H5F_shared_t *f_sh, bool flush);
H5_DLL herr_t H5F__accum_reset(H5F_shared_t *f_sh, bool flush, bool force);

/* Shared file list related routines */
H5_DLL herr_t H5F__sfile_add(H5F_shared_t *shared);
Expand Down
6 changes: 3 additions & 3 deletions src/H5Ppublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -2499,8 +2499,8 @@ H5_DLL herr_t H5Pset_deflate(hid_t plist_id, unsigned level);
* pipeline
* \param[in] flags Bit vector specifying certain general properties of
* the filter
* \param[in] cd_nelmts Number of elements in \p c_values
* \param[in] c_values Auxiliary data for the filter
* \param[in] cd_nelmts Number of elements in \p cd_values
* \param[in] cd_values Auxiliary data for the filter
*
* \return \herr_t
*
Expand Down Expand Up @@ -2756,7 +2756,7 @@ H5_DLL herr_t H5Pset_deflate(hid_t plist_id, unsigned level);
*
*/
H5_DLL herr_t H5Pset_filter(hid_t plist_id, H5Z_filter_t filter, unsigned int flags, size_t cd_nelmts,
const unsigned int c_values[]);
const unsigned int cd_values[]);
/**
* \ingroup OCPL
*
Expand Down
2 changes: 1 addition & 1 deletion test/accum.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ void accum_printf(const H5F_t *f);
#define accum_read(a, s, b) H5F_block_read(f, H5FD_MEM_DEFAULT, (haddr_t)(a), (size_t)(s), (b))
#define accum_free(f, a, s) H5F__accum_free(f->shared, H5FD_MEM_DEFAULT, (haddr_t)(a), (hsize_t)(s))
#define accum_flush(f) H5F__accum_flush(f->shared)
#define accum_reset(f) H5F__accum_reset(f->shared, true)
#define accum_reset(f) H5F__accum_reset(f->shared, true, false)

/* ================= */
/* Main Test Routine */
Expand Down

0 comments on commit cb13cf2

Please sign in to comment.