diff options
| -rw-r--r-- | release_docs/RELEASE.txt | 2 | ||||
| -rw-r--r-- | src/H5F.c | 48 | ||||
| -rw-r--r-- | src/H5Lexternal.c | 20 | ||||
| -rw-r--r-- | src/H5Ocopy.c | 10 | ||||
| -rw-r--r-- | test/links.c | 139 |
5 files changed, 191 insertions, 28 deletions
diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index a12ed6a..1e845e3 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -208,6 +208,8 @@ Bug Fixes since HDF5-1.8.0 release Library ------- + - Fixed a bug where the library, when traversing an external link, would + reopen the source file if nothing else worked. (NAF - 2010/03/05) - Fixed an intermittent bug in the b-tree code which could be triggered by expanding and shrinking chunked datasets in certain ways. (NAF - 2010/02/16) @@ -71,7 +71,7 @@ static H5F_t *H5F_new(H5F_file_t *shared, hid_t fcpl_id, hid_t fapl_id, H5FD_t *lf); static herr_t H5F_build_actual_name(const H5F_t *f, const H5P_genplist_t *fapl, const char *name, char ** /*out*/ actual_name); -static herr_t H5F_dest(H5F_t *f, hid_t dxpl_id); +static herr_t H5F_dest(H5F_t *f, hid_t dxpl_id, hbool_t flush); static herr_t H5F_close(H5F_t *f); /* Declare a free list to manage the H5F_t struct */ @@ -967,7 +967,7 @@ done: *------------------------------------------------------------------------- */ static herr_t -H5F_dest(H5F_t *f, hid_t dxpl_id) +H5F_dest(H5F_t *f, hid_t dxpl_id, hbool_t flush) { herr_t ret_value = SUCCEED; /* Return value */ @@ -978,6 +978,14 @@ H5F_dest(H5F_t *f, hid_t dxpl_id) HDassert(f->shared); if(1 == f->shared->nrefs) { + /* Flush at this point since the file will be closed. + * Only try to flush the file if it was opened with write access, and if + * the caller requested a flush. + */ + if((f->shared->flags & H5F_ACC_RDWR) && flush) + if(H5F_flush(f, dxpl_id) < 0) + HDONE_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache") + /* Release objects that depend on the superblock being initialized */ if(f->shared->sblock) { /* Shutdown file free space manager(s) */ @@ -1314,7 +1322,7 @@ H5F_open(const char *name, unsigned flags, hid_t fcpl_id, hid_t fapl_id, done: if(!ret_value && file) - if(H5F_dest(file, dxpl_id) < 0) + if(H5F_dest(file, dxpl_id, FALSE) < 0) HDONE_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, NULL, "problems closing file") FUNC_LEAVE_NOAPI(ret_value) @@ -1857,24 +1865,16 @@ H5F_try_close(H5F_t *f) if(H5F_close_mounts(f) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't unmount child files") - /* Flush at this point since the file will be closed. Don't invalidate - * the cache, since this file might still be open using another handle. - * However, make sure we flush in case that handle is read-only; its - * copy of the cache needs to be clean. - * Only try to flush the file if it was opened with write access. - */ - if(f->intent & H5F_ACC_RDWR) { - /* Flush all caches */ - if(H5F_flush(f, H5AC_dxpl_id) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache") - } /* end if */ + /* Delay flush until the shared file struct is closed, in H5F_dest. If the + * application called H5Fclose, it would have been flushed in that function + * (unless it will have been flushed in H5F_dest anyways). */ /* * Destroy the H5F_t struct and decrement the reference count for the * shared H5F_file_t struct. If the reference count for the H5F_file_t * struct reaches zero then destroy it also. */ - if(H5F_dest(f, H5AC_dxpl_id) < 0) + if(H5F_dest(f, H5AC_dxpl_id, TRUE) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "problems closing file") done: @@ -1906,6 +1906,8 @@ done: herr_t H5Fclose(hid_t file_id) { + H5F_t *f = NULL; + int nref; herr_t ret_value = SUCCEED; FUNC_ENTER_API(H5Fclose, FAIL) @@ -1915,6 +1917,20 @@ H5Fclose(hid_t file_id) if(H5I_FILE != H5I_get_type(file_id)) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a file ID") + /* Flush file if this is the last reference to this id and we have write + * intent, unless it will be flushed by the "shared" file being closed. + * This is only necessary to replicate previous behaviour, and could be + * disabled by an option/property to improve performance. */ + if(NULL == (f = (H5F_t *)H5I_object(file_id))) + HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid file identifier") + if((f->shared->nrefs > 1) && (H5F_INTENT(f) & H5F_ACC_RDWR)) { + if((nref = H5I_get_ref(file_id, FALSE)) < 0) + HGOTO_ERROR(H5E_ATOM, H5E_CANTGET, FAIL, "can't get ID ref count") + if(nref == 1) + if(H5F_flush(f, H5AC_dxpl_id) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache") + } /* end if */ + /* * Decrement reference count on atom. When it reaches zero the file will * be closed. @@ -1982,7 +1998,7 @@ H5Freopen(hid_t file_id) done: if(ret_value < 0 && new_file) - if(H5F_dest(new_file, H5AC_dxpl_id) < 0) + if(H5F_dest(new_file, H5AC_dxpl_id, FALSE) < 0) HDONE_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't close file") FUNC_LEAVE_API(ret_value) diff --git a/src/H5Lexternal.c b/src/H5Lexternal.c index db762cf..7ccdb68 100644 --- a/src/H5Lexternal.c +++ b/src/H5Lexternal.c @@ -412,19 +412,15 @@ H5L_extern_traverse(const char UNUSED *link_name, hid_t cur_group, /* get last component of file_name */ GET_LAST_DELIMITER(actual_file_name, ptr) - if(ptr) { - /* Truncate filename portion from actual file name path */ - *ptr = '\0'; + if(!ptr) + HGOTO_ERROR(H5E_LINK, H5E_CANTOPENFILE, FAIL, "unable to open external file, external link file name = '%s', temp_file_name = '%s'", file_name, temp_file_name) - /* Build new file name for the external file */ - if(H5L_build_name(actual_file_name, temp_file_name, &full_name/*out*/) < 0) - HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "can't prepend prefix to filename") - } /* end if */ - else { - /* Transfer ownership of actual file name to 'full_name' variable */ - full_name = actual_file_name; - actual_file_name = NULL; - } /* end else */ + /* Truncate filename portion from actual file name path */ + *ptr = '\0'; + + /* Build new file name for the external file */ + if(H5L_build_name(actual_file_name, temp_file_name, &full_name/*out*/) < 0) + HGOTO_ERROR(H5E_LINK, H5E_CANTGET, FAIL, "can't prepend prefix to filename") /* Try opening with the resolved name */ if(NULL == (ext_file = H5F_open(full_name, intent, H5P_FILE_CREATE_DEFAULT, fapl_id, H5AC_dxpl_id))) diff --git a/src/H5Ocopy.c b/src/H5Ocopy.c index 8bdbed5..1712706 100644 --- a/src/H5Ocopy.c +++ b/src/H5Ocopy.c @@ -946,6 +946,7 @@ H5O_copy_obj(H5G_loc_t *src_loc, H5G_loc_t *dst_loc, const char *dst_name, H5G_name_t new_path; /* Copied object group hier. path */ H5O_loc_t new_oloc; /* Copied object object location */ H5G_loc_t new_loc; /* Group location of object copied */ + H5F_t *cached_dst_file; /* Cached destination file */ hbool_t entry_inserted=FALSE; /* Flag to indicate that the new entry was inserted into a group */ unsigned cpy_option = 0; /* Copy options */ herr_t ret_value = SUCCEED; /* Return value */ @@ -972,10 +973,19 @@ H5O_copy_obj(H5G_loc_t *src_loc, H5G_loc_t *dst_loc, const char *dst_name, H5G_loc_reset(&new_loc); new_oloc.file = dst_loc->oloc->file; + /* Make a copy of the destination file, in case the original is changed by + * H5O_copy_header. If and when oloc's point to the shared file struct, + * this will no longer be necessary, so this code can be removed. */ + cached_dst_file = dst_loc->oloc->file; + /* Copy the object from the source file to the destination file */ if(H5O_copy_header(src_loc->oloc, &new_oloc, dxpl_id, cpy_option) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object") + /* Patch dst_loc. Again, this can be removed once oloc's point to shared + * file structs. */ + dst_loc->oloc->file = cached_dst_file; + /* Insert the new object in the destination file's group */ if(H5L_link(dst_loc, dst_name, &new_loc, lcpl_id, H5P_DEFAULT, dxpl_id) < 0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "unable to insert link") diff --git a/test/links.c b/test/links.c index bf6d1d4..9146a3e 100644 --- a/test/links.c +++ b/test/links.c @@ -6611,6 +6611,143 @@ external_symlink(const char *env_h5_drvr, hid_t fapl, hbool_t new_format) /*------------------------------------------------------------------------- + * Function: external_copy_invalid_object + * + * Purpose: Check that attempting to copy an object through an + * external link to the source file but with an invalid + * object name fails gracefully, and does not leave lingering + * file ids. + * + * Return: Success: 0 + * Failure: -1 + * + * Programmer: Neil Fortner + * Wednesday, March 3, 2010 + * + *------------------------------------------------------------------------- + */ +static int +external_copy_invalid_object(hid_t fapl, hbool_t new_format) +{ + hid_t fid = (-1); /* File ID */ + hid_t gid = (-1); /* Group ID */ + hid_t ocpyplid = (-1); /* Object copy plist ID */ + char filename[NAME_BUF_SIZE]; + + if(new_format) + TESTING("copying invalid external links to the source file (w/new group format)") + else + TESTING("copying invalid external links to the source file") + + /* Set up filename */ + h5_fixname(FILENAME[0], fapl, filename, sizeof filename); + + /* Create object copy plist, set expand external flag */ + if((ocpyplid = H5Pcreate(H5P_OBJECT_COPY)) < 0) TEST_ERROR + if(H5Pset_copy_object(ocpyplid, H5O_COPY_EXPAND_EXT_LINK_FLAG) < 0) TEST_ERROR + + /* Create file and group */ + if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((gid = H5Gcreate2(fid, "group1", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + + /* Create an external link in the group to the source file with an invalid + * object name */ + if(H5Lcreate_external(filename, "no_object", fid, "/group1/link", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + + /* Copy the group containing the external link */ + if(H5Ocopy(fid, "group1", fid, "group2", ocpyplid, H5P_DEFAULT) < 0) TEST_ERROR + + /* Close file */ + if(H5Fclose(fid) < 0) TEST_ERROR + + /* Attempt to truncate the file again. If there is a lingering id for this + * file this will fail */ + if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + + /* Close */ + if(H5Fclose(fid) < 0) TEST_ERROR + if(H5Pclose(ocpyplid) < 0) TEST_ERROR + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY { + H5Gclose(gid); + H5Fclose(fid); + H5Pclose(ocpyplid); + } H5E_END_TRY + + return -1; +} /* end external_copy_invalid_object */ + + +/*------------------------------------------------------------------------- + * Function: external_dont_fail_to_source + * + * Purpose: Check that external links with invalid target file names + * work properly and do not attempt to (re)open the source + * file. + * + * Return: Success: 0 + * Failure: -1 + * + * Programmer: Neil Fortner + * Wednesday, March 3, 2010 + * + *------------------------------------------------------------------------- + */ +static int +external_dont_fail_to_source(hid_t fapl, hbool_t new_format) +{ + hid_t fid = (-1); /* File ID */ + hid_t gid = (-1); /* Group ID */ + hid_t oid = (-1); /* Object ID */ + char filename[NAME_BUF_SIZE]; + + if(new_format) + TESTING("that invalid external links don't open the source file (w/new group format)") + else + TESTING("that invalid external links don't open the source file") + + /* Set up filename */ + h5_fixname(FILENAME[0], fapl, filename, sizeof filename); + + /* Create file and group */ + if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((gid = H5Gcreate2(fid, "group", H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + + /* Create an external link with an invalid file name, but the same object + * name as the group. This way, if the external link is interpreted to + * refer to the source file, it will link to the group */ + if(H5Lcreate_external("no_file", "/group", fid, "link", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + + /* Attempt to open the object the link points to. This should fail */ + H5E_BEGIN_TRY { + oid = H5Oopen(fid, "link", H5P_DEFAULT); + } H5E_END_TRY + if(oid >= 0) FAIL_PUTS_ERROR("Succeeded in opening target of invalid external link") + + /* Close */ + if(H5Fclose(fid) < 0) TEST_ERROR + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY { + H5Gclose(gid); + H5Oclose(oid); + H5Fclose(fid); + } H5E_END_TRY + + return -1; +} /* end external_dont_fail_to_source */ + + +/*------------------------------------------------------------------------- * Function: ud_hard_links * * Purpose: Check that the functionality of hard links can be duplicated @@ -13889,6 +14026,8 @@ main(void) nerrors += external_link_win9(my_fapl, new_format) < 0 ? 1 : 0; #endif nerrors += external_symlink(env_h5_drvr, my_fapl, new_format) < 0 ? 1 : 0; + nerrors += external_copy_invalid_object(my_fapl, new_format) < 0 ? 1 : 0; + nerrors += external_dont_fail_to_source(my_fapl, new_format) < 0 ? 1 : 0; /* These tests assume that external links are a form of UD links, * so assume that everything that passed for external links |