From bb83e1ff9a7c424ed9e8a72d737703c9f6e78ad8 Mon Sep 17 00:00:00 2001 From: Raymond Lu Date: Thu, 5 Aug 2010 10:53:16 -0500 Subject: [svn-r19172] Bug fix for #1239 - The filter's public function CAN_APPLY should return htri_t not herr_t. To minimize the change of the library's behavior, in the function H5Z_prelude_callback of H5Z.c, if the return value of can_apply is FALSE and the filter is MANDATE, this function returns a FAILURE. If the return value is FALSE but the filter is OPTIONAL, this function returns a SUCCEED. During the IO, the filter will fail and return a size of zero. But the pipeline will skip this filter. Tested on jam, linew, and amani. Tested on jam with szip. --- release_docs/RELEASE.txt | 2 + src/H5Z.c | 21 +++-- src/H5Znbit.c | 6 +- src/H5Zpublic.h | 2 +- src/H5Zscaleoffset.c | 10 +-- src/H5Zszip.c | 6 +- test/dsets.c | 219 +++++++++++++++++++++++++++++++++++++++++++++-- test/links.c | 2 +- 8 files changed, 238 insertions(+), 30 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 6b4c671..4d4893f 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -114,6 +114,8 @@ Bug Fixes since HDF5-1.8.5 Library ------- + - Fixed the bug in the filter's public CAN_APPLY function. The return + value should be htri_t not herr_t (Bug #1239). (SLU - 2010/8/5) - Fixed a bug in the direct I/O driver that could render files with certain kinds of unaligned data unreadable or corrupt them. (NAF - 2010/07/28) - valgrind reported an error of copying data to itself when a new attribute diff --git a/src/H5Z.c b/src/H5Z.c index c295df3..ee46941 100644 --- a/src/H5Z.c +++ b/src/H5Z.c @@ -498,7 +498,7 @@ H5Z_prelude_callback(const H5O_pline_t *pline, hid_t dcpl_id, hid_t type_id, { H5Z_class2_t *fclass; /* Individual filter information */ size_t u; /* Local index variable */ - herr_t ret_value = SUCCEED; /* Return value */ + htri_t ret_value = TRUE; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5Z_prelude_callback) @@ -526,17 +526,16 @@ H5Z_prelude_callback(const H5O_pline_t *pline, hid_t dcpl_id, hid_t type_id, /* Check if there is a "can apply" callback */ if(fclass->can_apply) { /* Make callback to filter's "can apply" function */ - herr_t status = (fclass->can_apply)(dcpl_id, type_id, space_id); + htri_t status = (fclass->can_apply)(dcpl_id, type_id, space_id); - /* Check return value */ - if(status <= 0) { - /* Indicate filter can't apply to this combination of parameters */ - if(status == 0) - HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "filter parameters not appropriate") - /* Indicate error during filter callback */ - else - HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "error during user callback") - } /* end if */ + /* Indicate error during filter callback */ + if(status < 0) + HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "error during user callback") + + /* Indicate filter can't apply to this combination of parameters. + * If the filter is NOT optional, returns failure. */ + if(status == FALSE && !(pline->filter[u].flags & H5Z_FLAG_OPTIONAL)) + HGOTO_ERROR(H5E_PLINE, H5E_CANAPPLY, FAIL, "filter parameters not appropriate") } /* end if */ break; diff --git a/src/H5Znbit.c b/src/H5Znbit.c index 8f785a2..bcdd549 100644 --- a/src/H5Znbit.c +++ b/src/H5Znbit.c @@ -38,7 +38,7 @@ typedef struct { } parms_atomic; /* Local function prototypes */ -static herr_t H5Z_can_apply_nbit(hid_t dcpl_id, hid_t type_id, hid_t space_id); +static htri_t H5Z_can_apply_nbit(hid_t dcpl_id, hid_t type_id, hid_t space_id); static herr_t H5Z_set_local_nbit(hid_t dcpl_id, hid_t type_id, hid_t space_id); static size_t H5Z_filter_nbit(unsigned flags, size_t cd_nelmts, const unsigned cd_values[], size_t nbytes, size_t *buf_size, void **buf); @@ -129,11 +129,11 @@ static unsigned parms_index = 0; * *------------------------------------------------------------------------- */ -static herr_t +static htri_t H5Z_can_apply_nbit(hid_t UNUSED dcpl_id, hid_t type_id, hid_t UNUSED space_id) { const H5T_t *type; /* Datatype */ - herr_t ret_value = TRUE; /* Return value */ + htri_t ret_value = TRUE; /* Return value */ FUNC_ENTER_NOAPI(H5Z_can_apply_nbit, FAIL) diff --git a/src/H5Zpublic.h b/src/H5Zpublic.h index 44d2bbb..5d9b5ed 100644 --- a/src/H5Zpublic.h +++ b/src/H5Zpublic.h @@ -158,7 +158,7 @@ extern "C" { * The "can_apply" callback returns positive a valid combination, zero for an * invalid combination and negative for an error. */ -typedef herr_t (*H5Z_can_apply_func_t)(hid_t dcpl_id, hid_t type_id, hid_t space_id); +typedef htri_t (*H5Z_can_apply_func_t)(hid_t dcpl_id, hid_t type_id, hid_t space_id); /* * After the "can_apply" callbacks are checked for new datasets, the "set_local" diff --git a/src/H5Zscaleoffset.c b/src/H5Zscaleoffset.c index 5737702..4bfc8b1 100644 --- a/src/H5Zscaleoffset.c +++ b/src/H5Zscaleoffset.c @@ -41,7 +41,7 @@ enum H5Z_scaleoffset_t {t_bad=0, t_uchar=1, t_ushort, t_uint, t_ulong, t_ulong_l /* Local function prototypes */ static double H5Z_scaleoffset_rnd(double val); -static herr_t H5Z_can_apply_scaleoffset(hid_t dcpl_id, hid_t type_id, hid_t space_id); +static htri_t H5Z_can_apply_scaleoffset(hid_t dcpl_id, hid_t type_id, hid_t space_id); static enum H5Z_scaleoffset_t H5Z_scaleoffset_get_type(unsigned dtype_class, unsigned dtype_size, unsigned dtype_sign); static herr_t H5Z_scaleoffset_set_parms_fillval(H5P_genplist_t *dcpl_plist, @@ -611,13 +611,13 @@ H5Z_class2_t H5Z_SCALEOFFSET[1] = {{ * *------------------------------------------------------------------------- */ -static herr_t +static htri_t H5Z_can_apply_scaleoffset(hid_t UNUSED dcpl_id, hid_t type_id, hid_t UNUSED space_id) { const H5T_t *type; /* Datatype */ H5T_class_t dtype_class; /* Datatype's class */ H5T_order_t dtype_order; /* Datatype's endianness order */ - herr_t ret_value = TRUE; /* Return value */ + htri_t ret_value = TRUE; /* Return value */ FUNC_ENTER_NOAPI(H5Z_can_apply_scaleoffset, FAIL) @@ -640,9 +640,9 @@ H5Z_can_apply_scaleoffset(hid_t UNUSED dcpl_id, hid_t type_id, hid_t UNUSED spac /* Range check datatype's endianness order */ if(dtype_order != H5T_ORDER_LE && dtype_order != H5T_ORDER_BE) - HGOTO_ERROR(H5E_PLINE, H5E_BADTYPE, FAIL, "bad datatype endianness order") + HGOTO_ERROR(H5E_PLINE, H5E_BADTYPE, FALSE, "bad datatype endianness order") } else - HGOTO_ERROR(H5E_PLINE, H5E_BADTYPE, FAIL, "datatype class not supported by scaleoffset") + HGOTO_ERROR(H5E_PLINE, H5E_BADTYPE, FALSE, "datatype class not supported by scaleoffset") done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Zszip.c b/src/H5Zszip.c index ba8da6c..e74692c 100644 --- a/src/H5Zszip.c +++ b/src/H5Zszip.c @@ -34,7 +34,7 @@ #endif /* Local function prototypes */ -static herr_t H5Z_can_apply_szip(hid_t dcpl_id, hid_t type_id, hid_t space_id); +static htri_t H5Z_can_apply_szip(hid_t dcpl_id, hid_t type_id, hid_t space_id); static herr_t H5Z_set_local_szip(hid_t dcpl_id, hid_t type_id, hid_t space_id); static size_t H5Z_filter_szip (unsigned flags, size_t cd_nelmts, const unsigned cd_values[], size_t nbytes, size_t *buf_size, void **buf); @@ -76,13 +76,13 @@ H5Z_class2_t H5Z_SZIP[1] = {{ * *------------------------------------------------------------------------- */ -static herr_t +static htri_t H5Z_can_apply_szip(hid_t UNUSED dcpl_id, hid_t type_id, hid_t UNUSED space_id) { const H5T_t *type; /* Datatype */ unsigned dtype_size; /* Datatype's size (in bits) */ H5T_order_t dtype_order; /* Datatype's endianness order */ - herr_t ret_value = TRUE; /* Return value */ + htri_t ret_value = TRUE; /* Return value */ FUNC_ENTER_NOAPI(H5Z_can_apply_szip, FAIL) diff --git a/test/dsets.c b/test/dsets.c index a269545..da7d106 100644 --- a/test/dsets.c +++ b/test/dsets.c @@ -80,6 +80,7 @@ const char *FILENAME[] = { #define DSET_BOGUS_NAME "bogus" #define DSET_MISSING_NAME "missing" #define DSET_CAN_APPLY_NAME "can_apply" +#define DSET_CAN_APPLY_NAME2 "can_apply2" #define DSET_CAN_APPLY_SZIP_NAME "can_apply_szip" #define DSET_SET_LOCAL_NAME "set_local" #define DSET_SET_LOCAL_NAME_2 "set_local_2" @@ -117,6 +118,7 @@ const char *FILENAME[] = { #define H5Z_FILTER_SET_LOCAL_TEST 308 #define H5Z_FILTER_DEPREC 309 #define H5Z_FILTER_EXPAND 310 +#define H5Z_FILTER_CAN_APPLY_TEST2 311 /* Flags for testing filters */ #define DISABLE_FLETCHER32 0 @@ -199,10 +201,12 @@ double points_dbl[DSET_DIM1][DSET_DIM2], check_dbl[DSET_DIM1][DSET_DIM2]; /* Local prototypes for filter functions */ static size_t filter_bogus(unsigned int flags, size_t cd_nelmts, const unsigned int *cd_values, size_t nbytes, size_t *buf_size, void **buf); -static herr_t can_apply_bogus(hid_t dcpl_id, hid_t type_id, hid_t space_id); +static htri_t can_apply_bogus(hid_t dcpl_id, hid_t type_id, hid_t space_id); static herr_t set_local_bogus2(hid_t dcpl_id, hid_t type_id, hid_t space_id); static size_t filter_bogus2(unsigned int flags, size_t cd_nelmts, const unsigned int *cd_values, size_t nbytes, size_t *buf_size, void **buf); +static size_t filter_bogus3(unsigned int flags, size_t cd_nelmts, + const unsigned int *cd_values, size_t nbytes, size_t *buf_size, void **buf); static size_t filter_corrupt(unsigned int flags, size_t cd_nelmts, const unsigned int *cd_values, size_t nbytes, size_t *buf_size, void **buf); static size_t filter_expand(unsigned int flags, size_t cd_nelmts, @@ -1108,13 +1112,15 @@ const H5Z_class2_t H5Z_BOGUS[1] = {{ * *------------------------------------------------------------------------- */ -static herr_t +static htri_t can_apply_bogus(hid_t UNUSED dcpl_id, hid_t type_id, hid_t UNUSED space_id) { if(H5Tequal(type_id,H5T_NATIVE_DOUBLE)) return 0; - else + else if(H5Tequal(type_id,H5T_NATIVE_INT)) return 1; + else + return -1; } @@ -1254,6 +1260,31 @@ filter_bogus2(unsigned int flags, size_t cd_nelmts, return(nbytes); } + +/*------------------------------------------------------------------------- + * Function: filter_bogus3 + * + * Purpose: A bogus compression method that returns a failure. + * + * Return: Success: Data chunk size + * + * Failure: 0 + * + * Programmer: Raymond Lu + * 4 August 2010 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static size_t +filter_bogus3(unsigned int UNUSED flags, size_t UNUSED cd_nelmts, + const unsigned int UNUSED *cd_values, size_t nbytes, + size_t UNUSED *buf_size, void UNUSED **buf) +{ + return 0; +} + /* This message derives from H5Z */ const H5Z_class2_t H5Z_CORRUPT[1] = {{ H5Z_CLASS_T_VERS, /* H5Z_class_t version */ @@ -4868,7 +4899,9 @@ const H5Z_class2_t H5Z_CAN_APPLY_TEST[1] = {{ * Function: test_can_apply * * Purpose: Tests library behavior when filter indicates it can't - * apply to certain combinations of creation parameters + * apply to certain combinations of creation parameters. + * The filter is mandate. If the CAN_APPLY callback function + * indicates wrong datatype, the dataset creation should fail. * * Return: Success: 0 * Failure: -1 @@ -4907,6 +4940,7 @@ test_can_apply(hid_t file) printf(" Line %d: Can't register 'can apply' filter\n",__LINE__); goto error; } + /* The filter is mandate. */ if(H5Pset_filter(dcpl, H5Z_FILTER_CAN_APPLY_TEST, 0, (size_t)0, NULL) < 0) { H5_FAILED(); printf(" Line %d: Can't set bogus filter\n",__LINE__); @@ -4921,7 +4955,8 @@ test_can_apply(hid_t file) } /* end if */ /* Create new dataset */ - /* (Should fail because the 'can apply' filter should indicate inappropriate combination) */ + /* (Should fail because the 'can apply' function should indicate inappropriate + * combination. And the filter is mandate.) */ H5E_BEGIN_TRY { dsid = H5Dcreate2(file, DSET_CAN_APPLY_NAME, H5T_NATIVE_DOUBLE, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT); } H5E_END_TRY; @@ -4932,6 +4967,17 @@ test_can_apply(hid_t file) goto error; } /* end if */ + /* (Should fail because the 'can apply' function should fail) */ + H5E_BEGIN_TRY { + dsid = H5Dcreate2(file, DSET_CAN_APPLY_NAME, H5T_NATIVE_FLOAT, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT); + } H5E_END_TRY; + if(dsid >=0) { + H5_FAILED(); + printf(" Line %d: Shouldn't have created dataset!\n",__LINE__); + H5Dclose(dsid); + goto error; + } /* end if */ + /* Create new dataset */ if((dsid = H5Dcreate2(file, DSET_CAN_APPLY_NAME, H5T_NATIVE_INT, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) { H5_FAILED(); @@ -5018,6 +5064,166 @@ error: return -1; } /* end test_can_apply() */ +/* This message derives from H5Z */ +const H5Z_class2_t H5Z_CAN_APPLY_TEST2[1] = {{ + H5Z_CLASS_T_VERS, + H5Z_FILTER_CAN_APPLY_TEST2, /* Filter id number */ + 1, 1, + "can_apply_test", /* Filter name for debugging */ + can_apply_bogus, /* The "can apply" callback */ + NULL, /* The "set local" callback */ + filter_bogus3, /* The actual filter function */ +}}; + + +/*------------------------------------------------------------------------- + * Function: test_can_apply2 + * + * Purpose: Tests library behavior when an optional filter indicates + * it can't apply to certain combinations of creation + * parameters. The filter function FILTER_BOGUS3 does nothing + * than returning a failure. Because the filter is optional, + * the library skips the filter even though the CAN_APPLY_BOGUS + * indicates the datatype DOUBLE can't apply to the dataset. + * + * Return: Success: 0 + * Failure: -1 + * + * Programmer: Raymond Lu + * 4 August 2010 + * + *------------------------------------------------------------------------- + */ +static herr_t +test_can_apply2(hid_t file) +{ + hid_t dsid; /* Dataset ID */ + hid_t sid; /* Dataspace ID */ + hid_t dcpl; /* Dataspace creation property list ID */ + const hsize_t dims[2] = {DSET_DIM1, DSET_DIM2}; /* Dataspace dimensions */ + const hsize_t chunk_dims[2] = {2, 25}; /* Chunk dimensions */ + hsize_t dset_size; /* Dataset size */ + size_t i,j; /* Local index variables */ + + TESTING("dataset filter 'can apply' callback second"); + + /* Create dcpl with special filter */ + if((dcpl = H5Pcreate(H5P_DATASET_CREATE)) < 0) { + H5_FAILED(); + printf(" Line %d: Can't create dcpl\n",__LINE__); + goto error; + } /* end if */ + if(H5Pset_chunk(dcpl, 2, chunk_dims) < 0) { + H5_FAILED(); + printf(" Line %d: Can't set chunk sizes\n",__LINE__); + goto error; + } /* end if */ + if(H5Zregister (H5Z_CAN_APPLY_TEST2) < 0) { + H5_FAILED(); + printf(" Line %d: Can't register 'can apply' filter\n",__LINE__); + goto error; + } + /* The filter is optional. */ + if(H5Pset_filter(dcpl, H5Z_FILTER_CAN_APPLY_TEST2, H5Z_FLAG_OPTIONAL, (size_t)0, NULL) < 0) { + H5_FAILED(); + printf(" Line %d: Can't set bogus filter\n",__LINE__); + goto error; + } + + /* Create the data space */ + if((sid = H5Screate_simple(2, dims, NULL)) < 0) { + H5_FAILED(); + printf(" Line %d: Can't open dataspace\n",__LINE__); + goto error; + } /* end if */ + + /* Create new dataset */ + if((dsid = H5Dcreate2(file, DSET_CAN_APPLY_NAME2, H5T_NATIVE_DOUBLE, sid, H5P_DEFAULT, dcpl, H5P_DEFAULT)) < 0) { + H5_FAILED(); + printf(" Line %d: Can't create dataset\n",__LINE__); + goto error; + } /* end if */ + + /* Write data */ + if(H5Dwrite(dsid, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, points) < 0) { + H5_FAILED(); + printf(" Line %d: Error writing dataset data\n",__LINE__); + goto error; + } /* end if */ + + /* Flush the file (to clear the cache) */ + if(H5Fflush(file, H5F_SCOPE_GLOBAL) < 0) { + H5_FAILED(); + printf(" Line %d: Error flushing file\n",__LINE__); + goto error; + } /* end if */ + + /* Query the dataset's size on disk */ + if((dset_size=H5Dget_storage_size(dsid))==0) { + H5_FAILED(); + printf(" Line %d: Error querying dataset size\n",__LINE__); + goto error; + } /* end if */ + + /* Verify that the size indicates data is uncompressed */ + if((H5Tget_size(H5T_NATIVE_DOUBLE)*dims[0]*dims[1])!=dset_size) { + H5_FAILED(); + printf(" Line %d: Incorrect dataset size: %lu\n",__LINE__,(unsigned long)dset_size); + goto error; + } /* end if */ + + /* Read data */ + if(H5Dread(dsid, H5T_NATIVE_INT, H5S_ALL, H5S_ALL, H5P_DEFAULT, check) < 0) { + H5_FAILED(); + printf(" Line %d: Error reading dataset data\n",__LINE__); + goto error; + } /* end if */ + + /* Compare data */ + /* Check that the values read are the same as the values written */ + for(i=0; i<(size_t)dims[0]; i++) { + for(j=0; j<(size_t)dims[1]; j++) { + if(points[i][j] != check[i][j]) { + H5_FAILED(); + printf(" Line %d: Read different values than written.\n",__LINE__); + printf(" At index %lu,%lu\n", (unsigned long)(i), (unsigned long)(j)); + printf(" At original: %d\n",points[i][j]); + printf(" At returned: %d\n",check[i][j]); + goto error; + } /* end if */ + } /* end for */ + } /* end for */ + + /* Close dataset */ + if(H5Dclose(dsid) < 0) { + H5_FAILED(); + printf(" Line %d: Can't close dataset\n",__LINE__); + goto error; + } /* end if */ + + /* Close dataspace */ + if(H5Sclose(sid) < 0) { + H5_FAILED(); + printf(" Line %d: Can't close dataspace\n",__LINE__); + goto error; + } /* end if */ + + /* Close dataset creation property list */ + if(H5Pclose(dcpl) < 0) { + H5_FAILED(); + printf(" Line %d: Can't close dcpl\n",__LINE__); + goto error; + } /* end if */ + + + PASSED(); + return 0; + +error: + return -1; +} /* end test_can_apply2() */ + + /*------------------------------------------------------------------------- * Function: test_can_apply_szip @@ -6384,7 +6590,7 @@ error: #ifndef H5_NO_DEPRECATED_SYMBOLS /* Empty can_apply and set_local callbacks */ -static herr_t +static htri_t can_apply_deprec(hid_t UNUSED dcpl_id, hid_t UNUSED type_id, hid_t UNUSED space_id) { return 1; @@ -7590,6 +7796,7 @@ main(void) nerrors += (test_userblock_offset(envval, my_fapl) < 0 ? 1 : 0); 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_set_local(my_fapl) < 0 ? 1 : 0); nerrors += (test_can_apply_szip(file) < 0 ? 1 : 0); nerrors += (test_compare_dcpl(file) < 0 ? 1 : 0); diff --git a/test/links.c b/test/links.c index c402e20..0451c28 100644 --- a/test/links.c +++ b/test/links.c @@ -8879,7 +8879,7 @@ static enum { LFS_DECODED } link_filter_state; -static herr_t link_filter_can_apply(hid_t dcpl_id, hid_t type_id, hid_t space_id) +static htri_t link_filter_can_apply(hid_t dcpl_id, hid_t type_id, hid_t space_id) { if(dcpl_id >= 0 || type_id >= 0 || space_id >= 0) return -1; -- cgit v0.12