mirror/hdf5
2
0
Fork 0
mirror of https://github.com/HDFGroup/hdf5.git synced 2025-02-23 16:20:57 +08:00
Code Issues Packages Projects Releases Wiki Activity

[svn-r18374] Purpose: Fix bugs involving external links

Browse Source 
Description:
Previously, the library would reopen the source file when traversing an external
link if unable to find the target file otherwise.  This has been corrected.
Also moved the call to H5F_flush from H5F_try_close to H5F_dest, so the file is
only flushed when the last identifier for the file is closed.  This prevernts
situations where the library could attempt to flush a file with protected
metadata.

Tested: jam, amani, linew (h5committest); Fedora
This commit is contained in:
Neil Fortner 2010-03-05 12:24:17 -05:00
parent 3a06e04ac5
commit 179b54da83
5 changed files with 191 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
release_docs/RELEASE.txt
View File

@ -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)

48
src/H5F.c
View File

@ -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)

20
src/H5Lexternal.c
View File

@ -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)))

10
src/H5Ocopy.c
View File

@ -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")

139
test/links.c
View File

@ -6609,6 +6609,143 @@ external_symlink(const char *env_h5_drvr, hid_t fapl, hbool_t new_format)
#endif /* H5_HAVE_SYMLINK */
} /* end external_symlink() */
/*-------------------------------------------------------------------------
* 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
@ -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