Skip to content

Commit

Permalink
Fixed HDFFV-10933
Browse files Browse the repository at this point in the history
Description:
    Updated the original fix by Kent Y. in commit
        200a77d
    - used internal functions instead of public API
    - moved some code into the subroutine for a cleaner look.
    - added test to dsets.c
Platforms tested:
    Linux/64 (jelly)
  • Loading branch information
bmribler committed Aug 14, 2020
1 parent 044ee6f commit 16349c5
Show file tree
Hide file tree
Showing 5 changed files with 199 additions and 14 deletions.
10 changes: 10 additions & 0 deletions release_docs/RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,16 @@ Bug Fixes since HDF5-1.10.3 release

Library
-------
- Creation of dataset with optional filter

When the combination of type, space, etc doesn't work for filter
and the filter is optional, it was supposed to be skipped but it was
not skipped and the creation failed.

Allowed the creation of the dataset in such situation.

(BMR - 2020/8/13, HDFFV-10933)

- Explicitly declared dlopen to use RTLD_LOCAL

dlopen documentation states that if neither RTLD_GLOBAL nor
Expand Down
34 changes: 21 additions & 13 deletions src/H5Dint.c
Original file line number Diff line number Diff line change
Expand Up @@ -1299,18 +1299,24 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id,

/* Check if the dataset has a non-default DCPL & get important values, if so */
if(new_dset->shared->dcpl_id != H5P_DATASET_CREATE_DEFAULT) {
H5O_layout_t *layout; /* Dataset's layout information */
H5O_pline_t *pline; /* Dataset's I/O pipeline information */
H5O_fill_t *fill; /* Dataset's fill value info */
H5O_efl_t *efl; /* Dataset's external file list info */
H5O_layout_t *layout; /* Dataset's layout information */
H5O_pline_t *pline; /* Dataset's I/O pipeline information */
H5O_fill_t *fill; /* Dataset's fill value info */
H5O_efl_t *efl; /* Dataset's external file list info */
htri_t ignore_filters = FALSE; /* Ignore optional filters or not */

/* Check if the filters in the DCPL can be applied to this dataset */
if(H5Z_can_apply(new_dset->shared->dcpl_id, new_dset->shared->type_id) < 0)
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "I/O filters can't operate on this dataset")
if((ignore_filters = H5Z_ignore_filters(new_dset->shared->dcpl_id, dt, space))<0)
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "H5Z_has_optional_filter() failed")

/* Make the "set local" filter callbacks for this dataset */
if(H5Z_set_local(new_dset->shared->dcpl_id, new_dset->shared->type_id) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, NULL, "unable to set local filter parameters")
if(FALSE == ignore_filters) {
/* Check if the filters in the DCPL can be applied to this dataset */
if(H5Z_can_apply(new_dset->shared->dcpl_id, new_dset->shared->type_id) < 0)
HGOTO_ERROR(H5E_ARGS, H5E_CANTINIT, NULL, "I/O filters can't operate on this dataset")

/* Make the "set local" filter callbacks for this dataset */
if(H5Z_set_local(new_dset->shared->dcpl_id, new_dset->shared->type_id) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, NULL, "unable to set local filter parameters")
} /* ignore_filters */

/* Get new dataset's property list object */
if(NULL == (dc_plist = (H5P_genplist_t *)H5I_object(new_dset->shared->dcpl_id)))
Expand All @@ -1334,9 +1340,11 @@ H5D__create(H5F_t *file, hid_t type_id, const H5S_t *space, hid_t dcpl_id,
HGOTO_ERROR(H5E_DATASET, H5E_CANTGET, NULL, "can't retrieve external file list")
efl_copied = TRUE;

/* Check that chunked layout is used if filters are enabled */
if(pline->nused > 0 && H5D_CHUNKED != layout->type)
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "filters can only be used with chunked layout")
if(FALSE == ignore_filters) {
/* Check that chunked layout is used if filters are enabled */
if(pline->nused > 0 && H5D_CHUNKED != layout->type)
HGOTO_ERROR(H5E_DATASET, H5E_BADVALUE, NULL, "filters can only be used with chunked layout")
}

/* Check if the alloc_time is the default and error out */
if(fill->alloc_time == H5D_ALLOC_TIME_DEFAULT)
Expand Down
67 changes: 67 additions & 0 deletions src/H5Z.c
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,73 @@ H5Z_set_local_direct(const H5O_pline_t *pline)
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5Z_set_local_direct() */


/*-------------------------------------------------------------------------
* Function: H5Z_ignore_filters
*
* Purpose: Determine whether filters can be ignored.
*
* Description:
* When the filters are optional (i.e., H5Z_FLAG_OPTIONAL is provided,)
* if any of the following conditions is met, the filters will be ignored:
* - dataspace is either H5S_NULL or H5S_SCALAR
* - datatype is variable-length (string or non-string)
* However, if any of these conditions exists and a filter is not
* optional, the function will produce an error.
*
* Return: Non-negative(TRUE/FALSE) on success
* Negative on failure
*
*-------------------------------------------------------------------------
*/
htri_t
H5Z_ignore_filters(hid_t dcpl_id, const H5T_t *type, const H5S_t *space)
{
H5P_genplist_t *dc_plist; /* Dataset creation property list object */
H5O_pline_t pline; /* Object's I/O pipeline information */
H5S_class_t space_class; /* To check class of space */
H5T_class_t type_class; /* To check if type is VL */
bool bad_for_filters = FALSE;/* Suitable to have filters */
htri_t ret_value = FALSE; /* TRUE for ignoring filters */

FUNC_ENTER_NOAPI(FAIL)

if (NULL == (dc_plist = (H5P_genplist_t *)H5I_object(dcpl_id)))
HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "can't get dataset creation property list")

/* Get pipeline information */
if (H5P_peek(dc_plist, H5O_CRT_PIPELINE_NAME, &pline) < 0)
HGOTO_ERROR(H5E_PLINE, H5E_CANTGET, FAIL, "can't retrieve pipeline filter")

/* Get datatype and dataspace classes for quick access */
space_class = H5S_GET_EXTENT_TYPE(space);
type_class = H5T_get_class(type, FALSE);

/* These conditions are not suitable for filters */
bad_for_filters = (H5S_NULL == space_class || H5S_SCALAR == space_class
|| H5T_VLEN == type_class
|| (H5T_STRING == type_class && TRUE == H5T_is_variable_str(type)));

/* When these conditions occur, if there are required filters in pline,
then report a failure, otherwise, set flag that they can be ignored */
if (bad_for_filters) {
size_t ii;
if (pline.nused > 0) {
for (ii = 0; ii < pline.nused; ii++)
{
if (!(pline.filter[ii].flags & H5Z_FLAG_OPTIONAL))
HGOTO_ERROR(H5E_PLINE, H5E_CANTFILTER, FAIL, "not suitable for filters")
}

/* All filters are optional, we can ignore them */
ret_value = TRUE;
}
} /* bad for filters */

done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5Z_ignore_filters() */


/*-------------------------------------------------------------------------
* Function: H5Z_modify
Expand Down
4 changes: 3 additions & 1 deletion src/H5Zprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ typedef struct H5Z_filter_info_t H5Z_filter_info_t;
#include "H5Zpublic.h"

/* Private headers needed by this file */
#include "H5Tprivate.h" /* Datatypes */
#include "H5Tprivate.h" /* Datatypes */
#include "H5Sprivate.h" /* Dataspace */

/**************************/
/* Library Private Macros */
Expand Down Expand Up @@ -89,6 +90,7 @@ H5_DLL herr_t H5Z_can_apply(hid_t dcpl_id, hid_t type_id);
H5_DLL herr_t H5Z_set_local(hid_t dcpl_id, hid_t type_id);
H5_DLL herr_t H5Z_can_apply_direct(const struct H5O_pline_t *pline);
H5_DLL herr_t H5Z_set_local_direct(const struct H5O_pline_t *pline);
H5_DLL htri_t H5Z_ignore_filters(hid_t dcpl_id, const H5T_t *type, const H5S_t *space);
H5_DLL H5Z_filter_info_t *H5Z_filter_info(const struct H5O_pline_t *pline,
H5Z_filter_t filter);
H5_DLL htri_t H5Z_filter_in_pline(const struct H5O_pline_t *pline, H5Z_filter_t filter);
Expand Down
98 changes: 98 additions & 0 deletions test/dsets.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ const char *FILENAME[] = {
#define DSET_FLETCHER32_NAME_3 "fletcher32_3"
#define DSET_SHUF_DEF_FLET_NAME "shuffle+deflate+fletcher32"
#define DSET_SHUF_DEF_FLET_NAME_2 "shuffle+deflate+fletcher32_2"
#define DSET_OPTIONAL_SCALAR "dataset_with_scalar_space"
#define DSET_OPTIONAL_VLEN "dataset_with_vlen_type"
#ifdef H5_HAVE_FILTER_SZIP
#define DSET_SZIP_NAME "szip"
#define DSET_SHUF_SZIP_FLET_NAME "shuffle+szip+fletcher32"
Expand Down Expand Up @@ -5770,6 +5772,101 @@ test_can_apply2(hid_t file)
} /* end test_can_apply2() */


/*-------------------------------------------------------------------------
* Function: test_optional_filters
*
* Purpose: Tests that H5Dcreate2 will not fail when a combination of
* type, space, etc... doesn't work for a filter and filter is
* optional.
*
* Return: Success: SUCCEED
* Failure: FAIL
*
* Programmer: Binh-Minh Ribler
* 24 July 2020
*
*-------------------------------------------------------------------------
*/
static herr_t
test_optional_filters(hid_t file)
{
unsigned int level = 9;
unsigned int cd_values[1] = {level};
size_t cd_nelmts = 1;
hsize_t dim1d[1]; /* Dataspace dimensions */
hid_t dsid = H5I_INVALID_HID; /* Dataset ID */
hid_t sid = H5I_INVALID_HID; /* Dataspace ID */
hid_t strtid = H5I_INVALID_HID; /* Datatype ID for string */
hid_t vlentid = H5I_INVALID_HID; /* Datatype ID for vlen */
hid_t dcplid = H5I_INVALID_HID; /* Dataspace creation property list ID */

TESTING("dataset with optional filters");

/* Create dcpl with special filter */
if((dcplid = H5Pcreate(H5P_DATASET_CREATE)) < 0) TEST_ERROR;

/* Create the datatype */
if((strtid = H5Tcreate(H5T_STRING, H5T_VARIABLE)) < 0) TEST_ERROR;

/* Create the data space */
if((sid = H5Screate(H5S_SCALAR)) < 0) TEST_ERROR;

/* The filter is optional. */
if(H5Pset_filter(dcplid, H5Z_FILTER_DEFLATE, H5Z_FLAG_OPTIONAL, cd_nelmts, cd_values) < 0)
TEST_ERROR;

/* Create dataset with optional filter */
if((dsid = H5Dcreate2(file, DSET_OPTIONAL_SCALAR, strtid, sid, H5P_DEFAULT, dcplid, H5P_DEFAULT)) < 0)
TEST_ERROR;

/* Close dataset */
if(H5Dclose(dsid) < 0) TEST_ERROR;

/* Close dataspace */
if(H5Sclose(sid) < 0) TEST_ERROR;

/* Close datatype */
if(H5Tclose(strtid) < 0) TEST_ERROR;

/* Set dataspace dimensions */
dim1d[0]=DIM1;

/* Create a non-scalar dataspace */
if((sid = H5Screate_simple(1, dim1d, NULL)) < 0) TEST_ERROR;

/* Create a vlen datatype */
if((vlentid = H5Tvlen_create(H5T_NATIVE_INT)) < 0) TEST_ERROR;

/* Create dataset with optional filter */
if((dsid = H5Dcreate2(file, DSET_OPTIONAL_VLEN, vlentid, sid, H5P_DEFAULT, dcplid, H5P_DEFAULT)) < 0)
TEST_ERROR;

/* Close dataset */
if(H5Dclose(dsid) < 0) TEST_ERROR;

/* Close dataspace */
if(H5Sclose(sid) < 0) TEST_ERROR;

/* Close datatype */
if(H5Tclose(vlentid) < 0) TEST_ERROR;

/* Close dataset creation property list */
if(H5Pclose(dcplid) < 0) TEST_ERROR;

PASSED();
return SUCCEED;

error:
H5E_BEGIN_TRY {
H5Dclose(dsid);
H5Sclose(sid);
H5Pclose(dcplid);
H5Tclose(strtid);
H5Tclose(vlentid);
} H5E_END_TRY;
return FAIL;
} /* end test_optional_filters() */


/*-------------------------------------------------------------------------
* Function: test_can_apply_szip
Expand Down Expand Up @@ -13862,6 +13959,7 @@ main(void)
nerrors += (test_missing_filter(file) < 0 ? 1 : 0);
nerrors += (test_can_apply(file) < 0 ? 1 : 0);
nerrors += (test_can_apply2(file) < 0 ? 1 : 0);
nerrors += (test_optional_filters(file) < 0 ? 1 : 0);
nerrors += (test_set_local(my_fapl) < 0 ? 1 : 0);
nerrors += (test_can_apply_szip(file) < 0 ? 1 : 0);
nerrors += (test_compare_dcpl(file) < 0 ? 1 : 0);
Expand Down

0 comments on commit 16349c5

Please sign in to comment.