diff options
author | Neil Fortner <nfortne2@hdfgroup.org> | 2009-09-25 18:11:40 (GMT) |
---|---|---|
committer | Neil Fortner <nfortne2@hdfgroup.org> | 2009-09-25 18:11:40 (GMT) |
commit | a2faf5621f38ff5d745acbc254dfbded02664e28 (patch) | |
tree | 5d74903f1cd5abd103c444d8d6ca6b086763c8c8 | |
parent | 51d3e9b1730437a4f9718d71909d311174acdfa0 (diff) | |
download | hdf5-a2faf5621f38ff5d745acbc254dfbded02664e28.zip hdf5-a2faf5621f38ff5d745acbc254dfbded02664e28.tar.gz hdf5-a2faf5621f38ff5d745acbc254dfbded02664e28.tar.bz2 |
[svn-r17531] Purpose: Fix bug 1555
Description:
At several places in H5Pint.c properties were being compared using memcmp, not
using the registered "cmp" callback. This could cause memory checkers to
report uninitialized memory errors, and could conceivably cause runtime errors
if memcmp reported false equality (for example if a value pointed to by the
property changed).
The code has been changed to always use the cmp callback. Tests have been added that check this in all the places that previously used memcmp.
Tested: jam, linew, smirom (h5committest)
-rw-r--r-- | src/H5Pint.c | 8 | ||||
-rw-r--r-- | test/tgenprop.c | 53 |
2 files changed, 52 insertions, 9 deletions
diff --git a/src/H5Pint.c b/src/H5Pint.c index 56f1929..364cab2 100644 --- a/src/H5Pint.c +++ b/src/H5Pint.c @@ -277,7 +277,7 @@ H5P_do_prop_cb1(H5SL_t *slist, H5P_genprop_t *prop, H5P_prp_cb1_t cb) HGOTO_ERROR (H5E_PLIST, H5E_CANTINIT, FAIL,"Property callback failed"); /* Check if the property value changed */ - if(HDmemcmp(tmp_value,prop->value,prop->size)) { + if((prop->cmp)(tmp_value,prop->value,prop->size)) { /* Make a copy of the class's property */ if((pcopy=H5P_dup_prop(prop,H5P_PROP_WITHIN_LIST)) == NULL) HGOTO_ERROR (H5E_PLIST, H5E_CANTCOPY, FAIL,"Can't copy property"); @@ -2264,7 +2264,7 @@ H5P_set(H5P_genplist_t *plist, const char *name, const void *value) HGOTO_ERROR(H5E_PLIST, H5E_CANTINIT, FAIL, "can't set property value"); } /* end if */ - if(HDmemcmp(tmp_value,prop->value,prop->size)) { + if((prop->cmp)(tmp_value,prop->value,prop->size)) { /* Make a copy of the class's property */ if((pcopy=H5P_dup_prop(prop,H5P_PROP_WITHIN_LIST)) == NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTCOPY,FAIL,"Can't copy property"); @@ -2282,7 +2282,7 @@ H5P_set(H5P_genplist_t *plist, const char *name, const void *value) } /* end if */ /* No 'set' callback, just copy value */ else { - if(HDmemcmp(value,prop->value,prop->size)) { + if((prop->cmp)(value,prop->value,prop->size)) { /* Make a copy of the class's property */ if((pcopy=H5P_dup_prop(prop,H5P_PROP_WITHIN_LIST)) == NULL) HGOTO_ERROR(H5E_PLIST,H5E_CANTCOPY,FAIL,"Can't copy property"); @@ -3614,7 +3614,7 @@ H5P_get(const H5P_genplist_t *plist, const char *name, void *value) HGOTO_ERROR(H5E_PLIST, H5E_CANTINIT, FAIL, "can't set property value"); } /* end if */ - if(HDmemcmp(tmp_value,prop->value,prop->size)) { + if((prop->cmp)(tmp_value,prop->value,prop->size)) { H5P_genprop_t *pcopy; /* Copy of property to insert into skip list */ /* Make a copy of the class's property */ diff --git a/test/tgenprop.c b/test/tgenprop.c index 931133e..f87d4ec 100644 --- a/test/tgenprop.c +++ b/test/tgenprop.c @@ -891,6 +891,7 @@ typedef struct { /* Global variables for Callback information */ prop_cb_info prop1_cb_info; /* Callback statistics for property #1 */ prop_cb_info prop2_cb_info; /* Callback statistics for property #2 */ +prop_cb_info prop3_cb_info; /* Callback statistics for property #3 */ /**************************************************************** ** @@ -974,12 +975,26 @@ test_genprop_prop_cop_cb1(const char *name, size_t size, void *value) ** ****************************************************************/ static int -test_genprop_prop_cmp_cb1(const void UNUSED *value1, const void UNUSED *value2, size_t UNUSED size) +test_genprop_prop_cmp_cb1(const void *value1, const void *value2, size_t size) { /* Set the information from the comparison call */ prop1_cb_info.cmp_count++; - return(0); + return(HDmemcmp(value1, value2, size)); +} + +/**************************************************************** +** +** test_genprop_prop_cmp_cb3(): Property comparison callback for test_genprop_list_callback +** +****************************************************************/ +static int +test_genprop_prop_cmp_cb3(const void *value1, const void *value2, size_t size) +{ + /* Set the information from the comparison call */ + prop3_cb_info.cmp_count++; + + return(HDmemcmp(value1, value2, size)); } /**************************************************************** @@ -1036,6 +1051,7 @@ test_genprop_list_callback(void) int prop1_new_value=20; /* Property #1 new value */ float prop2_value; /* Value for property #2 */ char prop3_value[10];/* Property #3 value */ + char prop3_new_value[10]="10 chairs"; /* Property #3 new value */ double prop4_value; /* Property #4 value */ struct { /* Struct for callbacks */ int count; @@ -1058,8 +1074,8 @@ test_genprop_list_callback(void) ret = H5Pregister2(cid1, PROP2_NAME, PROP2_SIZE, PROP2_DEF_VALUE, NULL, NULL, NULL,test_genprop_prop_del_cb2,NULL, NULL, NULL); CHECK_I(ret, "H5Pregister2"); - /* Insert third property into class (with no callbacks) */ - ret = H5Pregister2(cid1, PROP3_NAME, PROP3_SIZE, PROP3_DEF_VALUE, NULL, NULL, NULL, NULL, NULL, NULL, NULL); + /* Insert third property into class (with only compare callback) */ + ret = H5Pregister2(cid1, PROP3_NAME, PROP3_SIZE, PROP3_DEF_VALUE, NULL, NULL, NULL, NULL, NULL, test_genprop_prop_cmp_cb3, NULL); CHECK_I(ret, "H5Pregister2"); /* Insert fourth property into class (with no callbacks) */ @@ -1078,11 +1094,19 @@ test_genprop_list_callback(void) /* Initialize callback information for properties tracked */ HDmemset(&prop1_cb_info,0,sizeof(prop_cb_info)); HDmemset(&prop2_cb_info,0,sizeof(prop_cb_info)); + HDmemset(&prop3_cb_info,0,sizeof(prop_cb_info)); /* Create a property list from the class */ lid1 = H5Pcreate(cid1); CHECK_I(lid1, "H5Pcreate"); + /* The compare callback should have been called once on property 1 (to check + * if the create callback modified the value) */ + VERIFY(prop1_cb_info.cmp_count, 1, "H5Pequal"); + /* The compare callback should not have been called on property 3, as there + * is no create callback */ + VERIFY(prop3_cb_info.cmp_count, 0, "H5Pequal"); + /* Verify creation callback information for properties tracked */ VERIFY(prop1_cb_info.crt_count, 1, "H5Pcreate"); if(HDstrcmp(prop1_cb_info.crt_name, PROP1_NAME)!=0) @@ -1094,6 +1118,9 @@ test_genprop_list_callback(void) ret = H5Pget(lid1, PROP1_NAME,&prop1_value); CHECK_I(ret, "H5Pget"); VERIFY(prop1_value, *PROP1_DEF_VALUE, "H5Pget"); + /* The compare callback should have been called once (to check if the get + * callback modified the value) */ + VERIFY(prop1_cb_info.cmp_count, 2, "H5Pequal"); ret = H5Pget(lid1, PROP2_NAME,&prop2_value); CHECK_I(ret, "H5Pget"); /* Verify the floating-poing value in this way to avoid compiler warning. */ @@ -1106,6 +1133,9 @@ test_genprop_list_callback(void) CHECK_I(ret, "H5Pget"); if(HDmemcmp(&prop3_value, PROP3_DEF_VALUE, PROP3_SIZE)!=0) TestErrPrintf("Property #3 doesn't match!, line=%d\n",__LINE__); + /* The compare callback should not have been called, as there is no get + * callback for this property */ + VERIFY(prop3_cb_info.cmp_count, 0, "H5Pequal"); ret = H5Pget(lid1, PROP4_NAME,&prop4_value); CHECK_I(ret, "H5Pget"); /* Verify the floating-poing value in this way to avoid compiler warning. */ @@ -1133,6 +1163,18 @@ test_genprop_list_callback(void) if(HDmemcmp(prop1_cb_info.set_value,&prop1_new_value, PROP1_SIZE)!=0) TestErrPrintf("Property #1 value doesn't match!, line=%d\n",__LINE__); + /* The compare callback should have been called once (to check if the new + * value needed to be copied onto the property list) */ + VERIFY(prop1_cb_info.cmp_count, 3, "H5Pequal"); + + /* Set value of property #3 to different value */ + ret = H5Pset(lid1, PROP3_NAME,prop3_new_value); + CHECK_I(ret, "H5Pset"); + + /* The compare callback should have been called once (to check if the new + * value needed to be copied onto the property list) */ + VERIFY(prop3_cb_info.cmp_count, 1, "H5Pequal"); + /* Check new value of tracked properties */ ret = H5Pget(lid1, PROP1_NAME,&prop1_value); CHECK_I(ret, "H5Pget"); @@ -1178,7 +1220,8 @@ test_genprop_list_callback(void) VERIFY(ret, 1, "H5Pequal"); /* Verify compare callback information for properties tracked */ - VERIFY(prop1_cb_info.cmp_count, 1, "H5Pequal"); + VERIFY(prop1_cb_info.cmp_count, 4, "H5Pequal"); + VERIFY(prop3_cb_info.cmp_count, 2, "H5Pequal"); /* Close first list */ ret = H5Pclose(lid1); |