From a847246eb43ff8f38d7022158ea0a44a63303dee Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Tue, 13 Jan 2004 13:01:04 -0500 Subject: [svn-r8054] Purpose: Bug fix Description: Correct two problems with variable-length datatypes in datasets: - When overwriting an entire dataset, writing the fill value to the file would be skipped, causing problems for VL datatypes when objects in the file had been unlinked (and thus the space in the file was not all zeros) - When an application has set a fill-value for a dataset and the dataset's datatype contained a VL datatype, the library was filling space on disk with the memory form of the VL information, instead of the disk form. Platforms tested: FreeBSD 4.9 (sleipnir) h5committest --- release_docs/RELEASE.txt | 3 ++ src/H5Dio.c | 31 ++++++----- src/H5Ofill.c | 4 +- test/Makefile.in | 12 ++--- test/tvlstr.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 160 insertions(+), 23 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 35259ad..21f768f 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -78,6 +78,9 @@ Bug Fixes since HDF5-1.6.1 release Library ------- + - Fixed bug that caused variable-length datatypes (strings or sequences) + used for datasets in files with objects that were unlinked to + fail to be read/written to a file. QAK - 2004/01/13 - Fixed small internal memory leak of fill-value information. QAK - 2004/01/06 - Detect situation where szip 'pixels per block' is larger than the diff --git a/src/H5Dio.c b/src/H5Dio.c index 17382dd..7be69fb 100644 --- a/src/H5Dio.c +++ b/src/H5Dio.c @@ -220,14 +220,11 @@ H5D_fill(const void *fill, const H5T_t *fill_type, void *buf, const H5T_t *buf_t FUNC_ENTER_NOAPI_NOINIT(H5D_fill); /* Check args */ + assert(fill_type); assert(buf); assert(buf_type); assert(space); - /* Check for "default" fill value */ - if(fill_type==NULL) - fill_type=buf_type; - /* Get the memory and file datatype sizes */ src_type_size = H5T_get_size(fill_type); dst_type_size = H5T_get_size(buf_type); @@ -244,19 +241,20 @@ H5D_fill(const void *fill, const H5T_t *fill_type, void *buf, const H5T_t *buf_t /* Copy the user's data into the buffer for conversion */ HDmemcpy(tconv_buf,fill,src_type_size); - /* Convert memory buffer into disk buffer */ /* Set up type conversion function */ - if (NULL == (tpath = H5T_path_find(fill_type, buf_type, NULL, NULL, dxpl_id))) { + if (NULL == (tpath = H5T_path_find(fill_type, buf_type, NULL, NULL, dxpl_id))) HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, FAIL, "unable to convert between src and dest data types"); - } else if (!H5T_path_noop(tpath)) { + + /* Convert memory buffer into disk buffer */ + if (!H5T_path_noop(tpath)) { if ((src_id = H5I_register(H5I_DATATYPE, H5T_copy(fill_type, H5T_COPY_ALL)))<0 || (dst_id = H5I_register(H5I_DATATYPE, H5T_copy(buf_type, H5T_COPY_ALL)))<0) HGOTO_ERROR(H5E_DATASET, H5E_CANTREGISTER, FAIL, "unable to register types for conversion"); - } - /* Perform data type conversion */ - if (H5T_convert(tpath, src_id, dst_id, (hsize_t)1, 0, 0, tconv_buf, bkg_buf, dxpl_id)<0) - HGOTO_ERROR(H5E_DATASET, H5E_CANTCONVERT, FAIL, "data type conversion failed"); + /* Perform data type conversion */ + if (H5T_convert(tpath, src_id, dst_id, (hsize_t)1, 0, 0, tconv_buf, bkg_buf, dxpl_id)<0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTCONVERT, FAIL, "data type conversion failed"); + } /* end if */ } /* end if */ /* Fill the selection in the memory buffer */ @@ -599,7 +597,7 @@ H5D_read(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, HGOTO_DONE(SUCCEED); /* Go fill the user's selection with the dataset's fill value */ - if(H5D_fill(fill.buf,fill.type,buf,mem_type,mem_space, dxpl_id)<0) { + if(H5D_fill(fill.buf,dataset->type,buf,mem_type,mem_space, dxpl_id)<0) { HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "filling buf failed"); } else HGOTO_DONE(SUCCEED); @@ -815,13 +813,20 @@ H5D_write(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, if(nelmts > 0 && dataset->efl.nused==0 && dataset->layout.type!=H5D_COMPACT && dataset->layout.addr==HADDR_UNDEF) { hssize_t file_nelmts; /* Number of elements in file dataset's dataspace */ + hbool_t full_overwrite; /* Whether we are over-writing all the elements */ /* Get the number of elements in file dataset's dataspace */ if((file_nelmts=H5S_get_simple_extent_npoints(file_space))<0) HGOTO_ERROR (H5E_DATASET, H5E_BADVALUE, FAIL, "can't retrieve number of elements in file dataset"); + /* Always allow fill values to be written if the dataset has a VL datatype */ + if(H5T_detect_class(dataset->type, H5T_VLEN)) + full_overwrite=FALSE; + else + full_overwrite=(hsize_t)file_nelmts==nelmts ? TRUE : FALSE; + /* Allocate storage */ - if(H5D_alloc_storage(dataset->ent.file,dxpl_id,dataset,H5D_ALLOC_WRITE, TRUE, (hbool_t)((hsize_t)file_nelmts==nelmts ? TRUE : FALSE))<0) + if(H5D_alloc_storage(dataset->ent.file,dxpl_id,dataset,H5D_ALLOC_WRITE, TRUE, full_overwrite)<0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to initialize storage"); } /* end if */ diff --git a/src/H5Ofill.c b/src/H5Ofill.c index a2ffdb1..b0716f6 100644 --- a/src/H5Ofill.c +++ b/src/H5Ofill.c @@ -882,9 +882,9 @@ H5O_fill_convert(void *_fill, H5T_t *dset_type, hid_t dxpl_id) /* Don't bother doing anything if there will be no actual conversion */ if (!H5T_path_noop(tpath)) { if ((src_id = H5I_register(H5I_DATATYPE, - H5T_copy(fill->type, H5T_COPY_TRANSIENT)))<0 || + H5T_copy(fill->type, H5T_COPY_ALL)))<0 || (dst_id = H5I_register(H5I_DATATYPE, - H5T_copy(dset_type, H5T_COPY_TRANSIENT)))<0) + H5T_copy(dset_type, H5T_COPY_ALL)))<0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "unable to copy/register data type"); /* diff --git a/test/Makefile.in b/test/Makefile.in index ed2ad16..c5e7073 100644 --- a/test/Makefile.in +++ b/test/Makefile.in @@ -55,12 +55,12 @@ MOSTLYCLEAN=cmpd_dset.h5 compact_dataset.h5 dataset.h5 extend.h5 istore.h5 \ big[0-9][0-9][0-9][0-9][0-9].h5 dtypes[1-3].h5 tattr.h5 \ tselect.h5 mtime.h5 unlink.h5 fillval_[0-9].h5 fillval.raw \ mount_[0-9].h5 testmeta.h5 ttime.h5 trefer[1-3].h5 tvltypes.h5 \ - tvlstr.h5 flush.h5 enum1.h5 titerate.h5 ttsafe.h5 tarray1.h5 \ - tgenprop.h5 tmisc1.h5 tmisc2a.h5 tmisc2b.h5 tmisc3.h5 tmisc4a.h5 \ - tmisc4b.h5 tmisc5.h5 tmisc6.h5 tmisc7.h5 tmisc8.h5 tmisc9.h5 \ - tmisc10.h5 tmisc11.h5 tmisc12.h5 tmisc13a.h5 tmisc13b.h5 \ - tmisc14.h5 set_extent_read.h5 set_extent_create.h5 getname.h5 \ - getname1.h5 getname2.h5 getname3.h5 sec2_file.h5 \ + tvlstr.h5 tvlstr2.h5 flush.h5 enum1.h5 titerate.h5 ttsafe.h5 \ + tarray1.h5 tgenprop.h5 tmisc1.h5 tmisc2a.h5 tmisc2b.h5 tmisc3.h5 \ + tmisc4a.h5 tmisc4b.h5 tmisc5.h5 tmisc6.h5 tmisc7.h5 tmisc8.h5 \ + tmisc9.h5 tmisc10.h5 tmisc11.h5 tmisc12.h5 tmisc13a.h5 \ + tmisc13b.h5 tmisc14.h5 set_extent_read.h5 set_extent_create.h5 \ + getname.h5 getname1.h5 getname2.h5 getname3.h5 sec2_file.h5 \ family_file000[0-3][0-9].h5 multi_file-[rs].h5 core_file \ new_move_[ab].h5 ntypes.h5 dangle.h5 diff --git a/test/tvlstr.c b/test/tvlstr.c index f22f04d..60773f1 100644 --- a/test/tvlstr.c +++ b/test/tvlstr.c @@ -31,6 +31,7 @@ #include "hdf5.h" #define DATAFILE "tvlstr.h5" +#define DATAFILE2 "tvlstr2.h5" /* 1-D dataset with fixed dimensions */ #define SPACE1_NAME "Space1" @@ -45,6 +46,9 @@ #define VLSTR_TYPE "vl_string_type" +/* Definitions for the VL re-writing test */ +#define REWRITE_NDATASETS 32 + /* String for testing attributes */ static const char *string_att = "This is the string for the attribute"; static char *string_att_write=NULL; @@ -725,6 +729,127 @@ static void test_read_vl_string_attribute(void) return; } +/* Helper routine for test_vl_rewrite() */ +static void write_scalar_dset(hid_t file, hid_t type, hid_t space, char *name, char *data) +{ + hid_t dset; + herr_t ret; + + dset = H5Dcreate (file, name, type, space, H5P_DEFAULT); + CHECK(dset, FAIL, "H5Dcreate"); + + ret = H5Dwrite(dset, type, space, space, H5P_DEFAULT, &data); + CHECK(ret, FAIL, "H5Dwrite"); + + ret = H5Dclose(dset); + CHECK(ret, FAIL, "H5Dclose"); +} + +/* Helper routine for test_vl_rewrite() */ +static void read_scalar_dset(hid_t file, hid_t type, hid_t space, char *name, char *data) +{ + hid_t dset; + herr_t ret; + char *data_read; + + dset = H5Dopen (file, name); + CHECK(dset, FAIL, "H5Dopen"); + + ret = H5Dread(dset, type, space, space, H5P_DEFAULT, &data_read); + CHECK(ret, FAIL, "H5Dread"); + + ret = H5Dclose(dset); + CHECK(ret, FAIL, "H5Dclose"); + + if(HDstrcmp(data, data_read)) + TestErrPrintf("Expected %s for dataset %s but read %s\n", data, name, data_read); + + HDfree(data_read); +} + +/**************************************************************** +** +** test_vl_rewrite(): Test basic VL string code. +** Tests I/O on VL strings when lots of objects in the file +** have been linked/unlinked. +** +****************************************************************/ +static void test_vl_rewrite(void) +{ + hid_t file1, file2; /* File IDs */ + hid_t type; /* VL string datatype ID */ + hid_t space; /* Scalar dataspace */ + char name[256]; /* Buffer for names & data */ + int i; /* Local index variable */ + herr_t ret; /* Generic return value */ + + /* Create the VL string datatype */ + type = H5Tcopy(H5T_C_S1); + CHECK(type, FAIL, "H5Tcopy"); + + ret = H5Tset_size(type, H5T_VARIABLE); + CHECK(ret, FAIL, "H5Tset_size"); + + /* Create the scalar dataspace */ + space = H5Screate(H5S_SCALAR); + CHECK(space, FAIL, "H5Screate"); + + /* Open the files */ + file1 = H5Fcreate(DATAFILE, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file1, FAIL, "H5Fcreate"); + + file2 = H5Fcreate(DATAFILE2, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file1, FAIL, "H5Fcreate"); + + /* Create in file 1 */ + for(i=0; i