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.

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

Add H5Tdecode2, rename and deprecate H5Tdecode #5213

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from
5 changes: 3 additions & 2 deletions c++/src/H5DataType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ DataType::p_decode() const
}

// Call C function to decode the binary object description
hid_t encoded_dtype_id = H5Tdecode(encoded_buf);
hid_t encoded_dtype_id = H5Tdecode2(encoded_buf, buf_size);

mattjala marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for addressing the issue, but I'm sorry, I made a mistake in the original comment. If encoded_buf is not NULL then buf_size will already be the size of the encoded buffer. You can safely use buf_size in H5Tdecode2 as you originally did. I'm sorry again!

Copy link
Contributor

@bmribler bmribler Jan 8, 2025

Choose a reason for hiding this comment

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

Without having to call H5Tencode to get the size, that is. Checking buf_size is a good idea though. However, if you want to put a fix in DataType::close() to set encoded_buf to NULL then checking for NULL encoded_buf alone will be good enough. Question me if you see anything I said questionable. :D

// If H5Tdecode fails, raise exception
if (encoded_dtype_id < 0) {
Expand Down Expand Up @@ -924,7 +924,8 @@ DataType::close()
// Free and reset buffer of encoded object description if it's been used
if (encoded_buf != NULL) {
delete[] encoded_buf;
buf_size = 0;
encoded_buf = NULL;
buf_size = 0;
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions fortran/src/H5Tf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1898,6 +1898,7 @@ h5tcommitted_c(hid_t_f *dtype_id)
* INPUTS
*
* buf - Buffer for the data space object to be decoded.
* buf_size - Size of the buffer
* OUTPUTS
*
* obj_id - Object_id (non-negative)
Expand All @@ -1908,7 +1909,7 @@ h5tcommitted_c(hid_t_f *dtype_id)
*/

int_f
h5tdecode_c(_fcd buf, hid_t_f *obj_id)
h5tdecode_c(_fcd buf, size_t_f buf_size, hid_t_f *obj_id)
/******/
{
int ret_value = -1;
Expand All @@ -1921,7 +1922,7 @@ h5tdecode_c(_fcd buf, hid_t_f *obj_id)

c_buf = (unsigned char *)buf;

c_obj_id = H5Tdecode(c_buf);
c_obj_id = H5Tdecode2(c_buf, buf_size);
if (c_obj_id < 0)
return ret_value;

Expand Down
17 changes: 10 additions & 7 deletions fortran/src/H5Tff.F90
Original file line number Diff line number Diff line change
Expand Up @@ -1857,28 +1857,31 @@ END SUBROUTINE h5tcommitted_f
!!
!! \brief Decode A binary object description of data type and return a new object handle.
!!
!! \param buf Buffer for the data space object to be decoded.
!! \param obj_id Object ID.
!! \param hdferr \fortran_error
!! \param buf Buffer for the data space object to be decoded.
!! \param buf_size Size of the buffer.
!! \param obj_id Object ID.
!! \param hdferr \fortran_error
!!
!! See C API: @ref H5Tdecode()
!!
SUBROUTINE h5tdecode_f(buf, obj_id, hdferr)
SUBROUTINE h5tdecode_f(buf, buf_size, obj_id, hdferr)
IMPLICIT NONE
CHARACTER(LEN=*), INTENT(IN) :: buf
INTEGER(SIZE_T), INTENT(IN) :: buf_size
INTEGER(HID_T), INTENT(OUT) :: obj_id
INTEGER, INTENT(OUT) :: hdferr
INTERFACE
INTEGER FUNCTION h5tdecode_c(buf, obj_id) BIND(C,NAME='h5tdecode_c')
INTEGER FUNCTION h5tdecode_c(buf, buf_size, obj_id) BIND(C,NAME='h5tdecode_c')
IMPORT :: C_CHAR
IMPORT :: HID_T
IMPORT :: HID_T, SIZE_T
IMPLICIT NONE
CHARACTER(KIND=C_CHAR), DIMENSION(*), INTENT(IN) :: buf
INTEGER(SIZE_T), INTENT(IN) :: buf_size
INTEGER(HID_T), INTENT(OUT) :: obj_id
END FUNCTION h5tdecode_c
END INTERFACE

hdferr = h5tdecode_c(buf, obj_id)
hdferr = h5tdecode_c(buf, buf_size, obj_id)

END SUBROUTINE h5tdecode_f

Expand Down
2 changes: 1 addition & 1 deletion fortran/src/H5f90proto.h
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ H5_FCDLL int_f h5tvlen_create_c(hid_t_f *type_id, hid_t_f *vltype_id);
H5_FCDLL int_f h5tis_variable_str_c(hid_t_f *type_id, int_f *flag);
H5_FCDLL int_f h5tget_member_class_c(hid_t_f *type_id, int_f *member_no, int_f *cls);
H5_FCDLL int_f h5tcommit_anon_c(hid_t_f *loc_id, hid_t_f *dtype_id, hid_t_f *tcpl_id, hid_t_f *tapl_id);
H5_FCDLL int_f h5tdecode_c(_fcd buf, hid_t_f *obj_id);
H5_FCDLL int_f h5tdecode_c(_fcd buf, size_t_f buf_size, hid_t_f *obj_id);
H5_FCDLL int_f h5tencode_c(_fcd buf, hid_t_f *obj_id, size_t_f *nalloc);
H5_FCDLL int_f h5tget_create_plist_c(hid_t_f *dtype_id, hid_t_f *dtpl_id);
H5_FCDLL int_f h5tcompiler_conv_c(hid_t_f *src_id, hid_t_f *dst_id, int_f *c_flag);
Expand Down
4 changes: 2 additions & 2 deletions fortran/test/tH5T.F90
Original file line number Diff line number Diff line change
Expand Up @@ -555,14 +555,14 @@ SUBROUTINE compoundtest(cleanup, total_error)

! Try decoding bogus buffer

CALL H5Tdecode_f(cmpd_buf, decoded_tid1, error)
CALL H5Tdecode_f(cmpd_buf, cmpd_buf_size, decoded_tid1, error)
CALL verify("H5Tdecode_f", error, -1, total_error)

CALL H5Tencode_f(dtype_id, cmpd_buf, cmpd_buf_size, error)
CALL check("H5Tencode_f", error, total_error)

! Decode from the compound buffer and return an object handle
CALL H5Tdecode_f(cmpd_buf, decoded_tid1, error)
CALL H5Tdecode_f(cmpd_buf, cmpd_buf_size, decoded_tid1, error)
CALL check("H5Tdecode_f", error, total_error)

! Verify that the datatype was copied exactly
Expand Down
9 changes: 6 additions & 3 deletions java/src/hdf/hdf5lib/H5.java
Original file line number Diff line number Diff line change
Expand Up @@ -13865,16 +13865,19 @@ public static long H5Tcreate(int tclass, long size) throws HDF5LibraryException
* @param buf
* IN: Buffer for the data type object to be decoded.
*
* @param buf_size
* IN: Size of the buffer.
*
* @return a new object handle
*
* @exception HDF5LibraryException
* Error from the HDF5 Library.
* @exception NullPointerException
* buf is null.
**/
public static long H5Tdecode(byte[] buf) throws HDF5LibraryException, NullPointerException
public static long H5Tdecode(byte[] buf, long buf_size) throws HDF5LibraryException, NullPointerException
{
long id = _H5Tdecode(buf);
long id = _H5Tdecode(buf, buf_size);
if (id > 0) {
log.trace("OPEN_IDS: H5Tdecode add {}", id);
OPEN_IDS.add(id);
Expand All @@ -13883,7 +13886,7 @@ public static long H5Tdecode(byte[] buf) throws HDF5LibraryException, NullPointe
return id;
}

private synchronized static native long _H5Tdecode(byte[] buf)
Copy link
Collaborator

Choose a reason for hiding this comment

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

At first I was going to mention that there should be the corresponding JNI changes, but it seems there is no _H5Tdecode implementation currently. @byrnHDF We should either implement that or remove this function from being available.

private synchronized static native long _H5Tdecode(byte[] buf, long buf_size)
throws HDF5LibraryException, NullPointerException;

/**
Expand Down
17 changes: 17 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,23 @@ New Features
H5Iregister_type() will map to the new signature unless the library is
explicitly configured to use an older version of the API.

- The H5Tdecode() signature has changed

When provided malformed or too-small buffers, H5Tdecode() would crash.
The new buffer size parameter allows this to be reliably avoided.

The old signature has been renamed to H5Tdecode1() and is considered
deprecated:

hid_t H5Tdecode1(const void *buf);

The new signature is H5Tdecode2(). New code should use this version:

hid_t H5Tdecode2(const void *buf, size_t buf_size);

H5Tdecode() will map to the new signature unless the library is
explicitly configured to use an older version of the API.

- H5F_LIBVER_LATEST is now an enum value

This was previously #defined to the latest H5F_libver_t API version, but
Expand Down
21 changes: 11 additions & 10 deletions src/H5T.c
Original file line number Diff line number Diff line change
Expand Up @@ -3691,7 +3691,7 @@ H5Tencode(hid_t obj_id, void *buf, size_t *nalloc)
} /* end H5Tencode() */

/*-------------------------------------------------------------------------
mattjala marked this conversation as resolved.
Show resolved Hide resolved
* Function: H5Tdecode
* Function: H5Tdecode2
*
* Purpose: Decode a binary object description and return a new object
* handle.
Expand All @@ -3703,7 +3703,7 @@ H5Tencode(hid_t obj_id, void *buf, size_t *nalloc)
*-------------------------------------------------------------------------
*/
hid_t
H5Tdecode(const void *buf)
H5Tdecode2(const void *buf, size_t buf_size)
{
H5T_t *dt;
hid_t ret_value; /* Return value */
Expand All @@ -3714,13 +3714,8 @@ H5Tdecode(const void *buf)
if (buf == NULL)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "empty buffer");

/* Create datatype by decoding buffer
* There is no way to get the size of the buffer, so we pass in
* SIZE_MAX and assume the caller knows what they are doing.
* Really fixing this will require an H5Tdecode2() call that
* takes a size parameter.
*/
if (NULL == (dt = H5T_decode(SIZE_MAX, (const unsigned char *)buf)))
/* Create datatype by decoding buffer */
if (NULL == (dt = H5T_decode(buf_size, buf)))
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, H5I_INVALID_HID, "can't decode object");

/* Register the type and return the ID */
Expand All @@ -3729,7 +3724,7 @@ H5Tdecode(const void *buf)

done:
FUNC_LEAVE_API(ret_value)
} /* end H5Tdecode() */
} /* end H5Tdecode2() */

/*-------------------------------------------------------------------------
* API functions are above; library-private functions are below...
Expand Down Expand Up @@ -3812,10 +3807,16 @@ H5T_decode(size_t buf_size, const unsigned char *buf)
if (NULL == (f = H5F_fake_alloc((uint8_t)0)))
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTALLOC, NULL, "can't allocate fake file struct");

if (buf_size != SIZE_MAX && H5_IS_BUFFER_OVERFLOW(buf, 1, buf + buf_size))
HGOTO_ERROR(H5E_DATATYPE, H5E_BADMESG, NULL, "buffer too small to be datatype message");

/* Decode the type of the information */
if (*buf++ != H5O_DTYPE_ID)
HGOTO_ERROR(H5E_DATATYPE, H5E_BADMESG, NULL, "not an encoded datatype");

if (buf_size != SIZE_MAX && H5_IS_BUFFER_OVERFLOW(buf, 1, buf + buf_size))
HGOTO_ERROR(H5E_DATATYPE, H5E_BADMESG, NULL, "buffer too small to be datatype message");

/* Decode the version of the datatype information */
if (*buf++ != H5T_ENCODE_VERSION)
HGOTO_ERROR(H5E_DATATYPE, H5E_VERSION, NULL, "unknown version of encoded datatype");
Expand Down
44 changes: 44 additions & 0 deletions src/H5Tdeprec.c
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,48 @@ H5Topen1(hid_t loc_id, const char *name)

FUNC_LEAVE_API(ret_value)
} /* end H5Topen1() */

/*-------------------------------------------------------------------------
* Function: H5Tdecode1
*
* Purpose: Decode a binary object description and return a new object
* handle.
*
* Note: Deprecated in favor of H5Tdecode2
*
* Return: Success: datatype ID(non-negative)
*
* Failure: negative
*
*-------------------------------------------------------------------------
*/
hid_t
H5Tdecode1(const void *buf)
{
H5T_t *dt;
hid_t ret_value; /* Return value */

FUNC_ENTER_API(H5I_INVALID_HID)

/* Check args */
if (buf == NULL)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, H5I_INVALID_HID, "empty buffer");

/* Create datatype by decoding buffer
* There is no way to get the size of the buffer, so we pass in
* SIZE_MAX and assume the caller knows what they are doing.
* Really fixing this will require an H5Tdecode2() call that
* takes a size parameter.
*/
if (NULL == (dt = H5T_decode(SIZE_MAX, (const unsigned char *)buf)))
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTDECODE, H5I_INVALID_HID, "can't decode object");

/* Register the type and return the ID */
if ((ret_value = H5I_register(H5I_DATATYPE, dt, true)) < 0)
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREGISTER, H5I_INVALID_HID, "unable to register data type");

done:
FUNC_LEAVE_API(ret_value)
} /* end H5Tdecode1() */

#endif /* H5_NO_DEPRECATED_SYMBOLS */
39 changes: 35 additions & 4 deletions src/H5Tpublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -1452,11 +1452,12 @@ H5_DLL herr_t H5Tencode(hid_t obj_id, void *buf, size_t *nalloc);
* object handle
*
* \param[in] buf Buffer for the datatype object to be decoded
* \param[in] buf_size Size of the buffer
*
* \return \hid_t{datatype}
*
* \details H5Tdecode() Given an object description of datatype in binary in a
* buffer, H5Tdecode() reconstructs the HDF5 datatype object and
* \details H5Tdecode2() Given an object description of datatype in binary in a
* buffer, H5Tdecode2() reconstructs the HDF5 datatype object and
* returns a new object handle for it. The binary description of
* the object is encoded by H5Tencode(). User is responsible for
* passing in the right buffer.
Expand All @@ -1465,10 +1466,11 @@ H5_DLL herr_t H5Tencode(hid_t obj_id, void *buf, size_t *nalloc);
* with H5Tclose() when the identifier is no longer needed so that
* resource leaks will not develop.
*
* \since 1.2.0
* \since 2.0.0
*
*/
H5_DLL hid_t H5Tdecode(const void *buf);
H5_DLL hid_t H5Tdecode2(const void *buf, size_t buf_size);

/**
* \ingroup H5T
*
Expand Down Expand Up @@ -2910,6 +2912,35 @@ H5_DLL herr_t H5Treclaim(hid_t type_id, hid_t space_id, hid_t plist_id, void *bu
/* Typedefs */

/* Function prototypes */
/**
* \ingroup H5T
*
* \brief Decodes a binary object description of datatype and returns a new
* object handle
*
* \param[in] buf Buffer for the datatype object to be decoded
*
* \return \hid_t{datatype}
*
* \deprecated This function has been renamed from H5Tdecode() and is
* deprecated in favor of the macro #H5Tdecode or the function
* H5Tdecode2().
*
* \details H5Tdecode1() Given an object description of datatype in binary in a
* buffer, H5Tdecode() reconstructs the HDF5 datatype object and
* returns a new object handle for it. The binary description of
* the object is encoded by H5Tencode(). User is responsible for
* passing in the right buffer.
*
* The datatype identifier returned by this function can be released
* with H5Tclose() when the identifier is no longer needed so that
* resource leaks will not develop.
* \version 2.0.0 C function H5Tdecode() renamed to H5Tdecode1() and deprecated
* in this release.
* \since 1.2.0
*
*/
H5_DLL hid_t H5Tdecode1(const void *buf);
/**
* \ingroup H5T
*
Expand Down
1 change: 1 addition & 0 deletions src/H5vers.txt
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ FUNCTION: H5Tarray_create; ; v14, v18
FUNCTION: H5Tcommit; ; v10, v18
FUNCTION: H5Tget_array_dims; ; v14, v18
FUNCTION: H5Topen; ; v10, v18
FUNCTION: H5Tdecode; ; v12, v200

# API typedefs
# (although not required, it's easier to compare this file with the headers
Expand Down
Loading
Loading