diff options
| -rw-r--r-- | release_docs/RELEASE.txt | 2 | ||||
| -rw-r--r-- | src/H5Dio.c | 2 | ||||
| -rw-r--r-- | src/H5Dpkg.h | 2 | ||||
| -rw-r--r-- | src/H5Dscatgath.c | 20 | ||||
| -rw-r--r-- | src/H5T.c | 12 | ||||
| -rw-r--r-- | src/H5Tconv.c | 56 | ||||
| -rw-r--r-- | src/H5Tpkg.h | 4 | ||||
| -rw-r--r-- | src/H5Tprivate.h | 7 | ||||
| -rw-r--r-- | test/dtypes.c | 170 |
9 files changed, 228 insertions, 47 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 4d012c0..f52dd75 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -109,6 +109,8 @@ Bug Fixes since HDF5-1.8.0 release Library ------- + - Fixed an issue that could cause data to be improperly overwritten + during compound type conversion. (NAF - 2008/09/19) - Fixed pointer alignment violations that could occur during vlen conversion. (NAF - 2008/09/16) - Fixed problem where library could cause a segmentation fault when diff --git a/src/H5Dio.c b/src/H5Dio.c index 45fa21a..b7c1d72 100644 --- a/src/H5Dio.c +++ b/src/H5Dio.c @@ -745,7 +745,7 @@ H5D_typeinfo_init(const H5D_t *dset, const H5D_dxpl_cache_t *dxpl_cache, type_info->is_conv_noop = H5T_path_noop(type_info->tpath); type_info->is_xform_noop = H5Z_xform_noop(dxpl_cache->data_xform_prop); if(type_info->is_xform_noop && type_info->is_conv_noop) { - type_info->cmpd_subset = H5T_SUBSET_FALSE; + type_info->cmpd_subset = NULL; type_info->need_bkg = H5T_BKG_NO; } /* end if */ else { diff --git a/src/H5Dpkg.h b/src/H5Dpkg.h index 9f02dbc..f6732ff 100644 --- a/src/H5Dpkg.h +++ b/src/H5Dpkg.h @@ -86,7 +86,7 @@ typedef struct H5D_type_info_t { size_t max_type_size; /* Size of largest source/destination type */ hbool_t is_conv_noop; /* Whether the type conversion is a NOOP */ hbool_t is_xform_noop; /* Whether the data transform is a NOOP */ - H5T_subset_t cmpd_subset; /* Whether (and which) the source/destination datatypes are compound subsets of one another */ + const H5T_subset_info_t *cmpd_subset; /* Info related to the compound subset conversion functions */ H5T_bkg_t need_bkg; /* Type of background buf needed */ size_t request_nelmts; /* Requested strip mine */ uint8_t *tconv_buf; /* Datatype conv buffer */ diff --git a/src/H5Dscatgath.c b/src/H5Dscatgath.c index cb07708..867303f 100644 --- a/src/H5Dscatgath.c +++ b/src/H5Dscatgath.c @@ -519,7 +519,7 @@ H5D_scatgath_read(const H5D_io_info_t *io_info, const H5D_type_info_t *type_info * and no conversion is needed, copy the data directly into user's buffer and * bypass the rest of steps. */ - if(H5T_SUBSET_FALSE != type_info->cmpd_subset) { + if(type_info->cmpd_subset && H5T_SUBSET_FALSE != type_info->cmpd_subset->subset) { if(H5D_compound_opt_read(smine_nelmts, mem_space, &mem_iter, dxpl_cache, type_info, buf /*out*/) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "datatype conversion failed") @@ -649,7 +649,8 @@ H5D_scatgath_write(const H5D_io_info_t *io_info, const H5D_type_info_t *type_inf * is a subset of the destination, the optimization is done in conversion * function H5T_conv_struct_opt to protect the background data. */ - if(H5T_SUBSET_DST == type_info->cmpd_subset) { + if(type_info->cmpd_subset && H5T_SUBSET_DST == type_info->cmpd_subset->subset + && type_info->dst_type_size == type_info->cmpd_subset->copy_size) { if(H5D_compound_opt_write(smine_nelmts, type_info) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "datatype conversion failed") } /* end if */ @@ -745,7 +746,7 @@ H5D_compound_opt_read(size_t nelmts, const H5S_t *space, hsize_t *off = NULL; /* Pointer to sequence offsets */ size_t _len[H5D_IO_VECTOR_SIZE]; /* Array to store sequence lengths */ size_t *len = NULL; /* Pointer to sequence lengths */ - size_t src_stride, dst_stride, type_size; + size_t src_stride, dst_stride, copy_size; herr_t ret_value = SUCCEED; /*return value */ FUNC_ENTER_NOAPI_NOINIT(H5D_compound_opt_read) @@ -756,6 +757,9 @@ H5D_compound_opt_read(size_t nelmts, const H5S_t *space, HDassert(iter); HDassert(dxpl_cache); HDassert(type_info); + HDassert(type_info->cmpd_subset); + HDassert(H5T_SUBSET_SRC == type_info->cmpd_subset->subset || + H5T_SUBSET_DST == type_info->cmpd_subset->subset); HDassert(user_buf); /* Allocate the vector I/O arrays */ @@ -774,12 +778,8 @@ H5D_compound_opt_read(size_t nelmts, const H5S_t *space, src_stride = type_info->src_type_size; dst_stride = type_info->dst_type_size; - if(H5T_SUBSET_SRC == type_info->cmpd_subset) - type_size = src_stride; - else { - HDassert(H5T_SUBSET_DST == type_info->cmpd_subset); - type_size = dst_stride; - } /* end else */ + /* Get the size, in bytes, to copy for each element */ + copy_size = type_info->cmpd_subset->copy_size; /* Loop until all elements are written */ xdbuf = type_info->tconv_buf; @@ -811,7 +811,7 @@ H5D_compound_opt_read(size_t nelmts, const H5S_t *space, /* Copy the data into the right place. */ for(i = 0; i < curr_nelmts; i++) { - HDmemmove(xubuf, xdbuf, type_size); + HDmemmove(xubuf, xdbuf, copy_size); /* Update pointers */ xdbuf += src_stride; @@ -4526,19 +4526,23 @@ H5T_path_noop(const H5T_path_t *p) * TYPE5 E; * }; * - * Return: One of the values of H5T_subset_t (can't fail). + * 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. * * Programmer: Raymond Lu * 8 June 2007 * - * Modifications: + * Modifications: Neil Fortner + * 19 September 2008 + * Changed return value to H5T_subset_info_t + * (to allow it to return copy_size) * *------------------------------------------------------------------------- */ -H5T_subset_t +H5T_subset_info_t * H5T_path_compound_subset(const H5T_path_t *p) { - H5T_subset_t ret_value = FALSE; + H5T_subset_info_t *ret_value = NULL; FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5T_path_compound_subset); diff --git a/src/H5Tconv.c b/src/H5Tconv.c index 276d05a..b02e72a 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -883,7 +883,7 @@ typedef struct H5T_conv_struct_t { hid_t *src_memb_id; /*source member type ID's */ hid_t *dst_memb_id; /*destination member type ID's */ H5T_path_t **memb_path; /*conversion path for each member */ - H5T_subset_t smembs_subset; /*are source and dest members a subset of each other? */ + H5T_subset_info_t subset_info; /*info related to compound subsets */ } H5T_conv_struct_t; /* Conversion data for H5T_conv_enum() */ @@ -1827,7 +1827,8 @@ H5T_conv_struct_init(H5T_t *src, H5T_t *dst, H5T_cdata_t *cdata, hid_t dxpl_id) /* The flag of special optimization to indicate if source members and destination * members are a subset of each other. Initialize it to FALSE */ - priv->smembs_subset = H5T_SUBSET_FALSE; + priv->subset_info.subset = H5T_SUBSET_FALSE; + priv->subset_info.copy_size = 0; /* * Insure that members are sorted. @@ -1899,23 +1900,41 @@ H5T_conv_struct_init(H5T_t *src, H5T_t *dst, H5T_cdata_t *cdata, hid_t dxpl_id) cdata->need_bkg = H5T_BKG_YES; if(src_nmembs < dst_nmembs) { - priv->smembs_subset = H5T_SUBSET_SRC; + priv->subset_info.subset = H5T_SUBSET_SRC; for(i = 0; i < src_nmembs; i++) { /* If any of source members doesn't have counterpart in the same * order or there's conversion between members, don't do the * optimization. */ - if(src2dst[i] != i || (src->shared->u.compnd.memb[i].offset != dst->shared->u.compnd.memb[i].offset) || (priv->memb_path[i])->is_noop == FALSE) - priv->smembs_subset = H5T_SUBSET_FALSE; + if(src2dst[i] != i || (src->shared->u.compnd.memb[i].offset != dst->shared->u.compnd.memb[i].offset) || (priv->memb_path[i])->is_noop == FALSE) { + priv->subset_info.subset = H5T_SUBSET_FALSE; + break; + } /* end if */ } /* end for */ + /* Compute the size of the data to be copied for each element. It + * may be smaller than either src or dst if there is extra space at + * the end of src. + */ + if(priv->subset_info.subset == H5T_SUBSET_SRC) + priv->subset_info.copy_size = src->shared->u.compnd.memb[src_nmembs-1].offset + + src->shared->u.compnd.memb[src_nmembs-1].size; } else if(dst_nmembs < src_nmembs) { - priv->smembs_subset = H5T_SUBSET_DST; + priv->subset_info.subset = H5T_SUBSET_DST; for(i = 0; i < dst_nmembs; i++) { /* If any of source members doesn't have counterpart in the same order or * there's conversion between members, don't do the optimization. */ - if(src2dst[i] != i || (src->shared->u.compnd.memb[i].offset != dst->shared->u.compnd.memb[i].offset) || (priv->memb_path[i])->is_noop == FALSE) - priv->smembs_subset = H5T_SUBSET_FALSE; + if(src2dst[i] != i || (src->shared->u.compnd.memb[i].offset != dst->shared->u.compnd.memb[i].offset) || (priv->memb_path[i])->is_noop == FALSE) { + priv->subset_info.subset = H5T_SUBSET_FALSE; + break; + } } /* end for */ + /* Compute the size of the data to be copied for each element. It + * may be smaller than either src or dst if there is extra space at + * the end of dst. + */ + if(priv->subset_info.subset == H5T_SUBSET_DST) + priv->subset_info.copy_size = dst->shared->u.compnd.memb[dst_nmembs-1].offset + + dst->shared->u.compnd.memb[dst_nmembs-1].size; } else /* If the numbers of source and dest members are equal and no conversion is needed, * the case should have been handled as noop earlier in H5Dio.c. */ ; @@ -1945,14 +1964,15 @@ done: * TYPE5 E; * }; * - * Return: One of the value from H5T_subset_t. + * Return: A pointer to the subset info struct in p. Points directly + * into the structure. * * Programmer: Raymond Lu * 8 June 2007 * *------------------------------------------------------------------------- */ -H5T_subset_t +H5T_subset_info_t * H5T_conv_struct_subset(const H5T_cdata_t *cdata) { H5T_conv_struct_t *priv; @@ -1964,7 +1984,7 @@ H5T_conv_struct_subset(const H5T_cdata_t *cdata) priv = (H5T_conv_struct_t *)(cdata->priv); - FUNC_LEAVE_NOAPI(priv->smembs_subset) + FUNC_LEAVE_NOAPI((H5T_subset_info_t *) &priv->subset_info) } /* end H5T_conv_struct_subset() */ @@ -2388,22 +2408,12 @@ H5T_conv_struct_opt(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, buf_stride = src->shared->size; } - if(priv->smembs_subset == H5T_SUBSET_SRC || priv->smembs_subset == H5T_SUBSET_DST) { + if(priv->subset_info.subset == H5T_SUBSET_SRC || priv->subset_info.subset == H5T_SUBSET_DST) { /* If the optimization flag is set to indicate source members are a subset and * in the top of the destination, simply copy the source members to background buffer. */ xbuf = buf; xbkg = bkg; - if(dst->shared->size <= src->shared->size) - /* This is to deal with a very special situation when the fields and their - * offset for both source and destination are identical but the datatype - * sizes of source and destination are different. The library still - * considers these two types different and does conversion. It happens - * in table API test (hdf5/hl/test/test_table.c) when a table field is - * deleted. - */ - copy_size = dst->shared->size; - else - copy_size = src->shared->size; + copy_size = priv->subset_info.copy_size; for (elmtno=0; elmtno<nelmts; elmtno++) { HDmemmove(xbkg, xbuf, copy_size); diff --git a/src/H5Tpkg.h b/src/H5Tpkg.h index a534f1b..7ef4b63 100644 --- a/src/H5Tpkg.h +++ b/src/H5Tpkg.h @@ -288,7 +288,7 @@ typedef enum H5T_sort_t { typedef struct H5T_cmemb_t { char *name; /*name of this member */ size_t offset; /*offset from beginning of struct */ - size_t size; /*total size: dims * type_size */ + size_t size; /*size of this member */ struct H5T_t *type; /*type of this member */ } H5T_cmemb_t; @@ -1380,7 +1380,7 @@ H5_DLL herr_t H5T_insert(H5T_t *parent, const char *name, size_t offset, const H5T_t *member); H5_DLL size_t H5T_get_member_size(const H5T_t *dt, unsigned membno); H5_DLL htri_t H5T_is_packed(const H5T_t *dt); -H5_DLL H5T_subset_t H5T_conv_struct_subset(const H5T_cdata_t *cdata); +H5_DLL H5T_subset_info_t *H5T_conv_struct_subset(const H5T_cdata_t *cdata); /* Enumerated type functions */ H5_DLL H5T_t *H5T_enum_create(const H5T_t *parent); diff --git a/src/H5Tprivate.h b/src/H5Tprivate.h index f8f2abc..9189f04 100644 --- a/src/H5Tprivate.h +++ b/src/H5Tprivate.h @@ -92,6 +92,11 @@ typedef enum { H5T_SUBSET_CAP /* Must be the last value */ } H5T_subset_t; +typedef struct H5T_subset_info_t { + H5T_subset_t subset; /* See above */ + size_t copy_size; /* Size in bytes, to copy for each element */ +} H5T_subset_info_t; + /* Forward declarations for prototype arguments */ struct H5O_t; @@ -119,7 +124,7 @@ H5_DLL H5T_path_t *H5T_path_find(const H5T_t *src, const H5T_t *dst, const char *name, H5T_conv_t func, hid_t dxpl_id, hbool_t is_api); H5_DLL hbool_t H5T_path_noop(const H5T_path_t *p); H5_DLL H5T_bkg_t H5T_path_bkg(const H5T_path_t *p); -H5_DLL H5T_subset_t H5T_path_compound_subset(const H5T_path_t *p); +H5_DLL H5T_subset_info_t *H5T_path_compound_subset(const H5T_path_t *p); H5_DLL herr_t H5T_convert(H5T_path_t *tpath, hid_t src_id, hid_t dst_id, size_t nelmts, size_t buf_stride, size_t bkg_stride, void *buf, void *bkg, hid_t dset_xfer_plist); diff --git a/test/dtypes.c b/test/dtypes.c index f688371..674c42f 100644 --- a/test/dtypes.c +++ b/test/dtypes.c @@ -2683,10 +2683,10 @@ test_compound_14(void) } /* end if */ rdata1.c1 = rdata1.c2 = 0; - if(rdata1.str) free(rdata1.str); + if(rdata1.str) HDfree(rdata1.str); rdata2.c1 = rdata2.c2 = rdata2.l1 = rdata2.l2 = rdata2.l3 = rdata2.l4 = 0; - if(rdata2.str) free(rdata2.str); + if(rdata2.str) HDfree(rdata2.str); if(H5Dread(dset1_id, cmpd_m1_tid, H5S_ALL, H5S_ALL, H5P_DEFAULT, &rdata1) < 0) { H5_FAILED(); AT(); @@ -2713,8 +2713,8 @@ test_compound_14(void) goto error; } /* end if */ - if(rdata1.str) free(rdata1.str); - if(rdata2.str) free(rdata2.str); + if(rdata1.str) HDfree(rdata1.str); + if(rdata2.str) HDfree(rdata2.str); if(H5Dclose(dset1_id) < 0) goto error; @@ -2732,7 +2732,166 @@ test_compound_14(void) error: return 1; -} +} /* end test_compound_14() */ + + +/*------------------------------------------------------------------------- + * Function: test_compound_15 + * + * Purpose: Tests that conversion occurs correctly when the source is + * subset of the destination, but there is extra space at the + * end of the source type. + * + * Return: Success: 0 + * + * Failure: number of errors + * + * Programmer: Neil Fortner + * Friday, September 19, 2008 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static int +test_compound_15(void) +{ + typedef struct cmpd_struct { + int i1; + int i2; + } cmpd_struct; + + cmpd_struct wdata1 = {1254, 5471}; + cmpd_struct rdata; + int wdata2[2] = {1, 2}; + hid_t file; + hid_t cmpd_m_tid, cmpd_f_tid; + hid_t space_id; + hid_t dset_id; + hsize_t dim1[1]; + char filename[1024]; + + TESTING("compound subset conversion with extra space in source"); + + /* Create File */ + h5_fixname(FILENAME[3], H5P_DEFAULT, filename, sizeof filename); + if((file=H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0) { + H5_FAILED(); AT(); + printf("Can't create file!\n"); + goto error; + } /* end if */ + + /* Create file compound datatype */ + if((cmpd_f_tid = H5Tcreate( H5T_COMPOUND, sizeof(struct cmpd_struct))) < 0) { + H5_FAILED(); AT(); + printf("Can't create datatype!\n"); + goto error; + } /* end if */ + + if(H5Tinsert(cmpd_f_tid,"i1",HOFFSET(struct cmpd_struct,i1),H5T_NATIVE_INT) < 0) { + H5_FAILED(); AT(); + printf("Can't insert field 'i1'\n"); + goto error; + } /* end if */ + + if(H5Tinsert(cmpd_f_tid,"i2",HOFFSET(struct cmpd_struct,i2),H5T_NATIVE_INT) < 0) { + H5_FAILED(); AT(); + printf("Can't insert field 'i2'\n"); + goto error; + } /* end if */ + + /* Create memory compound datatype */ + if((cmpd_m_tid = H5Tcreate( H5T_COMPOUND, sizeof(struct cmpd_struct))) < 0) { + H5_FAILED(); AT(); + printf("Can't create datatype!\n"); + goto error; + } /* end if */ + + if(H5Tinsert(cmpd_m_tid,"i1",0,H5T_NATIVE_INT) < 0) { + H5_FAILED(); AT(); + printf("Can't insert field 'i1'\n"); + goto error; + } /* end if */ + + /* Create space, dataset, write wdata1 */ + dim1[0] = 1; + if((space_id = H5Screate_simple(1, dim1, NULL)) < 0) { + H5_FAILED(); AT(); + printf("Can't create space\n"); + goto error; + } /* end if */ + + if((dset_id = H5Dcreate2(file, "Dataset", cmpd_f_tid, space_id, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) { + H5_FAILED(); AT(); + printf("Can't create dataset\n"); + goto error; + } /* end if */ + + if(H5Dwrite(dset_id, cmpd_f_tid, H5S_ALL, H5S_ALL, H5P_DEFAULT, &wdata1) < 0) { + H5_FAILED(); AT(); + printf("Can't write data\n"); + goto error; + } /* end if */ + + /* Write wdata2. The use of cmpd_m_tid here should cause only the first + * element of wdata2 to be written. */ + if(H5Dwrite(dset_id, cmpd_m_tid, H5S_ALL, H5S_ALL, H5P_DEFAULT, &wdata2) < 0) { + H5_FAILED(); AT(); + printf("Can't write data\n"); + goto error; + } /* end if */ + + /* Read data */ + if(H5Dread(dset_id, cmpd_f_tid, H5S_ALL, H5S_ALL, H5P_DEFAULT, &rdata) < 0) { + H5_FAILED(); AT(); + printf("Can't read data\n"); + goto error; + } /* end if */ + + /* Check for correctness of read data */ + if(rdata.i1 != wdata2[0] || rdata.i2 != wdata1.i2) { + H5_FAILED(); AT(); + printf("incorrect read data\n"); + goto error; + } /* end if */ + + /* Now try reading only the i1 field, verify it does not overwrite i2 in the + * read buffer */ + rdata.i1 = wdata1.i1; + rdata.i2 = wdata2[1]; + + /* Read data */ + if(H5Dread(dset_id, cmpd_m_tid, H5S_ALL, H5S_ALL, H5P_DEFAULT, &rdata) < 0) { + H5_FAILED(); AT(); + printf("Can't read data\n"); + goto error; + } /* end if */ + + /* Check for correctness of read data */ + if(rdata.i1 != wdata2[0] || rdata.i2 != wdata2[1]) { + H5_FAILED(); AT(); + printf("incorrect read data\n"); + goto error; + } /* end if */ + + /* Close */ + if(H5Dclose(dset_id) < 0) + goto error; + if(H5Tclose(cmpd_f_tid) < 0) + goto error; + if(H5Tclose(cmpd_m_tid) < 0) + goto error; + if(H5Sclose(space_id) < 0) + goto error; + if(H5Fclose(file) < 0) + goto error; + + PASSED(); + return 0; + + error: + return 1; +} /* end test_compound_14() */ /*------------------------------------------------------------------------- @@ -5427,6 +5586,7 @@ main(void) nerrors += test_compound_12(); nerrors += test_compound_13(); nerrors += test_compound_14(); + nerrors += test_compound_15(); nerrors += test_conv_enum_1(); nerrors += test_conv_enum_2(); nerrors += test_conv_bitfield(); |