From 8ef713ae5dc454db66ea821635f6d6d6970858f0 Mon Sep 17 00:00:00 2001 From: vchoi-hdfgroup <55293060+vchoi-hdfgroup@users.noreply.github.com> Date: Mon, 12 Dec 2022 10:40:30 -0600 Subject: =?UTF-8?q?Fix=20for=20HDFFV-10840:=20Instead=20of=20using=20fill-?= =?UTF-8?q?>buf=20for=20datatype=20conversi=E2=80=A6=20(#2277)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix for HDFFV-10840: Instead of using fill->buf for datatype conversion (#2153) * Fix for HDFFV-10840: Instead of using fill->buf for datatype conversion if it is large enough, a buffer is allocated regardless so that the element in fill->buf can later be reclaimed. Valgrind is run on test/set_extent.c and there is no memory leak. * Add information of this fix to release notes. * Change macos version for CI to macos-11 until accum test failure is fixed for macos 12. Co-authored-by: Larry Knox --- release_docs/RELEASE.txt | 13 +++++++++++++ src/H5Ofill.c | 25 +++++++++++-------------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 0b6ca4e..022010c 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -227,6 +227,19 @@ Bug Fixes since HDF5-1.12.1 release (JTH - 2022/07/08, HDFFV-11316, HDFFV-11317) + - Memory leak + + A memory leak was observed with variable-length fill value in + H5O_fill_convert() function in H5Ofill.c. The leak is + manifested by running valgrind on test/set_extent.c. + + Previously, fill->buf is used for datatype conversion + if it is large enough and the variable-length information + is therefore lost. A buffer is now allocated regardless + so that the element in fill->buf can later be reclaimed. + + (VC - 2022/10/10, HDFFV-10840) + Java Library ------------ diff --git a/src/H5Ofill.c b/src/H5Ofill.c index 4106056..2197c09 100644 --- a/src/H5Ofill.c +++ b/src/H5Ofill.c @@ -1006,6 +1006,8 @@ H5O_fill_convert(H5O_fill_t *fill, H5T_t *dset_type, hbool_t *fill_changed) /* Don't bother doing anything if there will be no actual conversion */ if (!H5T_path_noop(tpath)) { + size_t fill_type_size; + if ((src_id = H5I_register(H5I_DATATYPE, H5T_copy(fill->type, H5T_COPY_ALL), FALSE)) < 0 || (dst_id = H5I_register(H5I_DATATYPE, H5T_copy(dset_type, H5T_COPY_ALL), FALSE)) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, FAIL, "unable to copy/register data type") @@ -1014,13 +1016,11 @@ H5O_fill_convert(H5O_fill_t *fill, H5T_t *dset_type, hbool_t *fill_changed) * Datatype conversions are always done in place, so we need a buffer * that is large enough for both source and destination. */ - if (H5T_get_size(fill->type) >= H5T_get_size(dset_type)) - buf = fill->buf; - else { - if (NULL == (buf = H5MM_malloc(H5T_get_size(dset_type)))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for type conversion") - H5MM_memcpy(buf, fill->buf, H5T_get_size(fill->type)); - } /* end else */ + fill_type_size = H5T_get_size(fill->type); + + if (NULL == (buf = H5MM_malloc(MAX(fill_type_size, H5T_get_size(dset_type))))) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for type conversion") + H5MM_memcpy(buf, fill->buf, fill_type_size); /* Use CALLOC here to clear the buffer in case later the library thinks there's * data in the background. */ @@ -1032,11 +1032,10 @@ H5O_fill_convert(H5O_fill_t *fill, H5T_t *dset_type, hbool_t *fill_changed) HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, FAIL, "datatype conversion failed") /* Update the fill message */ - if (buf != fill->buf) { - H5T_vlen_reclaim_elmt(fill->buf, fill->type); - H5MM_xfree(fill->buf); - fill->buf = buf; - } /* end if */ + H5T_vlen_reclaim_elmt(fill->buf, fill->type); + H5MM_xfree(fill->buf); + fill->buf = buf; + (void)H5T_close_real(fill->type); fill->type = NULL; H5_CHECKED_ASSIGN(fill->size, ssize_t, H5T_get_size(dset_type), size_t); @@ -1050,8 +1049,6 @@ done: HDONE_ERROR(H5E_OHDR, H5E_CANTDEC, FAIL, "unable to decrement ref count for temp ID") if (dst_id >= 0 && H5I_dec_ref(dst_id) < 0) HDONE_ERROR(H5E_OHDR, H5E_CANTDEC, FAIL, "unable to decrement ref count for temp ID") - if (buf != fill->buf) - H5MM_xfree(buf); if (bkg) H5MM_xfree(bkg); -- cgit v0.12