Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove some H5T_copy calls that are now unnecessary #4164

Merged
merged 2 commits into from
Mar 19, 2024
+1,260 −1,038
Merged

Remove some H5T_copy calls that are now unnecessary #4164

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a68c8c2
Remove some H5T_copy calls that are now unnecessary
jhendersonHDF Mar 12, 2024
a3c4995
Address some comments from review
jhendersonHDF Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 9 additions & 37 deletions src/H5Aint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -681,9 +681,7 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
uint8_t *bkg_buf = NULL; /* background buffer */
hssize_t snelmts; /* elements in attribute */
size_t nelmts; /* elements in attribute*/
H5T_path_t *tpath = NULL; /* type conversion info */
H5T_t *src_type = NULL; /* temporary datatype */
H5T_t *dst_type = NULL; /* temporary datatype */
H5T_path_t *tpath = NULL; /* type conversion info */
size_t src_type_size; /* size of source type */
size_t dst_type_size; /* size of destination type */
size_t buf_size; /* desired buffer size */
Expand Down Expand Up @@ -723,11 +721,6 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
if (!H5T_path_noop(tpath)) {
H5T_bkg_t need_bkg; /* Background buffer type */

if (NULL == (src_type = H5T_copy(attr->shared->dt, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy attribute datatype");
if (NULL == (dst_type = H5T_copy(mem_type, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy memory datatype");

/* Get the maximum buffer size needed and allocate it */
buf_size = nelmts * MAX(src_type_size, dst_type_size);
if (NULL == (tconv_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
Expand All @@ -752,8 +745,8 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)
}

/* Perform datatype conversion. */
if (H5T_convert(tpath, src_type, dst_type, nelmts, (size_t)0, (size_t)0, tconv_buf, bkg_buf) <
0)
if (H5T_convert(tpath, attr->shared->dt, mem_type, nelmts, (size_t)0, (size_t)0, tconv_buf,
bkg_buf) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTCONVERT, FAIL, "datatype conversion failed");

/* Copy the converted data into the user's buffer */
Expand All @@ -771,10 +764,6 @@ H5A__read(const H5A_t *attr, const H5T_t *mem_type, void *buf)

done:
/* Release resources */
if (src_type && H5T_close(src_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (dst_type && H5T_close(dst_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (tconv_buf)
tconv_buf = H5FL_BLK_FREE(attr_buf, tconv_buf);
if (bkg_buf)
Expand Down Expand Up @@ -805,9 +794,7 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
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*/
H5T_t *src_type = NULL; /* temporary datatype */
H5T_t *dst_type = NULL; /* temporary datatype */
H5T_path_t *tpath = NULL; /* conversion information*/
size_t src_type_size; /* size of source type */
size_t dst_type_size; /* size of destination type*/
size_t buf_size; /* desired buffer size */
Expand Down Expand Up @@ -843,11 +830,6 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
if (!H5T_path_noop(tpath)) {
H5T_bkg_t need_bkg; /* Background buffer type */

if (NULL == (src_type = H5T_copy(mem_type, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy memory datatype");
if (NULL == (dst_type = H5T_copy(attr->shared->dt, H5T_COPY_ALL)))
HGOTO_ERROR(H5E_ATTR, H5E_CANTCOPY, FAIL, "unable to copy attribute datatype");

/* Get the maximum buffer size needed and allocate it */
buf_size = nelmts * MAX(src_type_size, dst_type_size);
if (NULL == (tconv_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
Expand Down Expand Up @@ -880,7 +862,8 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)
}

/* Perform datatype conversion */
if (H5T_convert(tpath, src_type, dst_type, nelmts, (size_t)0, (size_t)0, tconv_buf, bkg_buf) < 0)
if (H5T_convert(tpath, mem_type, attr->shared->dt, nelmts, (size_t)0, (size_t)0, tconv_buf,
bkg_buf) < 0)
HGOTO_ERROR(H5E_ATTR, H5E_CANTCONVERT, FAIL, "datatype conversion failed");

/* Free the previous attribute data buffer, if there is one */
Expand Down Expand Up @@ -911,10 +894,6 @@ H5A__write(H5A_t *attr, const H5T_t *mem_type, const void *buf)

done:
/* Release resources */
if (src_type && H5T_close(src_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (dst_type && H5T_close(dst_type) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
if (tconv_buf)
tconv_buf = H5FL_BLK_FREE(attr_buf, tconv_buf);
if (bkg_buf)
Expand Down Expand Up @@ -2084,10 +2063,10 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
{
H5A_t *attr_dst = NULL; /* Destination attribute */
H5T_t *dt_mem = NULL; /* Memory datatype */
H5S_t *buf_space = NULL; /* Dataspace describing buffer */
void *buf = NULL; /* Buffer for copying data */
void *reclaim_buf = NULL; /* Buffer for reclaiming data */
void *bkg_buf = NULL; /* Background buffer */
hid_t buf_sid = -1; /* ID for buffer dataspace */
hssize_t sdst_nelmts; /* # of elements in destination attribute (signed) */
size_t dst_nelmts; /* # of elements in destination attribute */
size_t dst_dt_size; /* Size of destination attribute datatype */
Expand Down Expand Up @@ -2200,7 +2179,6 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
size_t src_dt_size; /* Source datatype size */
size_t tmp_dt_size; /* Temp. datatype size */
size_t max_dt_size; /* Max atatype size */
H5S_t *buf_space; /* Dataspace describing buffer */
hsize_t buf_dim; /* Dimension for buffer */
size_t nelmts; /* Number of elements in buffer */
size_t buf_size; /* Size of copy buffer */
Expand Down Expand Up @@ -2241,12 +2219,6 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
if (NULL == (buf_space = H5S_create_simple((unsigned)1, &buf_dim, NULL)))
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTCREATE, NULL, "can't create simple dataspace");

/* Register */
if ((buf_sid = H5I_register(H5I_DATASPACE, buf_space, false)) < 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ID was only being used as a wrapper for closing buf_space

H5S_close(buf_space);
HGOTO_ERROR(H5E_ID, H5E_CANTREGISTER, NULL, "unable to register dataspace ID");
} /* end if */

/* Allocate memory for recclaim buf */
if (NULL == (reclaim_buf = H5FL_BLK_MALLOC(attr_buf, buf_size)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation NULLed for raw data chunk");
Expand Down Expand Up @@ -2304,10 +2276,10 @@ H5A__attr_copy_file(const H5A_t *attr_src, H5F_t *file_dst, bool *recompute_size
ret_value = attr_dst;

done:
if (buf_sid > 0 && H5I_dec_ref(buf_sid) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTFREE, NULL, "can't decrement temporary dataspace ID");
if (dt_mem && (H5T_close(dt_mem) < 0))
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, NULL, "can't close temporary datatype");
if (buf_space && H5S_close(buf_space) < 0)
HDONE_ERROR(H5E_ATTR, H5E_CANTCLOSEOBJ, NULL, "can't close temporary dataspace");
if (buf)
buf = H5FL_BLK_FREE(attr_buf, buf);
if (reclaim_buf)
Expand Down
4 changes: 2 additions & 2 deletions src/H5D.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1853,7 +1853,7 @@ H5Dfill(const void *fill, hid_t fill_type_id, void *buf, hid_t buf_type_id, hid_
herr_t
H5Diterate(void *buf, hid_t type_id, hid_t space_id, H5D_operator_t op, void *operator_data)
{
H5T_t *type; /* Datatype */
const H5T_t *type; /* Datatype */
H5S_t *space; /* Dataspace for iteration */
H5S_sel_iter_op_t dset_op; /* Operator for iteration */
herr_t ret_value; /* Return value */
Expand All @@ -1868,7 +1868,7 @@ H5Diterate(void *buf, hid_t type_id, hid_t space_id, H5D_operator_t op, void *op
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "invalid buffer");
if (H5I_DATATYPE != H5I_get_type(type_id))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid datatype");
if (NULL == (type = (H5T_t *)H5I_object_verify(type_id, H5I_DATATYPE)))
if (NULL == (type = (const H5T_t *)H5I_object_verify(type_id, H5I_DATATYPE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not an valid base datatype");
if (NULL == (space = (H5S_t *)H5I_object_verify(space_id, H5I_DATASPACE)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid dataspace");
Expand Down
Loading
Loading