diff options
author | Neil Fortner <nfortne2@hdfgroup.org> | 2010-04-08 18:41:42 (GMT) |
---|---|---|
committer | Neil Fortner <nfortne2@hdfgroup.org> | 2010-04-08 18:41:42 (GMT) |
commit | 68a140d2cb284dc620306b5a48ea22b49c9dd775 (patch) | |
tree | 8f793ba36720fd96178d975b52b764988266ee00 | |
parent | ee7791d61c7854afeee048024ed47c191b4cd99e (diff) | |
download | hdf5-68a140d2cb284dc620306b5a48ea22b49c9dd775.zip hdf5-68a140d2cb284dc620306b5a48ea22b49c9dd775.tar.gz hdf5-68a140d2cb284dc620306b5a48ea22b49c9dd775.tar.bz2 |
[svn-r18536] Purpose: Fix bug 1815
Description:
Attempting to copy an object with NULL references (all bytes zero) with the
H5O_COPY_EXPAND_REFERENCE_FLAG flag set would cause a failure or an assertion
(depending on whether it was in debug mode). Changed copy routine to detect
NULL references (object and region) and avoid attempting to expand the reference
in this case.
Tested: jam, linew, amani (h5committest)
-rw-r--r-- | release_docs/RELEASE.txt | 2 | ||||
-rw-r--r-- | src/H5Ocopy.c | 79 | ||||
-rw-r--r-- | src/H5T.c | 6 | ||||
-rwxr-xr-x | test/objcopy.c | 165 |
4 files changed, 214 insertions, 38 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 0ee5583..6f6ae76 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -117,6 +117,8 @@ Bug Fixes since HDF5-1.8.4 Library ------- + - Fixed a bug when copying objects with NULL references with the + H5O_COPY_EXPAND_REFERENCE_FLAG flag set. (NAF - 2010/04/08 - 1815) - Added a mechanism to the H5I interface to save returned object ID structures for immediate re-use if needed. This addresses a potential performance issue by delaying the case when the next ID to be diff --git a/src/H5Ocopy.c b/src/H5Ocopy.c index f9bf533..8896795 100644 --- a/src/H5Ocopy.c +++ b/src/H5Ocopy.c @@ -354,15 +354,19 @@ H5O_copy_header_real(const H5O_loc_t *oloc_src, H5O_loc_t *oloc_dst /*out */, oh_dst->max_compact = oh_src->max_compact; oh_dst->min_dense = oh_src->min_dense; - /* Initialize size of chunk array. The destination always has only one - * chunk. + /* Initialize size of chunk array. Start off with zero chunks so this field + * is consistent with the current state of the chunk array. This is + * important if an error occurs. */ - oh_dst->alloc_nchunks = oh_dst->nchunks = 1; + oh_dst->alloc_nchunks = oh_dst->nchunks = 0; - /* Allocate memory for the chunk array */ - if(NULL == (oh_dst->chunk = H5FL_SEQ_MALLOC(H5O_chunk_t, oh_dst->alloc_nchunks))) + /* Allocate memory for the chunk array - always start with 1 chunk */ + if(NULL == (oh_dst->chunk = H5FL_SEQ_MALLOC(H5O_chunk_t, 1))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") + /* Update number of allocated chunks. There are still no chunks used. */ + oh_dst->alloc_nchunks = 1; + /* Allocate memory for "deleted" array. This array marks the message in * the source that shouldn't be copied to the destination. */ @@ -579,6 +583,9 @@ H5O_copy_header_real(const H5O_loc_t *oloc_src, H5O_loc_t *oloc_dst /*out */, oh_dst->chunk[0].size = (size_t)dst_oh_size; oh_dst->chunk[0].gap = dst_oh_gap; + /* Update size of chunk array. The destination now has one chunk. */ + oh_dst->nchunks = 1; + /* Set up raw pointers and copy messages that didn't need special * treatment. This has to happen after the destination header has been * allocated. @@ -1124,8 +1131,13 @@ H5O_copy_expand_ref(H5F_t *file_src, void *_src_ref, hid_t dxpl_id, dst_oloc.addr = HADDR_UNDEF; /* Attempt to copy object from source to destination file */ - if(H5O_copy_obj_by_ref(&src_oloc, dxpl_id, &dst_oloc, &dst_root_loc, cpy_info) < 0) - HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object") + if(src_oloc.addr != (haddr_t)0) { + if(H5O_copy_obj_by_ref(&src_oloc, dxpl_id, &dst_oloc, &dst_root_loc, cpy_info) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object") + } /* end if */ + else + /* Set parameters so the reference is written as all 0's */ + HDmemset(&dst_oloc.addr, 0, sizeof(dst_oloc.addr)); /* Set the object reference info for the destination file */ p = (uint8_t *)(&dst_ref[i]); @@ -1136,7 +1148,7 @@ H5O_copy_expand_ref(H5F_t *file_src, void *_src_ref, hid_t dxpl_id, else if(H5R_DATASET_REGION == ref_type) { hdset_reg_ref_t *src_ref = (hdset_reg_ref_t *)_src_ref; hdset_reg_ref_t *dst_ref = (hdset_reg_ref_t *)_dst_ref; - uint8_t *buf; /* Buffer to store serialized selection in */ + uint8_t *buf = NULL; /* Buffer to store serialized selection in */ H5HG_t hobjid; /* Heap object ID */ size_t buf_size; /* Length of object in heap */ @@ -1147,30 +1159,35 @@ H5O_copy_expand_ref(H5F_t *file_src, void *_src_ref, hid_t dxpl_id, H5F_addr_decode(src_oloc.file, (const uint8_t **)&p, &(hobjid.addr)); INT32DECODE(p, hobjid.idx); - /* Get the dataset region from the heap (allocate inside routine) */ - if((buf = (uint8_t *)H5HG_read(src_oloc.file, dxpl_id, &hobjid, NULL, &buf_size)) == NULL) - HGOTO_ERROR(H5E_REFERENCE, H5E_READERROR, FAIL, "Unable to read dataset region information") - - /* Get the object oid for the dataset */ - p = (uint8_t *)buf; - H5F_addr_decode(src_oloc.file, (const uint8_t **)&p, &(src_oloc.addr)); - dst_oloc.addr = HADDR_UNDEF; - - /* copy the object pointed by the ref to the destination */ - if(H5O_copy_obj_by_ref(&src_oloc, dxpl_id, &dst_oloc, &dst_root_loc, cpy_info) < 0) { - H5MM_xfree(buf); - HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object") - } /* end if */ - - /* Serialize object ID */ - p = (uint8_t *)buf; - H5F_addr_encode(dst_oloc.file, &p, dst_oloc.addr); - - /* Save the serialized buffer to the destination */ - if(H5HG_insert(dst_oloc.file, dxpl_id, buf_size, buf, &hobjid) < 0) { - H5MM_xfree(buf); - HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "Unable to write dataset region information") + if(hobjid.addr != (haddr_t)0) { + /* Get the dataset region from the heap (allocate inside routine) */ + if((buf = (uint8_t *)H5HG_read(src_oloc.file, dxpl_id, &hobjid, NULL, &buf_size)) == NULL) + HGOTO_ERROR(H5E_REFERENCE, H5E_READERROR, FAIL, "Unable to read dataset region information") + + /* Get the object oid for the dataset */ + p = (uint8_t *)buf; + H5F_addr_decode(src_oloc.file, (const uint8_t **)&p, &(src_oloc.addr)); + dst_oloc.addr = HADDR_UNDEF; + + /* copy the object pointed by the ref to the destination */ + if(H5O_copy_obj_by_ref(&src_oloc, dxpl_id, &dst_oloc, &dst_root_loc, cpy_info) < 0) { + H5MM_xfree(buf); + HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object") + } /* end if */ + + /* Serialize object ID */ + p = (uint8_t *)buf; + H5F_addr_encode(dst_oloc.file, &p, dst_oloc.addr); + + /* Save the serialized buffer to the destination */ + if(H5HG_insert(dst_oloc.file, dxpl_id, buf_size, buf, &hobjid) < 0) { + H5MM_xfree(buf); + HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "Unable to write dataset region information") + } /* end if */ } /* end if */ + else + /* Set parameters so the reference is written as all 0's */ + HDmemset(&hobjid, 0, sizeof(hobjid)); /* Set the dataset region reference info for the destination file */ p = (uint8_t *)(&dst_ref[i]); @@ -1863,12 +1863,6 @@ H5T_get_class(const H5T_t *dt, htri_t internal) assert(dt); - /* Lie to the user if they have a VL string and tell them it's in the string class */ - if(dt->shared->type==H5T_VLEN && dt->shared->u.vlen.type==H5T_VLEN_STRING) - ret_value=H5T_STRING; - else - ret_value=dt->shared->type; - /* Externally, a VL string is a string; internally, a VL string is a VL. */ if(internal) { ret_value=dt->shared->type; diff --git a/test/objcopy.c b/test/objcopy.c index aad08d9..7f971b4 100755 --- a/test/objcopy.c +++ b/test/objcopy.c @@ -7791,6 +7791,168 @@ error: /*------------------------------------------------------------------------- + * Function: test_copy_null_ref + * + * Purpose: Creates 2 datasets with references, one with object and + * the other with region references. Copies these datasets + * to a new file without expanding references, causing them + * to become NULL. Next, copies these references to a third + * file with expanding references, to verify that NULL + * references are handled correctly. + * + * Return: Success: 0 + * Failure: number of errors + * + * Programmer: Neil Fortner + * Wednesday, March 31, 2005 + * + *------------------------------------------------------------------------- + */ +static int +test_copy_null_ref(hid_t fcpl_src, hid_t fcpl_dst, hid_t fapl) +{ + hid_t fid1 = -1, fid2 = -1; /* File IDs */ + hid_t sid = -1; /* Dataspace ID */ + hid_t pid = -1; /* Object copy property list ID */ + hid_t did1 = -1, did2 = -1; /* Dataset IDs */ + hsize_t dim1d[1] = {2}; /* Dataset dimensions */ + hobj_ref_t obj_buf[2]; /* Buffer for object refs */ + hdset_reg_ref_t reg_buf[2]; /* Buffer for region refs */ + char zeros[MAX(sizeof(obj_buf),sizeof(reg_buf))]; /* Array of zeros, for memcmp */ + char src_filename[NAME_BUF_SIZE]; + char mid_filename[NAME_BUF_SIZE]; + char dst_filename[NAME_BUF_SIZE]; + + TESTING("H5Ocopy(): NULL references"); + + /* Initialize "zeros" array */ + HDmemset(zeros, 0, sizeof(zeros)); + + /* Initialize the filenames */ + h5_fixname(FILENAME[0], fapl, src_filename, sizeof src_filename); + h5_fixname(FILENAME[1], fapl, mid_filename, sizeof mid_filename); + h5_fixname(FILENAME[2], fapl, dst_filename, sizeof dst_filename); + + /* Reset file address checking info */ + addr_reset(); + + /* Create source file */ + if((fid1 = H5Fcreate(src_filename, H5F_ACC_TRUNC, fcpl_src, fapl)) < 0) + TEST_ERROR + + /* Create dataspace */ + if((sid = H5Screate_simple(1, dim1d, NULL)) < 0) TEST_ERROR + + /* Create object reference dataset at SRC file */ + if((did1 = H5Dcreate2(fid1, "obj_ref_dset", H5T_STD_REF_OBJ, sid, + H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) TEST_ERROR + + /* Create region reference dataset at SRC file */ + if((did2 = H5Dcreate2(fid1, "reg_ref_dset", H5T_STD_REF_DSETREG, + sid, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) TEST_ERROR + + /* Create references */ + if(H5Rcreate(&obj_buf[0], did1, ".", H5R_OBJECT, -1) < 0) TEST_ERROR + if(H5Rcreate(&obj_buf[1], did2, ".", H5R_OBJECT, -1) < 0) TEST_ERROR + if(H5Rcreate(®_buf[0], did1, ".", H5R_DATASET_REGION, sid) < 0) + TEST_ERROR + if(H5Rcreate(®_buf[1], did2, ".", H5R_DATASET_REGION, sid) < 0) + TEST_ERROR + + /* Write data into file */ + if(H5Dwrite(did1, H5T_STD_REF_OBJ, H5S_ALL, H5S_ALL, H5P_DEFAULT, obj_buf) + < 0) TEST_ERROR + if(H5Dwrite(did2, H5T_STD_REF_DSETREG, H5S_ALL, H5S_ALL, H5P_DEFAULT, + reg_buf) < 0) TEST_ERROR + + /* Close datasets */ + if(H5Dclose(did1) < 0) TEST_ERROR + if(H5Dclose(did2) < 0) TEST_ERROR + + /* Create middle file */ + if((fid2 = H5Fcreate(mid_filename, H5F_ACC_TRUNC, fcpl_src, fapl)) < 0) + TEST_ERROR + + /* Copy the source file to the middle file. Note the expand references + * flag is not set. */ + if(H5Ocopy(fid1, "/", fid2, "/A", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + + /* Close source file */ + if(H5Fclose(fid1) < 0) TEST_ERROR + + /* Open copied datasets */ + if((did1 = H5Dopen2(fid2, "/A/obj_ref_dset", H5P_DEFAULT)) < 0) TEST_ERROR + if((did2 = H5Dopen2(fid2, "/A/reg_ref_dset", H5P_DEFAULT)) < 0) TEST_ERROR + + /* Read copied datasets */ + if(H5Dread(did1, H5T_STD_REF_OBJ, H5S_ALL, H5S_ALL, H5P_DEFAULT, obj_buf) + < 0) TEST_ERROR + if(H5Dread(did2, H5T_STD_REF_DSETREG, H5S_ALL, H5S_ALL, H5P_DEFAULT, + reg_buf) < 0) TEST_ERROR + + /* Verify that the references contain only "0" bytes */ + if(HDmemcmp(obj_buf, zeros, sizeof(obj_buf))) TEST_ERROR + if(HDmemcmp(reg_buf, zeros, sizeof(reg_buf))) TEST_ERROR + + /* Close datasets */ + if(H5Dclose(did1) < 0) TEST_ERROR + if(H5Dclose(did2) < 0) TEST_ERROR + + /* Create destination file */ + if((fid1 = H5Fcreate(dst_filename, H5F_ACC_TRUNC, fcpl_dst, fapl)) < 0) + TEST_ERROR + + /* Create object copy property list */ + if((pid = H5Pcreate(H5P_OBJECT_COPY)) < 0) TEST_ERROR + + /* Set the "expand references" flag */ + if(H5Pset_copy_object(pid, H5O_COPY_EXPAND_REFERENCE_FLAG) < 0) TEST_ERROR + + /* Copy the middle file to the destination file. Note the expand references + * flag *is* set, even though the references are now NULL. */ + if(H5Ocopy(fid2, "/", fid1, "/AA", pid, H5P_DEFAULT) < 0) TEST_ERROR + + /* Close source file */ + if(H5Fclose(fid2) < 0) TEST_ERROR + + /* Open copied datasets */ + if((did1 = H5Dopen2(fid1, "/AA/A/obj_ref_dset", H5P_DEFAULT)) < 0) TEST_ERROR + if((did2 = H5Dopen2(fid1, "/AA/A/reg_ref_dset", H5P_DEFAULT)) < 0) TEST_ERROR + + /* Read copied datasets */ + if(H5Dread(did1, H5T_STD_REF_OBJ, H5S_ALL, H5S_ALL, H5P_DEFAULT, obj_buf) + < 0) TEST_ERROR + if(H5Dread(did2, H5T_STD_REF_DSETREG, H5S_ALL, H5S_ALL, H5P_DEFAULT, + reg_buf) < 0) TEST_ERROR + + /* Verify that the references contain only "0" bytes */ + if(HDmemcmp(obj_buf, zeros, sizeof(obj_buf))) TEST_ERROR + if(HDmemcmp(reg_buf, zeros, sizeof(reg_buf))) TEST_ERROR + + /* Close */ + if(H5Pclose(pid) < 0) TEST_ERROR + if(H5Dclose(did1) < 0) TEST_ERROR + if(H5Dclose(did2) < 0) TEST_ERROR + if(H5Fclose(fid1) < 0) TEST_ERROR + if(H5Sclose(sid) < 0) TEST_ERROR + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY { + H5Pclose(pid); + H5Dclose(did1); + H5Dclose(did2); + H5Fclose(fid1); + H5Fclose(fid2); + H5Sclose(sid); + } H5E_END_TRY; + return 1; +} /* end test_copy_null_ref */ + + +/*------------------------------------------------------------------------- * Function: test_copy_option * * Purpose: Create a group in SRC file and copy it to DST file @@ -8259,7 +8421,8 @@ main(void) nerrors += test_copy_same_file_named_datatype(fcpl_src, my_fapl); nerrors += test_copy_old_layout(fcpl_dst, my_fapl); - } + nerrors += test_copy_null_ref(fcpl_src, fcpl_dst, my_fapl); + } /* TODO: not implemented nerrors += test_copy_mount(my_fapl); |