[svn-r13531] When a dataset is created and no data is written, but fill value is defined,

    if there's VL type of data in the dataset, the address of the data is copied
    multiple times into the buffer, causing some trouble when the data is released.
    Instead, this checkin makes multiple copies of fill value first, then 
    does conversion on each element so that each of them has a copy of the 
    VL data.  A test is added, too.

    Tested it on kagiso, sol, and copper.

2 files changed, 267 insertions, 22 deletions

diff --git a/src/H5Dio.c b/src/H5Dio.c
index ceaf86c..f841f13 100644
--- a/src/H5Dio.c
+++ b/src/H5Dio.c
@@ -228,16 +228,23 @@ done:
     parameter is NULL, use "buf_type" for the fill value datatype.
  EXAMPLES
  REVISION LOG
+    Raymond Lu - 20 March 2007
+    If there's VL type of data, the address of the data is copied multiple
+    times into the buffer, causing some trouble when the data is released.
+    Instead, make multiple copies of fill value first, then do conversion
+    on each element so that each of them has a copy of the VL data. 
 --------------------------------------------------------------------------*/
 static herr_t
 H5D_fill(const void *fill, const H5T_t *fill_type, void *buf, const H5T_t *buf_type, const H5S_t *space, hid_t dxpl_id)
 {
     H5T_path_t *tpath = NULL;   /* Conversion information*/
     uint8_t *tconv_buf = NULL;  /* Data type conv buffer */
-    uint8_t *bkg_buf = NULL;    /* Temp conversion buffer */
+    uint8_t *bkg_buf = NULL;    /* Background conversion buffer */
+    uint8_t *tmp_buf = NULL;    /* Temp conversion buffer */
     hid_t src_id = -1, dst_id = -1;     /* Temporary type IDs */
     size_t src_type_size;       /* Size of source type	*/
     size_t dst_type_size;       /* Size of destination type*/
+    hssize_t nelmts;            /* Number of data elements */
     size_t buf_size;            /* Desired buffer size	*/
     herr_t ret_value=SUCCEED;   /* Return value */
 
@@ -259,42 +266,91 @@ H5D_fill(const void *fill, const H5T_t *fill_type, void *buf, const H5T_t *buf_t
 
     /* Get the maximum buffer size needed and allocate it */
     buf_size=MAX(src_type_size,dst_type_size);
-    if(NULL == (tconv_buf = H5FL_BLK_MALLOC(type_elem,buf_size)) || NULL==(bkg_buf = H5FL_BLK_CALLOC(type_elem,buf_size)))
+
+    /* Allocate space for conversion buffer */
+    if(NULL == (tconv_buf = H5FL_BLK_MALLOC(type_elem,buf_size)))
         HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed")
 
-    /* Check for actual fill value to replicate */
-    if(fill==NULL)
-        /* If there's no fill value, just use zeros */
+    /* If there's no fill value, just use zeros */
+    if(fill==NULL) {
         HDmemset(tconv_buf,0,dst_type_size);
-    else {
-        /* Copy the user's data into the buffer for conversion */
-        HDmemcpy(tconv_buf,fill,src_type_size);
 
-        /* Set up type conversion function */
-        if(NULL == (tpath = H5T_path_find(fill_type, buf_type, NULL, NULL, dxpl_id, FALSE)))
-            HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, FAIL, "unable to convert between src and dest data types")
+        /* Fill the selection in the memory buffer */
+        if(H5S_select_fill(tconv_buf, dst_type_size, space, buf) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTENCODE, FAIL, "filling selection failed")
+
+        HGOTO_DONE(ret_value);
+    }
+ 
+    /* Set up type conversion function */
+    if(NULL == (tpath = H5T_path_find(fill_type, buf_type, NULL, NULL, dxpl_id, FALSE)))
+        HGOTO_ERROR(H5E_DATASET, H5E_UNSUPPORTED, FAIL, "unable to convert between src and dest data types")
+
+    if(!H5T_path_noop(tpath)) {
+        if((src_id = H5I_register(H5I_DATATYPE, H5T_copy(fill_type, H5T_COPY_ALL))) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTREGISTER, FAIL, "unable to register types for conversion")
+
+        if((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")
+    }
+
+    /* Copy the user's data into the buffer for conversion */
+    HDmemcpy(tconv_buf,fill,src_type_size);
+
+    if(TRUE == H5T_detect_class(fill_type, H5T_VLEN)) {
+        /* If there's VL type of data, make multiple copies of fill value first, 
+         * then do conversion on each element so that each of them has a copy 
+         * of the VL data. 
+         */
+
+        /* Get the number of elements */
+        nelmts = H5S_get_simple_extent_npoints(space);
 
-        /* Convert memory buffer into disk buffer */
+        /* Allocate a temporary buffer and background buffer */
+        if(NULL == (tmp_buf = H5FL_BLK_MALLOC(type_conv,(size_t)nelmts*buf_size)) || NULL==(bkg_buf = H5FL_BLK_CALLOC(type_conv,(size_t)nelmts*buf_size)))
+            HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed")
+
+        /* Fill the selection in the temporary buffer */
+        if(H5S_select_fill(tconv_buf, src_type_size, space, tmp_buf) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTENCODE, FAIL, "filling selection failed")
+    
+        /* Convert disk buffer into memory 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")
+            if(H5T_convert(tpath, src_id, dst_id, (size_t)nelmts, (size_t)0, (size_t)0, tmp_buf, bkg_buf, dxpl_id) < 0)
+                HGOTO_ERROR(H5E_DATASET, H5E_CANTCONVERT, FAIL, "data type conversion failed")
+        } /* end if */
 
+        /* Copy the final data into the memory buffer */
+        HDmemcpy(buf, tmp_buf, (size_t)nelmts*dst_type_size);
+    } else {
+        /* If there's no VL type of data, do conversion first then fill the data into
+         * the memory buffer. */
+        if(NULL==(bkg_buf = H5FL_BLK_CALLOC(type_elem,buf_size)))
+            HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed")
+
+        /* Convert disk buffer into memory buffer */
+        if(!H5T_path_noop(tpath)) {
             /* Perform data type conversion */
             if(H5T_convert(tpath, src_id, dst_id, (size_t)1, (size_t)0, (size_t)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 */
-    if(H5S_select_fill(tconv_buf, dst_type_size, space, buf) < 0)
-        HGOTO_ERROR(H5E_DATASET, H5E_CANTENCODE, FAIL, "filling selection failed")
+        /* Fill the selection in the memory buffer */
+        if(H5S_select_fill(tconv_buf, dst_type_size, space, buf) < 0)
+            HGOTO_ERROR(H5E_DATASET, H5E_CANTENCODE, FAIL, "filling selection failed")
+    }
 
 done:
+    if (tmp_buf)
+        H5FL_BLK_FREE(type_conv,tmp_buf);
     if (tconv_buf)
         H5FL_BLK_FREE(type_elem,tconv_buf);
-    if (bkg_buf)
-        H5FL_BLK_FREE(type_elem,bkg_buf);
+    if (bkg_buf) {
+        if(TRUE == H5T_detect_class(fill_type, H5T_VLEN))
+            H5FL_BLK_FREE(type_conv,bkg_buf);
+        else
+            H5FL_BLK_FREE(type_elem,bkg_buf);
+    }
     FUNC_LEAVE_NOAPI(ret_value)
 }   /* H5D_fill() */
 
diff --git a/test/tvltypes.c b/test/tvltypes.c
index d8941e5..de51f4a 100644
--- a/test/tvltypes.c
+++ b/test/tvltypes.c
@@ -29,7 +29,7 @@
 
 /* 1-D dataset with fixed dimensions */
 #define SPACE1_RANK	1
-#define SPACE1_DIM1     4
+#define SPACE1_DIM1     4 
 
 /* 2-D dataset with fixed dimensions */
 #define SPACE2_RANK	2
@@ -2388,6 +2388,194 @@ rewrite_shorter_vltypes_vlen_vlen_atomic(void)
 
 /****************************************************************
 **
+**  test_vltypes_fill_value(): Test fill value for VL data.  
+**  One tests data space isn't allocated; another tests data
+**  space is allocated.
+**
+****************************************************************/
+static void
+test_vltypes_fill_value(void)
+{
+    typedef struct dtype1_struct {
+        int    i1;
+        char   *str;
+        int    i2;
+        int    i3;
+        int    i4;
+        int    i5;
+        int    i6;
+        int    i7;
+        int    i8;
+        float  f1;
+    } dtype1_struct;
+
+    herr_t ret;
+    hid_t file_id;
+    hid_t dtype1_id = -1;
+    hid_t str_id = -1;
+    hid_t dspace_id;
+    hid_t dcpl_id, xfer_pid;
+    hid_t dset_id; 
+    hsize_t dim1[] = {SPACE1_DIM1};
+    const dtype1_struct fill1 = {1, "foobar", 2, 3, 4, 5, 6, 7, 8, 9.0};
+    dtype1_struct buf[SPACE1_DIM1];
+    size_t mem_used=0;  /* Memory used during allocation */
+    int i;
+
+    /* Output message about test being performed */
+    MESSAGE(5, ("Check fill value for VL data\n"));
+
+    /* Create a compound data type */
+    dtype1_id = H5Tcreate(H5T_COMPOUND, sizeof(struct dtype1_struct));
+    CHECK(dtype1_id, FAIL, "H5Tcreate");
+
+    ret = H5Tinsert(dtype1_id,"i1",HOFFSET(struct dtype1_struct,i1),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    str_id = H5Tcopy(H5T_C_S1);
+    CHECK(str_id, FAIL, "H5Tcopy");
+    ret = H5Tset_size(str_id,H5T_VARIABLE);
+    CHECK(ret, FAIL, "H5Tset_size");
+
+    ret = H5Tinsert(dtype1_id,"vl_string",HOFFSET(dtype1_struct,str),str_id);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i2",HOFFSET(struct dtype1_struct,i2),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i3",HOFFSET(struct dtype1_struct,i3),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i4",HOFFSET(struct dtype1_struct,i4),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i5",HOFFSET(struct dtype1_struct,i5),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i6",HOFFSET(struct dtype1_struct,i6),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i7",HOFFSET(struct dtype1_struct,i7),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"i8",HOFFSET(struct dtype1_struct,i8),H5T_NATIVE_INT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tinsert(dtype1_id,"f1",HOFFSET(struct dtype1_struct,f1),H5T_NATIVE_FLOAT);
+    CHECK(ret, FAIL, "H5Tinsert");
+
+    ret = H5Tclose(str_id);
+    CHECK(ret, FAIL, "H5Tclose");
+
+    /* Create dataset create property list and set the fill value */
+    dcpl_id = H5Pcreate(H5P_DATASET_CREATE);
+    CHECK(dcpl_id, FAIL, "H5Pcreate");
+
+    ret = H5Pset_fill_value(dcpl_id, dtype1_id, &fill1);
+    CHECK(ret, FAIL, "H5Pset_fill_value");
+
+    /* Create the file */
+    file_id = H5Fcreate(FILENAME, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(file_id, FAIL, "H5Fcreate");
+
+    dspace_id=H5Screate_simple(SPACE1_RANK,dim1,NULL);
+    CHECK(dspace_id, FAIL, "H5Screate_simple");
+
+    /* Create first data set with default setting - no space is allocated */
+    dset_id = H5Dcreate(file_id, "dataset1", dtype1_id, dspace_id, dcpl_id);
+    CHECK(dset_id, FAIL, "H5Dcreate");
+
+    ret = H5Dclose(dset_id);
+    CHECK(ret, FAIL, "H5Dclose");
+
+    /* Create a second data set with space allocated and fill value written */
+    ret = H5Pset_fill_time(dcpl_id, H5D_FILL_TIME_IFSET);
+    CHECK(ret, FAIL, "H5Pset_fill_time");
+
+    ret = H5Pset_alloc_time(dcpl_id, H5D_ALLOC_TIME_EARLY);
+    CHECK(ret, FAIL, "H5Pset_alloc_time");
+
+    dset_id = H5Dcreate(file_id, "dataset2", dtype1_id, dspace_id, dcpl_id);
+    CHECK(dset_id, FAIL, "H5Dcreate");
+
+    ret = H5Dclose(dset_id);
+    CHECK(ret, FAIL, "H5Dclose");
+
+    ret = H5Fclose(file_id);
+    CHECK(ret, FAIL, "H5Fclose");
+
+
+    /* Open the file to check data set value */
+    file_id = H5Fopen(FILENAME, H5F_ACC_RDONLY, H5P_DEFAULT);
+    CHECK(file_id, FAIL, "H5Fopen");
+
+    /* Open first data set */
+    dset_id = H5Dopen(file_id, "dataset1");
+    CHECK(dset_id, FAIL, "H5Dopen");
+
+    /* Change to the custom memory allocation routines for reading VL data */
+    xfer_pid=H5Pcreate(H5P_DATASET_XFER);
+    CHECK(xfer_pid, FAIL, "H5Pcreate");
+
+    ret=H5Pset_vlen_mem_manager(xfer_pid,test_vltypes_alloc_custom,&mem_used,test_vltypes_free_custom,&mem_used);
+    CHECK(ret, FAIL, "H5Pset_vlen_mem_manager");
+
+    /* Read in the data of fill value */
+    ret = H5Dread(dset_id, dtype1_id, dspace_id, dspace_id,xfer_pid, buf);
+    CHECK(ret, FAIL, "H5Dread");
+
+    /* Compare data read in */
+    for(i=0; i<SPACE1_DIM1; i++) {
+        if(strcmp(buf[i].str, "foobar")) {
+            TestErrPrintf("%d: VL data doesn't match!, buf[%d].str=%s\n",__LINE__,(int)i,buf[i].str);
+            /*continue;*/
+        } /* end if */
+    } /* end for */
+
+    ret = H5Dclose(dset_id);
+    CHECK(ret, FAIL, "H5Dclose");
+
+    /* Release the space */
+    ret = H5Dvlen_reclaim(dtype1_id,dspace_id,xfer_pid,buf);
+    CHECK(ret, FAIL, "H5Dvlen_reclaim");
+
+    /* Open the second data set to check the value of data */
+    dset_id = H5Dopen(file_id, "dataset2");
+    CHECK(dset_id, FAIL, "H5Dopen");
+
+    ret = H5Dread(dset_id, dtype1_id, dspace_id, dspace_id,xfer_pid, buf);
+    CHECK(ret, FAIL, "H5Dread");
+
+    /* Compare data read in */
+    for(i=0; i<SPACE1_DIM1; i++) {
+        if(strcmp(buf[i].str, "foobar")) {
+            TestErrPrintf("%d: VL data doesn't match!, buf[%d].str=%s\n",__LINE__,(int)i,buf[i].str);
+            /*continue;*/
+        } /* end if */
+    } /* end for */
+
+    ret = H5Dclose(dset_id);
+    CHECK(ret, FAIL, "H5Dclose");
+
+    /* Release the space */
+    ret = H5Dvlen_reclaim(dtype1_id,dspace_id,xfer_pid,buf);
+    CHECK(ret, FAIL, "H5Dvlen_reclaim");
+
+    ret = H5Sclose(dspace_id);
+    CHECK(ret, FAIL, "H5Sclose");
+
+    ret = H5Pclose(dcpl_id);
+    CHECK(ret, FAIL, "H5Pclose");
+
+    ret = H5Tclose(dtype1_id);
+    CHECK(ret, FAIL, "H5Tclose");
+
+    ret = H5Fclose(file_id);
+    CHECK(ret, FAIL, "H5Fclose");
+} /* end test_vltypes_fill_value() */
+
+/****************************************************************
+**
 **  test_vltypes(): Main VL datatype testing routine.
 **
 ****************************************************************/
@@ -2412,6 +2600,7 @@ test_vltypes(void)
     rewrite_shorter_vltypes_vlen_vlen_atomic();  /*overwrite with VL data of shorted sequence*/
     test_vltypes_compound_vlen_vlen();/* Test compound datatypes with VL atomic components */
     test_vltypes_compound_vlstr();    /* Test data rewritten of nested VL data */
+    test_vltypes_fill_value();        /* Test fill value for VL data */
 }   /* test_vltypes() */