Fix for HDFFV-10840: Instead of using fill->buf for datatype conversion (#2153) (#2278)

* 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.

2 files changed, 24 insertions, 14 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index e2a36a4..f605ffe 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -220,6 +220,19 @@ Bug Fixes since HDF5-1.10.9 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 b86a1da..6ba7a11 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);