diff options
| -rw-r--r-- | release_docs/RELEASE.txt | 9 | ||||
| -rw-r--r-- | src/H5Aint.c | 53 | ||||
| -rw-r--r-- | test/dtypes.c | 131 |
3 files changed, 184 insertions, 9 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index e3932e1..bf2e386 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -170,6 +170,15 @@ Bug Fixes since HDF5-1.12.1 release =================================== Library ------- + - Fixed an issue with attribute type conversion with compound datatypes + + Previously, when performing type conversion for attribute I/O with a + compound datatype, the library would not fill the background buffer with + the contents of the destination, potentially causing data to be lost when + only writing to a subset of the compound fields. + + (NAF - 2022/08/22, GitHub #2016) + - Modified H5Fstart_swmr_write() to preserve DAPL properties Internally, H5Fstart_swmr_write() closes and reopens the file in question diff --git a/src/H5Aint.c b/src/H5Aint.c index 49d1bc4..876bfa2 100644 --- a/src/H5Aint.c +++ b/src/H5Aint.c @@ -626,6 +626,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) @@ -635,12 +637,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") @@ -691,9 +704,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*/ @@ -727,6 +739,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") @@ -735,12 +749,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") @@ -751,7 +786,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 { @@ -777,7 +812,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); diff --git a/test/dtypes.c b/test/dtypes.c index eaa7627..c72fce0 100644 --- a/test/dtypes.c +++ b/test/dtypes.c @@ -3363,6 +3363,136 @@ error: } /* 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 * * Purpose: Tests that committed types that can be registered during @@ -8829,6 +8959,7 @@ main(void) nerrors += test_compound_13(); nerrors += test_compound_14(); nerrors += test_compound_15(); + nerrors += test_compound_15_attr(); nerrors += test_compound_16(); nerrors += test_compound_17(); nerrors += test_compound_18(); |