Skip to content

Commit

Permalink
Fix segfault on user compound dtype conversion callback (#3842)
Browse files Browse the repository at this point in the history
* Fix segfault on user compound conversion

* Check if lib conversion func is in use
  • Loading branch information
mattjala authored Nov 22, 2023
1 parent 3ea21cc commit af4c6c4
Show file tree
Hide file tree
Showing 2 changed files with 135 additions and 7 deletions.
17 changes: 10 additions & 7 deletions src/H5T.c
Original file line number Diff line number Diff line change
Expand Up @@ -5174,10 +5174,10 @@ H5T_path_noop(const H5T_path_t *p)
/*-------------------------------------------------------------------------
* Function: H5T_path_compound_subset
*
* Purpose: Checks if the source and destination types are both compound.
* Tells whether whether the source members are a subset of
* destination, and the order is the same, and no conversion
* is needed. For example:
* Purpose: Checks if the library's compound conversion function
* is in use. Tells whether whether the source members are
* a subset of destination, and the order is the same, and
* no conversion is needed. For example:
* struct source { struct destination {
* TYPE1 A; --> TYPE1 A;
* TYPE2 B; --> TYPE2 B;
Expand All @@ -5186,8 +5186,9 @@ H5T_path_noop(const H5T_path_t *p)
* TYPE5 E;
* };
*
* Return: A pointer to the subset info struct in p, or NULL if there are
* no compounds. Points directly into the H5T_path_t structure.
* Return: A pointer to the subset info struct in p, or NULL if the
* library's compound conversion function is not in use.
* Points directly into the H5T_path_t structure.
*
*-------------------------------------------------------------------------
*/
Expand All @@ -5200,7 +5201,9 @@ H5T_path_compound_subset(const H5T_path_t *p)

assert(p);

if (p->are_compounds)
/* Only retrieve private info if the library compound conversion
* function is in use */
if (!p->conv.is_app && (p->conv.u.lib_func == H5T__conv_struct))
ret_value = H5T__conv_struct_subset(&(p->cdata));

FUNC_LEAVE_NOAPI(ret_value)
Expand Down
125 changes: 125 additions & 0 deletions test/dtypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,15 @@ typedef enum dtype_t {
OTHER
} dtype_t; /* This doesn't seem to be used anywhere... -BMR */

typedef struct src_cmpd_t {
uint32_t a;
float b;
} src_cmpd_t;

typedef struct dst_cmpd_t {
float b;
} dst_cmpd_t;

typedef enum { E1_RED, E1_GREEN, E1_BLUE, E1_ORANGE, E1_YELLOW } color_t;

/* Constant for size of conversion buffer for int <-> float exception test */
Expand Down Expand Up @@ -160,6 +169,28 @@ reset_hdf5(void)
#endif
}

/* Conversion function to test user compound conversion functions */
static herr_t
user_compound_convert(hid_t H5_ATTR_UNUSED src_id, hid_t H5_ATTR_UNUSED dst_id, H5T_cdata_t *cdata,
size_t nelmts, size_t H5_ATTR_UNUSED buf_stride, size_t H5_ATTR_UNUSED bkg_stride,
void *buf, void H5_ATTR_UNUSED *bkg, hid_t H5_ATTR_UNUSED dxpl)
{
herr_t retval = EXIT_SUCCESS;
switch (cdata->command) {
case H5T_CONV_INIT:
break;
case H5T_CONV_CONV:
for (size_t i = 0; i < nelmts; ++i)
((dst_cmpd_t *)buf)[i].b = ((src_cmpd_t *)buf)[i].b;
break;
case H5T_CONV_FREE:
break;
default:
break;
}
return retval;
}

/*-------------------------------------------------------------------------
* Function: test_classes
*
Expand Down Expand Up @@ -3788,6 +3819,99 @@ test_compound_18(void)
return 1;
} /* end test_compound_18() */

/*-------------------------------------------------------------------------
* Function: test_user_compound_conversion
*
* Purpose: Tests that library correctly handles a user-defined
* conversion function between two compound types.
*
* Return: Success: 0
* Failure: number of errors
*
*-------------------------------------------------------------------------
*/
static int
test_user_compound_conversion(void)
{
hid_t file_id = H5I_INVALID_HID;
hid_t dset_id = H5I_INVALID_HID;
hid_t src = H5I_INVALID_HID, dst = H5I_INVALID_HID;
hid_t space_id = H5I_INVALID_HID;
struct src_cmpd_t buf[] = {{1, 1.0}};

hsize_t dim = 1;
char filename[1024];

TESTING("compound conversion via user conversion callback");

/* Create the source compound datatype */
if ((src = H5Tcreate(H5T_COMPOUND, sizeof(struct src_cmpd_t))) < 0)
TEST_ERROR;

if (H5Tinsert(src, "a", HOFFSET(struct src_cmpd_t, a), H5T_NATIVE_UINT32) < 0)
TEST_ERROR;

if (H5Tinsert(src, "b", HOFFSET(struct src_cmpd_t, b), H5T_NATIVE_FLOAT) < 0)
TEST_ERROR;

/* Create the destination compound datatype */
if ((dst = H5Tcreate(H5T_COMPOUND, sizeof(struct dst_cmpd_t))) < 0)
TEST_ERROR;

if (H5Tinsert(dst, "b", HOFFSET(struct dst_cmpd_t, b), H5T_IEEE_F32LE) < 0)
TEST_ERROR;

if (H5Tregister(H5T_PERS_SOFT, "src_cmpd_t->dst_cmpd_t", src, dst, &user_compound_convert) < 0)
TEST_ERROR;

/* Create File */
h5_fixname(FILENAME[0], H5P_DEFAULT, filename, sizeof filename);
if ((file_id = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
TEST_ERROR;

/* Create a dataspace to use */
if ((space_id = H5Screate_simple(1, &dim, NULL)) < 0)
TEST_ERROR;

/* Create a dataset with the destination compound datatype */
if ((dset_id = H5Dcreate2(file_id, "dataset", dst, space_id, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0)
TEST_ERROR;

if (H5Dwrite(dset_id, src, space_id, H5S_ALL, H5P_DEFAULT, buf) < 0)
TEST_ERROR;

/* Close IDs */
if (H5Tunregister(H5T_PERS_SOFT, "src_cmpd_t->dst_cmpd_t", src, dst, &user_compound_convert) < 0)
TEST_ERROR;
if (H5Tclose(src) < 0)
TEST_ERROR;
if (H5Tclose(dst) < 0)
TEST_ERROR;
if (H5Sclose(space_id) < 0)
TEST_ERROR;
if (H5Dclose(dset_id) < 0)
TEST_ERROR;
if (H5Fclose(file_id) < 0)
TEST_ERROR;

PASSED();
return 0;

error:
H5E_BEGIN_TRY
{
H5Tunregister(H5T_PERS_SOFT, "src_cmpd_t->dst_cmpd_t", src, dst, &user_compound_convert);
H5Tclose(src);
H5Tclose(dst);
H5Sclose(space_id);
H5Dclose(dset_id);
H5Fclose(file_id);
}
H5E_END_TRY;

return 1;
} /* end test_user_compound_conversion() */

/*-------------------------------------------------------------------------
* Function: test_query
*
Expand Down Expand Up @@ -8786,6 +8910,7 @@ main(void)
nerrors += test_compound_16();
nerrors += test_compound_17();
nerrors += test_compound_18();
nerrors += test_user_compound_conversion();
nerrors += test_conv_enum_1();
nerrors += test_conv_enum_2();
nerrors += test_conv_bitfield();
Expand Down

0 comments on commit af4c6c4

Please sign in to comment.