Fix bug in attribute type conversion wiith compound types (#2016)

* Fix bug in attribute type conversion where the background buffer would
not be initialized with the destination contents when necessary.  Other
minor simplification.

* Committing clang-format changes

* Fix warnings.

* Address review comments.

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

1 files changed, 44 insertions, 9 deletions

diff --git a/src/H5Aint.c b/src/H5Aint.c
index fb54f0c..4403da8 100644
--- a/src/H5Aint.c
+++ b/src/H5Aint.c
@@ -740,6 +740,8 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
 
             /* Check for type conversion required */
             if (!H5T_path_noop(tpath)) {
+                H5T_bkg_t need_bkg; /* Background buffer type */
+
                 if ((src_id = H5I_register(H5I_DATATYPE, H5T_copy(attr->shared->dt, H5T_COPY_ALL), FALSE)) <
                         0 ||
                     (dst_id = H5I_register(H5I_DATATYPE, H5T_copy(mem_type, H5T_COPY_ALL), FALSE)) < 0)
@@ -749,12 +751,23 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
                 buf_size = nelmts * MAX(src_type_size, dst_type_size);
                 if (NULL == (tconv_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
                     HGOTO_ERROR(H5E_ATTR, H5E_NOSPACE, FAIL, "memory allocation failed")
-                if (NULL == (bkg_buf = H5FL_BLK_CALLOC(attr_buf, buf_size)))
-                    HGOTO_ERROR(H5E_ATTR, H5E_NOSPACE, FAIL, "memory allocation failed")
 
                 /* Copy the attribute data into the buffer for conversion */
                 H5MM_memcpy(tconv_buf, attr->shared->data, (src_type_size * nelmts));
 
+                /* Check if we need a background buffer */
+                need_bkg = H5T_path_bkg(tpath);
+
+                if (need_bkg) {
+                    /* Allocate background buffer */
+                    if (NULL == (bkg_buf = H5FL_BLK_CALLOC(attr_buf, buf_size)))
+                        HGOTO_ERROR(H5E_ATTR, H5E_NOSPACE, FAIL, "memory allocation failed")
+
+                    /* Copy the application buffer into the background buffer if necessary */
+                    if (need_bkg == H5T_BKG_YES)
+                        H5MM_memcpy(bkg_buf, buf, (dst_type_size * nelmts));
+                }
+
                 /* Perform datatype conversion.  */
                 if (H5T_convert(tpath, src_id, dst_id, nelmts, (size_t)0, (size_t)0, tconv_buf, bkg_buf) < 0)
                     HGOTO_ERROR(H5E_ATTR, H5E_CANTENCODE, FAIL, "datatype conversion failed")
@@ -804,9 +817,8 @@ done:
 herr_t
 H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
 {
-    uint8_t    *tconv_buf   = NULL;       /* datatype conv buffer */
-    hbool_t     tconv_owned = FALSE;      /* Whether the datatype conv buffer is owned by attribute */
-    uint8_t    *bkg_buf     = NULL;       /* temp conversion buffer */
+    uint8_t    *tconv_buf = NULL;         /* datatype conv buffer */
+    uint8_t    *bkg_buf   = NULL;         /* temp conversion buffer */
     hssize_t    snelmts;                  /* elements in attribute */
     size_t      nelmts;                   /* elements in attribute */
     H5T_path_t *tpath  = NULL;            /* conversion information*/
@@ -840,6 +852,8 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
 
         /* Check for type conversion required */
         if (!H5T_path_noop(tpath)) {
+            H5T_bkg_t need_bkg; /* Background buffer type */
+
             if ((src_id = H5I_register(H5I_DATATYPE, H5T_copy(mem_type, H5T_COPY_ALL), FALSE)) < 0 ||
                 (dst_id = H5I_register(H5I_DATATYPE, H5T_copy(attr->shared->dt, H5T_COPY_ALL), FALSE)) < 0)
                 HGOTO_ERROR(H5E_ATTR, H5E_CANTREGISTER, FAIL, "unable to register types for conversion")
@@ -848,12 +862,33 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
             buf_size = nelmts * MAX(src_type_size, dst_type_size);
             if (NULL == (tconv_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
                 HGOTO_ERROR(H5E_ATTR, H5E_CANTALLOC, FAIL, "memory allocation failed")
-            if (NULL == (bkg_buf = H5FL_BLK_CALLOC(attr_buf, buf_size)))
-                HGOTO_ERROR(H5E_ATTR, H5E_CANTALLOC, FAIL, "memory allocation failed")
 
             /* Copy the user's data into the buffer for conversion */
             H5MM_memcpy(tconv_buf, buf, (src_type_size * nelmts));
 
+            /* Check if we need a background buffer */
+            if (H5T_detect_class(attr->shared->dt, H5T_VLEN, FALSE))
+                need_bkg = H5T_BKG_YES;
+            else
+                need_bkg = H5T_path_bkg(tpath);
+
+            if (need_bkg) {
+                /* Use the existing attribute data buffer, if present, as the background buffer,
+                 * otherwise allocate one.  Note we don't need to track which one it is since both
+                 * use the "attr_buf" free list block. */
+                if (attr->shared->data) {
+                    bkg_buf            = attr->shared->data;
+                    attr->shared->data = NULL;
+
+                    /* Clear background buffer if it's not supposed to be initialized with file
+                     * contents */
+                    if (need_bkg == H5T_BKG_TEMP)
+                        HDmemset(bkg_buf, 0, dst_type_size * nelmts);
+                }
+                else if (NULL == (bkg_buf = H5FL_BLK_CALLOC(attr_buf, buf_size)))
+                    HGOTO_ERROR(H5E_ATTR, H5E_CANTALLOC, FAIL, "memory allocation failed")
+            }
+
             /* Perform datatype conversion */
             if (H5T_convert(tpath, src_id, dst_id, nelmts, (size_t)0, (size_t)0, tconv_buf, bkg_buf) < 0)
                 HGOTO_ERROR(H5E_ATTR, H5E_CANTENCODE, FAIL, "datatype conversion failed")
@@ -864,7 +899,7 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
 
             /* Set the pointer to the attribute data to the converted information */
             attr->shared->data = tconv_buf;
-            tconv_owned        = TRUE;
+            tconv_buf          = NULL;
         } /* end if */
         /* No type conversion necessary */
         else {
@@ -890,7 +925,7 @@ done:
         HDONE_ERROR(H5E_ATTR, H5E_CANTDEC, FAIL, "unable to close temporary object")
     if (dst_id >= 0 && H5I_dec_ref(dst_id) < 0)
         HDONE_ERROR(H5E_ATTR, H5E_CANTDEC, FAIL, "unable to close temporary object")
-    if (tconv_buf && !tconv_owned)
+    if (tconv_buf)
         tconv_buf = H5FL_BLK_FREE(attr_buf, tconv_buf);
     if (bkg_buf)
         bkg_buf = H5FL_BLK_FREE(attr_buf, bkg_buf);