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>
This commit is contained in:
Neil Fortner
2022-08-16 14:04:48 -07:00
committed by GitHub
co-authored by github-actions
parent 10e4dd4ef1
commit 75f4d9b042
2 changed files with 175 additions and 9 deletions
+44 -9
View File
@@ -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);
+131
View File
@@ -3364,6 +3364,136 @@ error:
return 1;
} /* end test_compound_15() */
/*-------------------------------------------------------------------------
* Function: test_compound_15_attr
*
* Purpose: Tests that conversion occurs correctly when the source is
* subset of the destination, but there is extra space at the
* end of the source type. This one tests on attribute while
* test_compound_15() tests on dataset. That's the only
* difference.
*
* Return: Success: 0
*
* Failure: number of errors
*
* Programmer: Ray Lu
* 14 July 2022
*
* Modifications:
*
*-------------------------------------------------------------------------
*/
static int
test_compound_15_attr(void)
{
typedef struct cmpd_struct {
int i1;
int i2;
} cmpd_struct;
cmpd_struct wdata1 = {1254, 5471};
cmpd_struct rdata;
int wdata2[2] = {1, 2};
hid_t file = H5I_INVALID_HID;
hid_t cmpd_m_tid = H5I_INVALID_HID;
hid_t cmpd_f_tid = H5I_INVALID_HID;
hid_t space_id = H5I_INVALID_HID;
hid_t attr_id = H5I_INVALID_HID;
hsize_t dim1[1];
char filename[1024];
TESTING("compound subset conversion with extra space in source for attribute");
/* Create File */
h5_fixname(FILENAME[3], H5P_DEFAULT, filename, sizeof(filename));
if ((file = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
FAIL_PUTS_ERROR("Can't create file!\n");
/* Create file compound datatype */
if ((cmpd_f_tid = H5Tcreate(H5T_COMPOUND, sizeof(struct cmpd_struct))) < 0)
FAIL_PUTS_ERROR("Can't create datatype!\n");
if (H5Tinsert(cmpd_f_tid, "i1", HOFFSET(struct cmpd_struct, i1), H5T_NATIVE_INT) < 0)
FAIL_PUTS_ERROR("Can't insert field 'i1'\n");
if (H5Tinsert(cmpd_f_tid, "i2", HOFFSET(struct cmpd_struct, i2), H5T_NATIVE_INT) < 0)
FAIL_PUTS_ERROR("Can't insert field 'i2'\n");
/* Create memory compound datatype */
if ((cmpd_m_tid = H5Tcreate(H5T_COMPOUND, sizeof(struct cmpd_struct))) < 0)
FAIL_PUTS_ERROR("Can't create datatype!\n");
if (H5Tinsert(cmpd_m_tid, "i1", (size_t)0, H5T_NATIVE_INT) < 0)
FAIL_PUTS_ERROR("Can't insert field 'i1'\n");
/* Create space, dataset, write wdata1 */
dim1[0] = 1;
if ((space_id = H5Screate_simple(1, dim1, NULL)) < 0)
FAIL_PUTS_ERROR("Can't create space\n");
if ((attr_id = H5Acreate_by_name(file, ".", "attr_cmpd", cmpd_f_tid, space_id, H5P_DEFAULT, H5P_DEFAULT,
H5P_DEFAULT)) < 0)
FAIL_PUTS_ERROR("Can't create dataset\n");
if (H5Awrite(attr_id, cmpd_f_tid, &wdata1) < 0)
FAIL_PUTS_ERROR("Can't write data\n");
/* Write wdata2. The use of cmpd_m_tid here should cause only the first
* element of wdata2 to be written. */
if (H5Awrite(attr_id, cmpd_m_tid, &wdata2) < 0)
FAIL_PUTS_ERROR("Can't write data\n");
/* Read data */
if (H5Aread(attr_id, cmpd_f_tid, &rdata) < 0)
FAIL_PUTS_ERROR("Can't read data\n");
/* Check for correctness of read data */
if (rdata.i1 != wdata2[0] || rdata.i2 != wdata1.i2)
FAIL_PUTS_ERROR("incorrect read data\n");
/* Now try reading only the i1 field, verify it does not overwrite i2 in the
* read buffer */
rdata.i1 = wdata1.i1;
rdata.i2 = wdata2[1];
/* Read data */
if (H5Aread(attr_id, cmpd_m_tid, &rdata) < 0)
FAIL_PUTS_ERROR("Can't read data\n");
/* Check for correctness of read data */
if (rdata.i1 != wdata2[0] || rdata.i2 != wdata2[1])
FAIL_PUTS_ERROR("incorrect read data\n");
/* Close */
if (H5Aclose(attr_id) < 0)
goto error;
if (H5Tclose(cmpd_f_tid) < 0)
goto error;
if (H5Tclose(cmpd_m_tid) < 0)
goto error;
if (H5Sclose(space_id) < 0)
goto error;
if (H5Fclose(file) < 0)
goto error;
PASSED();
return 0;
error:
H5E_BEGIN_TRY
{
H5Aclose(attr_id);
H5Tclose(cmpd_f_tid);
H5Tclose(cmpd_m_tid);
H5Sclose(space_id);
H5Fclose(file);
}
H5E_END_TRY;
return 1;
} /* end test_compound_15_attr() */
/*-------------------------------------------------------------------------
* Function: test_compound_16
*
@@ -8852,6 +8982,7 @@ main(void)
}
nerrors += test_compound_15();
nerrors += test_compound_15_attr();
nerrors += test_compound_16();
nerrors += test_compound_17();
nerrors += test_compound_18();