From 75d22ed839a6b7a5e048ac52708d04eb4ad590d3 Mon Sep 17 00:00:00 2001 From: James Laird Date: Fri, 18 Aug 2006 15:48:54 -0500 Subject: [svn-r12596] Refactored how external files are opened and closed. Object header locations (H5O_loc_t's) can now "hold open" a file and decrement its open object count when they close. This means that locations (H5O_loc_t's and H5G_loc_t's) should always be freed. Added more thorough tests to ensure that external files are closed. --- src/H5AC.c | 1 + src/H5D.c | 51 ++++++--- src/H5Fmount.c | 22 +++- src/H5G.c | 185 +++++++++++++++++++------------- src/H5Gloc.c | 13 ++- src/H5Gobj.c | 99 +---------------- src/H5Gpkg.h | 7 +- src/H5Gprivate.h | 11 +- src/H5Gtraverse.c | 152 +++++++++++++------------- src/H5L.c | 66 +++++++----- src/H5Lexternal.c | 1 - src/H5O.c | 102 ++++++++++++++++-- src/H5Oprivate.h | 4 + src/H5Tcommit.c | 27 ++++- test/links.c | 310 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 15 files changed, 730 insertions(+), 321 deletions(-) diff --git a/src/H5AC.c b/src/H5AC.c index 49625b9..454078f 100644 --- a/src/H5AC.c +++ b/src/H5AC.c @@ -1893,6 +1893,7 @@ H5AC_protect(H5F_t *f, /* check args */ HDassert(f); + HDassert(f->shared); HDassert(f->shared->cache); HDassert(type); HDassert(type->flush); diff --git a/src/H5D.c b/src/H5D.c index 850c4ea..154809d 100644 --- a/src/H5D.c +++ b/src/H5D.c @@ -1195,9 +1195,13 @@ hid_t H5Dcreate(hid_t loc_id, const char *name, hid_t type_id, hid_t space_id, hid_t dcpl_id) { - H5G_loc_t loc; /* Object location to insert dataset into */ - H5G_loc_t dset_loc; /* Object location of the dataset */ - H5F_t* file; /* File in which dataset is being created */ + H5G_loc_t loc; /* Object location to insert dataset into */ + H5G_loc_t dset_loc; /* Object location of the dataset */ + H5G_loc_t insertion_loc; /* Loc of group in which to create object */ + H5G_name_t insert_path; /* Path of group in which to create object */ + H5O_loc_t insert_oloc; /* oloc of group in which to create object */ + hbool_t insert_loc_valid = FALSE; /* Is insertion_loc valid? */ + H5F_t* file; /* File in which dataset is being created */ H5D_t *new_dset = NULL; /* New dataset's info */ const H5S_t *space; /* Dataspace for dataset */ hid_t dset_id = -1; /* New dataset's id */ @@ -1222,8 +1226,13 @@ H5Dcreate(hid_t loc_id, const char *name, hid_t type_id, hid_t space_id, HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not dataset create property list ID") /* What file is the dataset being added to? */ - if(NULL == (file = H5G_insertion_file(&loc, name, H5AC_dxpl_id))) + insertion_loc.path = &insert_path; + insertion_loc.oloc = &insert_oloc; + H5G_loc_reset(&insertion_loc); + if(H5G_insertion_loc(&loc, name, &insertion_loc, H5AC_dxpl_id) < 0) HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to locate insertion point") + insert_loc_valid=TRUE; + file = insertion_loc.oloc->file; /* build and open the new dataset */ if(NULL == (new_dset = H5D_create(file, type_id, space, dcpl_id, H5AC_dxpl_id))) @@ -1243,6 +1252,10 @@ H5Dcreate(hid_t loc_id, const char *name, hid_t type_id, hid_t space_id, ret_value = dset_id; done: + if(insert_loc_valid) { + if(H5G_loc_free(&insertion_loc) < 0) + HDONE_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "unable to free location") + } if(ret_value < 0) { if(dset_id >= 0) { @@ -1372,7 +1385,7 @@ H5Dopen(hid_t loc_id, const char *name) H5G_loc_t dset_loc; /* Object location of dataset */ H5G_name_t path; /* Dataset group hier. path */ H5O_loc_t oloc; /* Dataset object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Location at 'name' found */ hid_t dxpl_id = H5AC_dxpl_id; /* dxpl to use to open datset */ hid_t ret_value; @@ -1393,7 +1406,7 @@ H5Dopen(hid_t loc_id, const char *name) /* Find the dataset object */ if(H5G_loc_find(&loc, name, &dset_loc, H5P_DEFAULT, dxpl_id) < 0) HGOTO_ERROR(H5E_DATASET, H5E_NOTFOUND, FAIL, "not found") - ent_found = TRUE; + loc_found = TRUE; /* Check that the object found is the correct type */ if(H5O_obj_type(&oloc, dxpl_id) != H5G_DATASET) @@ -1401,7 +1414,6 @@ H5Dopen(hid_t loc_id, const char *name) /* Open the dataset */ if((dset = H5D_open(&dset_loc, dxpl_id)) == NULL) { - ent_found = FALSE; /* Reset this, since H5D_open 'owns' it and then free's it on failure */ HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "can't open dataset") } /* end if */ @@ -1416,8 +1428,10 @@ done: HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release dataset") } /* end if */ else { - if(ent_found) - H5G_name_free(&path); + if(loc_found) { + if(H5G_loc_free(&dset_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } } /* end else */ } /* end if */ @@ -1450,7 +1464,7 @@ H5Dopen_expand(hid_t loc_id, const char *name, hid_t dapl_id) H5G_loc_t dset_loc; /* Object location of dataset */ H5G_name_t path; /* Dataset group hier. path */ H5O_loc_t oloc; /* Dataset object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Location at 'name' found */ hid_t dxpl_id = H5AC_dxpl_id; /* dxpl to use to open datset */ hid_t ret_value; @@ -1478,7 +1492,7 @@ H5Dopen_expand(hid_t loc_id, const char *name, hid_t dapl_id) /* Find the dataset object */ if(H5G_loc_find(&loc, name, &dset_loc, dapl_id, dxpl_id) < 0) HGOTO_ERROR(H5E_DATASET, H5E_NOTFOUND, FAIL, "not found") - ent_found = TRUE; + loc_found = TRUE; /* Check that the object found is the correct type */ if(H5O_obj_type(&oloc, dxpl_id) != H5G_DATASET) @@ -1486,7 +1500,6 @@ H5Dopen_expand(hid_t loc_id, const char *name, hid_t dapl_id) /* Open the dataset */ if((dset = H5D_open(&dset_loc, dxpl_id)) == NULL) { - ent_found = FALSE; /* Reset this, since H5D_open 'owns' it and then free's it on failure */ HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "can't open dataset") } /* end if */ @@ -1501,8 +1514,10 @@ done: HDONE_ERROR(H5E_DATASET, H5E_CLOSEERROR, FAIL, "unable to release dataset") } /* end if */ else { - if(ent_found) - H5G_name_free(&path); + if(loc_found) { + if(H5G_loc_free(&dset_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } } /* end else */ } /* end if */ @@ -2647,12 +2662,14 @@ H5D_open(const H5G_loc_t *loc, hid_t dxpl_id) done: if(ret_value == NULL) { + /* Free the location--casting away const*/ if(dataset) { - /* Free the dataset's group hier. path */ - H5G_name_free(&dataset->path); - if(shared_fo == NULL) /* Need to free shared fo */ H5FL_FREE(H5D_shared_t, dataset->shared); + + H5O_loc_free(&(dataset->oloc)); + H5O_loc_free(&(dataset->path)); + H5FL_FREE(H5D_t, dataset); } if(shared_fo) diff --git a/src/H5Fmount.c b/src/H5Fmount.c index 75af431..20950d0 100644 --- a/src/H5Fmount.c +++ b/src/H5Fmount.c @@ -141,6 +141,7 @@ H5F_mount(H5G_loc_t *loc, const char *name, H5F_t *child, /* * Check that the child isn't mounted, that the mount point exists, that + * the mount point wasn't reached via external link, that * the parent & child files have the same file close degree, and * that the mount wouldn't introduce a cycle in the mount tree. */ @@ -148,6 +149,14 @@ H5F_mount(H5G_loc_t *loc, const char *name, H5F_t *child, HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "file is already mounted") if(H5G_loc_find(loc, name, &mp_loc/*out*/, H5P_DEFAULT, dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "group not found") + /* If the mount location is holding its file open, that file will close + * and remove the mount as soon as we exit this function. Prevent the + * user from doing this. + */ + if(mp_loc.oloc->holding_file != FALSE) + HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "mount path cannot contain links to external files") + + /* Open the mount point group */ if(NULL == (mount_point = H5G_open(&mp_loc, dxpl_id))) HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "mount point not found") @@ -229,9 +238,16 @@ H5F_mount(H5G_loc_t *loc, const char *name, H5F_t *child, HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "unable to replace name") done: - if(ret_value < 0 && mount_point) - if(H5G_close(mount_point) < 0) - HDONE_ERROR(H5E_FILE, H5E_CANTCLOSEOBJ, FAIL, "unable to close mounted group") + if(ret_value < 0) { + if(mount_point) { + if(H5G_close(mount_point) < 0) + HDONE_ERROR(H5E_FILE, H5E_CANTCLOSEOBJ, FAIL, "unable to close mounted group") + } + else { + if(H5G_loc_free(&mp_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "unable to free mount location") + } + } FUNC_LEAVE_NOAPI(ret_value) } /* end H5F_mount() */ diff --git a/src/H5G.c b/src/H5G.c index 4582a0d..468181f 100644 --- a/src/H5G.c +++ b/src/H5G.c @@ -100,7 +100,7 @@ /* User data for path traversal routine for "insertion file" routine */ typedef struct { - H5F_t *file; /* Pointer to the file for insertion */ + H5G_loc_t *loc; /* Pointer to the location for insertion */ } H5G_trav_ud1_t; /* User data for path traversal routine for getting object info */ @@ -138,14 +138,14 @@ static H5G_t *H5G_create(H5F_t *file, hid_t dxpl_id, hid_t gcpl_id, hid_t gapl_i static herr_t H5G_open_oid(H5G_t *grp, hid_t dxpl_id); static herr_t H5G_get_objinfo_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5G_set_comment(H5G_loc_t *loc, const char *name, const char *buf, hid_t dxpl_id); static int H5G_get_comment(H5G_loc_t *loc, const char *name, size_t bufsize, char *buf, hid_t dxpl_id); -static herr_t H5G_insertion_file_cb(H5G_loc_t *grp_loc/*in*/, const char *name, +static herr_t H5G_insertion_loc_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5G_copy(H5G_loc_t *src_loc, H5G_loc_t *dst_loc, const char *dst_name, hid_t ocpypl_id, hid_t lcpl_id); @@ -183,6 +183,10 @@ H5Gcreate(hid_t loc_id, const char *name, size_t size_hint) H5G_loc_t loc; H5G_loc_t grp_loc; H5G_t *grp = NULL; + H5G_loc_t insertion_loc; /* Loc of group in which to create object */ + H5G_name_t insert_path; /* Path of group in which to create object */ + H5O_loc_t insert_oloc; /* oloc of group in which to create object */ + hbool_t insert_loc_valid = FALSE; /* Is insertion_loc valid? */ H5F_t *file; /* File the group will be inserted into */ hid_t tmp_gcpl = (-1); /* Temporary group creation property list */ hid_t grp_id = (-1); /* ID of group being created */ @@ -230,8 +234,13 @@ H5Gcreate(hid_t loc_id, const char *name, size_t size_hint) /* What file is the group being added to? This may not be the same file * that loc_id is in if mounting is being used. */ - if(NULL == (file = H5G_insertion_file(&loc, name, H5AC_dxpl_id))) - HGOTO_ERROR(H5E_SYM, H5E_CANTINIT, FAIL, "unable to locate insertion point") + insertion_loc.path = &insert_path; + insertion_loc.oloc = &insert_oloc; + H5G_loc_reset(&insertion_loc); + if(H5G_insertion_loc(&loc, name, &insertion_loc, H5AC_dxpl_id) < 0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to locate insertion point") + insert_loc_valid=TRUE; + file = insertion_loc.oloc->file; /* Create the group */ if(NULL == (grp = H5G_create(file, H5AC_dxpl_id, tmp_gcpl, H5P_DEFAULT))) @@ -254,6 +263,10 @@ done: if(H5I_dec_ref(tmp_gcpl) < 0) HDONE_ERROR(H5E_SYM, H5E_CLOSEERROR, FAIL, "unable to release property list") + if(insert_loc_valid) { + if(H5G_loc_free(&insertion_loc) < 0) + HDONE_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "unable to free location") + } if(ret_value < 0) { if(grp_id >= 0) H5I_dec_ref(grp_id); @@ -373,7 +386,7 @@ H5Gopen(hid_t loc_id, const char *name) H5G_loc_t grp_loc; /* Location used to open group */ H5G_name_t grp_path; /* Opened object group hier. path */ H5O_loc_t grp_oloc; /* Opened object object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Location at 'name' found */ hid_t ret_value = FAIL; FUNC_ENTER_API(H5Gopen, FAIL) @@ -393,7 +406,7 @@ H5Gopen(hid_t loc_id, const char *name) /* Find the group object */ if(H5G_loc_find(&loc, name, &grp_loc/*out*/, H5P_DEFAULT, H5AC_dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "group not found") - ent_found = TRUE; + loc_found = TRUE; /* Check that the object found is the correct type */ if(H5O_obj_type(&grp_oloc, H5AC_dxpl_id) != H5G_GROUP) @@ -412,8 +425,10 @@ done: if(grp != NULL) H5G_close(grp); else { - if(ent_found) - H5G_name_free(&grp_path); + if(loc_found) { + if(H5G_loc_free(&grp_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } } /* end else */ } /* end if */ @@ -446,7 +461,7 @@ H5Gopen_expand(hid_t loc_id, const char *name, hid_t gapl_id) H5G_loc_t grp_loc; /* Location used to open group */ H5G_name_t grp_path; /* Opened object group hier. path */ H5O_loc_t grp_oloc; /* Opened object object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Location at 'name' found */ hid_t ret_value=FAIL; /* Return value */ FUNC_ENTER_API(H5Gopen_expand, FAIL); @@ -473,7 +488,7 @@ H5Gopen_expand(hid_t loc_id, const char *name, hid_t gapl_id) /* Find the group object using the gapl passed in */ if(H5G_loc_find(&loc, name, &grp_loc/*out*/, gapl_id, H5AC_dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "group not found") - ent_found = TRUE; + loc_found = TRUE; /* Check that the object found is the correct type */ if(H5O_obj_type(&grp_oloc, H5AC_dxpl_id) != H5G_GROUP) @@ -492,8 +507,10 @@ done: if(grp != NULL) H5G_close(grp); else { - if(ent_found) - H5G_name_free(&grp_path); + if(loc_found) { + if(H5G_loc_free(&grp_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } } /* end else */ } /* end if */ @@ -984,7 +1001,7 @@ H5Gcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, /* for opening the destination object */ H5G_name_t src_path; /* Opened source object hier. path */ H5O_loc_t src_oloc; /* Opened source object object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Location at 'name' found */ hbool_t obj_open = FALSE; /* Entry at 'name' found */ herr_t ret_value = SUCCEED; /* Return value */ @@ -1011,7 +1028,7 @@ H5Gcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, /* Find the source object to copy */ if(H5G_loc_find(&loc, src_name, &src_loc/*out*/, H5P_DEFAULT, H5AC_dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "source object not found") - ent_found = TRUE; + loc_found = TRUE; if(H5O_open(&src_oloc) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTOPENOBJ, FAIL, "unable to open object") @@ -1036,8 +1053,10 @@ H5Gcopy(hid_t src_loc_id, const char *src_name, hid_t dst_loc_id, HGOTO_ERROR(H5E_OHDR, H5E_CANTCOPY, FAIL, "unable to copy object") done: - if(ent_found) - H5G_name_free(&src_path); + if(loc_found) { + if(H5G_loc_free(&src_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } if (obj_open) H5O_close(&src_oloc); @@ -1561,7 +1580,11 @@ H5G_open(H5G_loc_t *loc, hid_t dxpl_id) done: if (!ret_value && grp) + { + H5O_loc_free(&(grp->oloc)); + H5G_name_free(&(grp->path)); H5FL_FREE(H5G_t,grp); + } FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_open() */ @@ -1839,7 +1862,7 @@ H5G_fileof(H5G_t *grp) */ static herr_t H5G_get_objinfo_cb(H5G_loc_t *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5G_trav_ud4_t *udata = (H5G_trav_ud4_t *)_udata; /* User data passed in */ herr_t ret_value = SUCCEED; /* Return value */ @@ -1899,7 +1922,7 @@ H5G_get_objinfo_cb(H5G_loc_t *grp_loc/*in*/, const char UNUSED *name, const H5O_ done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_get_objinfo_cb() */ @@ -1996,29 +2019,42 @@ done: static herr_t H5G_set_comment(H5G_loc_t *loc, const char *name, const char *buf, hid_t dxpl_id) { - H5O_loc_t obj_oloc; /* Object's location */ + H5G_loc_t obj_loc; /* Object's location */ + H5G_name_t path; + H5O_loc_t oloc; + hbool_t loc_valid = FALSE; H5O_name_t comment; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5G_set_comment) /* Get the symbol table entry for the object */ - if(H5G_obj_find(loc, name, H5G_TARGET_NORMAL, NULL, &obj_oloc/*out*/, H5P_DEFAULT, dxpl_id) < 0) + obj_loc.path = &path; + obj_loc.oloc = &oloc; + H5G_loc_reset(&obj_loc); + if(H5G_loc_find(loc, name, &obj_loc/*out*/, H5P_DEFAULT, dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "object not found") + loc_valid = TRUE; /* Remove the previous comment message if any */ - if(H5O_remove(&obj_oloc, H5O_NAME_ID, 0, TRUE, dxpl_id) < 0) + if(H5O_remove(obj_loc.oloc, H5O_NAME_ID, 0, TRUE, dxpl_id) < 0) H5E_clear_stack(NULL); /* Add the new message */ if(buf && *buf) { /* Casting away const OK -QAK */ comment.s = (char *)buf; - if(H5O_modify(&obj_oloc, H5O_NAME_ID, H5O_NEW_MESG, 0, H5O_UPDATE_TIME, &comment, dxpl_id) < 0) + if(H5O_modify(obj_loc.oloc, H5O_NAME_ID, H5O_NEW_MESG, 0, H5O_UPDATE_TIME, &comment, dxpl_id) < 0) HGOTO_ERROR(H5E_OHDR, H5E_CANTINIT, FAIL, "unable to set comment object header message") } /* end if */ done: + /* Release obj_loc */ + if(loc_valid) { + if(H5G_loc_free(&obj_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "unable to free location") + } + FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_set_comment() */ @@ -2043,18 +2079,25 @@ static int H5G_get_comment(H5G_loc_t *loc, const char *name, size_t bufsize, char *buf, hid_t dxpl_id) { H5O_name_t comment; - H5O_loc_t obj_oloc; /* Object's location */ - int ret_value; /* Return value */ + H5G_loc_t obj_loc; /* Object's location */ + H5G_name_t path; + H5O_loc_t oloc; + hbool_t loc_valid = FALSE; + int ret_value; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5G_get_comment) /* Get the symbol table entry for the object */ - if(H5G_obj_find(loc, name, H5G_TARGET_NORMAL, NULL, &obj_oloc/*out*/, H5P_DEFAULT, dxpl_id) < 0) + obj_loc.path = &path; + obj_loc.oloc = &oloc; + H5G_loc_reset(&obj_loc); + if(H5G_loc_find(loc, name, &obj_loc/*out*/, H5P_DEFAULT, dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "object not found") + loc_valid = TRUE; /* Get the message */ comment.s = NULL; - if(NULL == H5O_read(&obj_oloc, H5O_NAME_ID, 0, &comment, dxpl_id)) { + if(NULL == H5O_read(obj_loc.oloc, H5O_NAME_ID, 0, &comment, dxpl_id)) { if(buf && bufsize > 0) buf[0] = '\0'; ret_value = 0; @@ -2066,98 +2109,94 @@ H5G_get_comment(H5G_loc_t *loc, const char *name, size_t bufsize, char *buf, hid } /* end else */ done: + /* Release obj_loc */ + if(loc_valid) { + if(H5G_loc_free(&obj_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "unable to free location") + } + FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_get_comment() */ /*------------------------------------------------------------------------- - * Function: H5G_insertion_file_cb + * Function: H5G_insertion_loc_cb * - * Purpose: Callback for finding insertion file. This routine sets the - * correct information for the file pointer to return. + * Purpose: Callback for finding insertion location. This routine sets the + * correct information in the location passed in through the udata. * * Return: Non-negative on success/Negative on failure * - * Programmer: Quincey Koziol - * Monday, September 19, 2005 + * Programmer: James Laird + * Wednesday, August 16, 2006 * *------------------------------------------------------------------------- */ static herr_t -H5G_insertion_file_cb(H5G_loc_t *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t UNUSED *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) +H5G_insertion_loc_cb(H5G_loc_t *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t UNUSED *lnk, + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5G_trav_ud1_t *udata = (H5G_trav_ud1_t *)_udata; /* User data passed in */ herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_NOAPI_NOINIT(H5G_insertion_file_cb) + FUNC_ENTER_NOAPI_NOINIT(H5G_insertion_loc_cb) /* Check if the name in this group resolves to a valid location */ /* (which is not what we want) */ if(obj_loc != NULL) HGOTO_ERROR(H5E_SYM, H5E_EXISTS, FAIL, "name already exists") - /* Get file pointer for location */ - udata->file = grp_loc->oloc->file; - - *own_obj_loc = FALSE; + /* Take ownership of the grp_loc */ + if(H5G_loc_copy(udata->loc, grp_loc, H5_COPY_SHALLOW) < 0) + HGOTO_ERROR(H5E_SYM, H5E_CANTCOPY, FAIL, "couldn't take ownership of group location") + *own_loc = H5G_OWN_GRP_LOC; done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5G_insertion_file_cb() */ +} /* end H5G_insertion_loc_cb() */ /*------------------------------------------------------------------------- - * Function: H5G_insertion_file + * Function: H5G_insertion_loc * * Purpose: Given a location and name that specifies a not-yet-existing - * object return the file into which the object is about to be - * inserted. + * object return the location of the group into which the object + * is about to be inserted. * - * Return: Success: File pointer + * Return: Success: H5G_loc_t pointer + * (should be released with H5G_loc_free) * * Failure: NULL * - * Programmer: Robb Matzke - * Wednesday, October 14, 1998 + * Programmer: James Laird + * Wednesday, August 16, 2006 * *------------------------------------------------------------------------- */ -H5F_t * -H5G_insertion_file(H5G_loc_t *loc, const char *name, hid_t dxpl_id) +herr_t +H5G_insertion_loc(H5G_loc_t *src_loc, const char *name, + H5G_loc_t *insertion_loc/*out*/, hid_t dxpl_id) { - H5F_t *ret_value; /* Return value */ + H5G_trav_ud1_t udata; /* User data for traversal */ + herr_t ret_value = SUCCEED; /* Return value */ - FUNC_ENTER_NOAPI(H5G_insertion_file, NULL) + FUNC_ENTER_NOAPI(H5G_insertion_loc, FAIL) - HDassert(loc); + HDassert(src_loc); + HDassert(insertion_loc); HDassert(name && *name); - /* Check if the location the object will be inserted into is part of a - * file mounting chain (either a parent or a child) and perform a more - * rigorous determination of the location's file (which traverses into - * mounted files, etc.). + /* + * Look up the name to get the containing group's location and to make + * sure the name doesn't already exist. */ - if(H5F_has_mount(loc->oloc->file) || H5F_is_mount(loc->oloc->file)) { - H5G_trav_ud1_t udata; /* User data for traversal */ - - /* - * Look up the name to get the containing group and to make sure the name - * doesn't already exist. - */ - if(H5G_traverse(loc, name, H5G_TARGET_NORMAL, H5G_insertion_file_cb, &udata, H5P_DEFAULT, dxpl_id) < 0) - HGOTO_ERROR(H5E_SYM, H5E_EXISTS, NULL, "name already exists") - - /* Set return value */ - ret_value = udata.file; - } /* end if */ - else - /* Use the location's file */ - ret_value = loc->oloc->file; + udata.loc = insertion_loc; + if(H5G_traverse(src_loc, name, H5G_TARGET_NORMAL, H5G_insertion_loc_cb, &udata, H5P_DEFAULT, dxpl_id) < 0) + HGOTO_ERROR(H5E_SYM, H5E_EXISTS, FAIL, "name already exists") done: FUNC_LEAVE_NOAPI(ret_value) -} /* end H5G_insertion_file() */ +} /* end H5G_insertion_loc() */ /*------------------------------------------------------------------------- diff --git a/src/H5Gloc.c b/src/H5Gloc.c index c022169..791c278 100644 --- a/src/H5Gloc.c +++ b/src/H5Gloc.c @@ -47,7 +47,7 @@ typedef struct { /* PRIVATE PROTOTYPES */ static herr_t H5G_loc_find_cb(H5G_loc_t *grp_loc, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); /*------------------------------------------------------------------------- @@ -88,7 +88,10 @@ H5G_loc(hid_t loc_id, H5G_loc_t *loc) */ /* (but only for non-mounted files) */ if(!H5F_is_mount(f)) + { loc->oloc->file = f; + loc->oloc->holding_file = FALSE; + } } /* end case */ break; @@ -263,7 +266,9 @@ H5G_loc_free(H5G_loc_t *loc) HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, FAIL, "unable to free entry") #endif /* NOT_YET */ if(H5G_name_free(loc->path) < 0) - HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, FAIL, "unable to free path") + HGOTO_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "unable to free path") + if(H5O_loc_free(loc->oloc) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "unable to free object header location") done: FUNC_LEAVE_NOAPI(ret_value) @@ -284,7 +289,7 @@ done: */ static herr_t H5G_loc_find_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t UNUSED *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5G_loc_ud1_t *udata = (H5G_loc_ud1_t *)_udata; /* User data passed in */ herr_t ret_value = SUCCEED; /* Return value */ @@ -300,7 +305,7 @@ H5G_loc_find_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const * of the group location for the object, or freeing it. - QAK) */ H5G_loc_copy(udata->loc, obj_loc, H5_COPY_SHALLOW); - *own_obj_loc = TRUE; + *own_loc = H5G_OWN_OBJ_LOC; done: FUNC_LEAVE_NOAPI(ret_value) diff --git a/src/H5Gobj.c b/src/H5Gobj.c index 9b61a26..4cf99ef 100644 --- a/src/H5Gobj.c +++ b/src/H5Gobj.c @@ -190,6 +190,7 @@ H5G_obj_ent_decode(H5F_t *f, const uint8_t **pp, H5O_loc_t *oloc) /* Set file pointer for root object location */ oloc->file = f; + oloc->holding_file = FALSE; /* decode header */ *pp += H5F_SIZEOF_SIZE(f); /* Skip over local heap address */ @@ -418,6 +419,7 @@ H5G_obj_insert(H5O_loc_t *grp_oloc, const char *name, H5O_link_t *obj_lnk, /* Increment link count on object, if appropriate */ if(inc_link) { H5O_loc_t obj_oloc; /* Object location */ + H5O_loc_reset(&obj_oloc); /* Convert to object location */ obj_oloc.file = grp_oloc->file; @@ -888,101 +890,4 @@ done: } /* end H5G_obj_lookup() */ -/*------------------------------------------------------------------------- - * Function: H5G_obj_find_cb - * - * Purpose: Callback for retrieving object location for an object in a group - * - * Return: Non-negative on success/Negative on failure - * - * Programmer: Quincey Koziol - * Tuesday, September 20, 2005 - * - *------------------------------------------------------------------------- - */ -static herr_t -H5G_obj_find_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) -{ - H5G_obj_ud2_t *udata = (H5G_obj_ud2_t *)_udata; /* User data passed in */ - herr_t ret_value = SUCCEED; /* Return value */ - - FUNC_ENTER_NOAPI_NOINIT(H5G_obj_find_cb) - - /* Copy link for object */ - if(udata->lnk) { - /* Check if the name in this group resolved to a valid link */ - if(lnk == NULL) - HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "name doesn't exist") - - /* Copy link information */ -#ifdef H5_GROUP_REVISION - if(H5O_copy(H5O_LINK_ID, lnk, udata->lnk) == NULL) - HGOTO_ERROR(H5E_SYM, H5E_CANTCOPY, H5O_ITER_ERROR, "can't copy link message") -#else /* H5_GROUP_REVISION */ - *udata->lnk = *lnk; - HDassert(lnk->name); - udata->lnk->name = H5MM_xstrdup(lnk->name); - if(lnk->type == H5L_LINK_SOFT) - udata->lnk->u.soft.name = H5MM_xstrdup(lnk->u.soft.name); -#endif /* H5_GROUP_REVISION */ - } /* end if */ - - /* Copy object location for object */ - if(udata->oloc) { - /* Check if the name in this group resolved to a valid object */ - if(obj_loc == NULL) - HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "name doesn't exist") - - H5O_loc_copy(udata->oloc, obj_loc->oloc, H5_COPY_DEEP); - } /* end if */ - -done: - /* Indicate that this callback didn't take ownership of the group * - * location for the object */ - *own_obj_loc = FALSE; - - FUNC_LEAVE_NOAPI(ret_value) -} /* end H5G_obj_find_cb() */ - - -/*------------------------------------------------------------------------- - * Function: H5G_obj_find - * - * Purpose: Look up an object relative to a group. - * - * Return: Non-negative on success/Negative on failure - * - * Programmer: Quincey Koziol - * koziol@ncsa.uiuc.edu - * Sep 20 2005 - * - *------------------------------------------------------------------------- - */ -herr_t -H5G_obj_find(H5G_loc_t *loc, const char *name, unsigned traverse_flags, - H5O_link_t *lnk, H5O_loc_t *obj_oloc, hid_t lapl_id, hid_t dxpl_id) -{ - H5G_obj_ud2_t udata; /* User data for traversal callback */ - herr_t ret_value = SUCCEED; /* Return value */ - - FUNC_ENTER_NOAPI(H5G_obj_find, FAIL) - - /* check arguments */ - HDassert(loc && loc->oloc->file); - HDassert(name && *name); - HDassert(obj_oloc); - HDassert(H5P_CLS_LINK_ACCESS_g != -1); - - /* Set up user data for locating object */ - udata.lnk = lnk; - udata.oloc = obj_oloc; - - /* Traverse group hierarchy to locate object */ - if(H5G_traverse(loc, name, traverse_flags, H5G_obj_find_cb, &udata, lapl_id, dxpl_id) < 0) - HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "can't find object") - -done: - FUNC_LEAVE_NOAPI(ret_value) -} /* end H5G_obj_find() */ diff --git a/src/H5Gpkg.h b/src/H5Gpkg.h index 77eec67..aa7d582 100644 --- a/src/H5Gpkg.h +++ b/src/H5Gpkg.h @@ -246,12 +246,13 @@ typedef struct H5G_bt_it_ud5_t { * lnk is the link between the group and the object * obj_loc is the target of the traversal (or NULL if the object doesn't exist) * operator_data is whatever udata was supplied when H5G_traverse was called - * own_obj_loc should be set to TRUE if this callback takes ownership of obj_loc, - * and FALSE if obj_loc needs to be deleted. + * own_loc should be set to H5G_OWN_OBJ_LOC if this callback takes ownership of obj_loc, + * H5G_OWN_GRP_LOC if it takes ownership of grp_loc, and H5G_OWN_NONE if obj_loc and + * grp_loc need to be deleted. */ typedef herr_t (*H5G_traverse_t)(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk/*in*/, H5G_loc_t *obj_loc/*out*/, void *operator_data/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); /* * During name lookups (see H5G_traverse()) we sometimes want information about diff --git a/src/H5Gprivate.h b/src/H5Gprivate.h index d4278a4..acef4a3 100644 --- a/src/H5Gprivate.h +++ b/src/H5Gprivate.h @@ -87,6 +87,14 @@ typedef enum { H5G_NAME_UNMOUNT /* H5Funmount call */ } H5G_names_op_t; +/* Status returned from traversal callbacks; whether the object location + * or group location need to be closed */ +#define H5G_OWN_NONE 0 +#define H5G_OWN_OBJ_LOC 1 +#define H5G_OWN_GRP_LOC 2 +#define H5G_OWN_BOTH 3 +typedef int H5G_own_loc_t; + /* Structure to store information about the name an object was opened with */ typedef struct { H5RS_str_t *full_path_r; /* Path to object, as seen from root of current file mounting hierarchy */ @@ -124,7 +132,8 @@ H5_DLL H5G_t *H5G_open(H5G_loc_t *loc, hid_t dxpl_id); H5_DLL herr_t H5G_close(H5G_t *grp); H5_DLL herr_t H5G_get_objinfo(const H5G_loc_t *loc, const char *name, hbool_t follow_link, H5G_stat_t *statbuf/*out*/, hid_t dxpl_id); -H5_DLL H5F_t *H5G_insertion_file(H5G_loc_t *loc, const char *name, hid_t dxpl_id); +H5_DLL herr_t H5G_insertion_loc(H5G_loc_t *src_loc, const char *name, + H5G_loc_t *insertion_loc/*out*/, hid_t dxpl_id); H5_DLL herr_t H5G_free_grp_name(H5G_t *grp); H5_DLL herr_t H5G_get_shared_count(H5G_t *grp); H5_DLL herr_t H5G_mount(H5G_t *grp); diff --git a/src/H5Gtraverse.c b/src/H5Gtraverse.c index 9715785..4128604 100644 --- a/src/H5Gtraverse.c +++ b/src/H5Gtraverse.c @@ -54,7 +54,7 @@ static size_t H5G_comp_alloc_g = 0; /*sizeof component buffer */ /* PRIVATE PROTOTYPES */ static herr_t H5G_traverse_link_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5G_traverse_ud(H5G_loc_t *grp_loc/*in,out*/, H5O_link_t *lnk, H5G_loc_t *obj_loc/*in,out*/, size_t *nlinks/*in,out*/, hid_t lapl_id, hid_t dxpl_id); @@ -113,7 +113,7 @@ H5G_traverse_term_interface(void) */ static herr_t H5G_traverse_link_cb(H5G_loc_t UNUSED *grp_loc, const char UNUSED *name, const H5O_link_t UNUSED *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5G_trav_ud1_t *udata = (H5G_trav_ud1_t *)_udata; /* User data passed in */ herr_t ret_value = SUCCEED; /* Return value */ @@ -130,7 +130,7 @@ H5G_traverse_link_cb(H5G_loc_t UNUSED *grp_loc, const char UNUSED *name, const H done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_traverse_link_cb() */ @@ -155,13 +155,16 @@ static herr_t H5G_traverse_ud(H5G_loc_t *grp_loc/*in,out*/, H5O_link_t *lnk, { const H5L_link_class_t *link_class; /* User-defined link class */ hid_t cb_return = -1; /* The ID the user-defined callback returned */ + H5G_loc_t grp_loc_copy; + H5G_name_t grp_path_copy; + H5O_loc_t grp_oloc_copy; H5O_loc_t *new_oloc=NULL; H5F_t *temp_file=NULL; H5F_t *new_file=NULL; H5G_t *grp; H5P_genplist_t *lapl_default; H5P_genplist_t *lapl; /* LAPL with nlinks set */ - hid_t cur_grp; + hid_t cur_grp = (-1); herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5G_traverse_ud) @@ -181,8 +184,16 @@ static herr_t H5G_traverse_ud(H5G_loc_t *grp_loc/*in,out*/, H5O_link_t *lnk, if(NULL == (link_class = H5L_find_class(lnk->type))) HGOTO_ERROR(H5E_LINK, H5E_NOTREGISTERED, FAIL, "unable to get UD link class") - /* Set up location for user-defined callback */ - if((grp = H5G_open(grp_loc, dxpl_id)) == NULL) + /* Set up location for user-defined callback. Use a copy of our current + * grp_loc. */ + grp_loc_copy.path = &grp_path_copy; + grp_loc_copy.oloc = &grp_oloc_copy; + H5G_loc_reset(&grp_loc_copy); + if(H5G_loc_copy(&grp_loc_copy, grp_loc, H5_COPY_DEEP) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTCOPY, FAIL, "unable to copy object location") + + /* Create a group to pass to the user-defined callback */ + if((grp = H5G_open(&grp_loc_copy, dxpl_id)) == NULL) HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, FAIL, "unable to open group") if((cur_grp = H5I_register(H5I_GROUP, grp)) < 0) HGOTO_ERROR(H5E_ATOM, H5E_CANTREGISTER, FAIL, "unable to register group") @@ -233,26 +244,23 @@ static herr_t H5G_traverse_ud(H5G_loc_t *grp_loc/*in,out*/, H5O_link_t *lnk, HGOTO_ERROR(H5E_ATOM, H5E_BADTYPE, FAIL, "not a valid location or object ID") } + /* Copy the location the user returned to us */ if(H5O_loc_copy(obj_loc->oloc, new_oloc, H5_COPY_DEEP) < 0) HGOTO_ERROR(H5E_FILE, H5E_CANTCOPY, FAIL, "unable to copy object location") - /* The user has given us an open object, but we only want its location. However, - * if the object is in another file and we close it, the file will close as well and - * clear its information from the location we've just copied. - * Thus, we play with the number of open objects in the file to close the object without - * closing the file. -JML 5/06*/ - obj_loc->oloc->file->nopen_objs++; + /* Hold the file open until we free this object header (otherwise the + * object location will be invalidated when the file closes). + */ + if(H5O_loc_hold_file(obj_loc->oloc) <0) + HGOTO_ERROR(H5E_OHDR, H5E_LINKCOUNT, FAIL, "unable to hold file open") + + /* We have a copy of the location and we're holding the file open. + * Close the open ID the user passed back. + */ H5Idec_ref(cb_return); - HDassert(obj_loc->oloc->file->nopen_objs > 0); - obj_loc->oloc->file->nopen_objs--; done: - /* Close location given to callback. - * This has the side effect of calling H5F_try_close on grp_loc's file. - * If we have a series of external links (file1 to file2 to file3 to - * file4), this closes file2 and file3 when we're done traversing - * through them (unless they have other IDs holding them open). - */ + /* Close location given to callback. */ if(cur_grp > 0) { if(H5I_object_verify(cur_grp, H5I_GROUP)) @@ -267,7 +275,6 @@ done: } FUNC_LEAVE_NOAPI(ret_value) - } /*------------------------------------------------------------------------- @@ -299,7 +306,7 @@ H5G_traverse_slink(H5G_loc_t *grp_loc/*in,out*/, H5O_link_t *lnk, H5O_loc_t tmp_grp_oloc; /* Temporary copy of group entry */ H5G_name_t tmp_grp_path; /* Temporary copy of group's path */ H5G_loc_t tmp_grp_loc; /* Temporary copy of group's location */ - hbool_t tmp_grp_path_set = FALSE; /* Flag to indicate that tmp group path is initialized */ + hbool_t tmp_grp_loc_set = FALSE; /* Flag to indicate that tmp group location is initialized */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5G_traverse_slink) @@ -323,7 +330,7 @@ H5G_traverse_slink(H5G_loc_t *grp_loc/*in,out*/, H5O_link_t *lnk, * link traversal on the object's & group's paths - QAK) */ H5G_loc_copy(&tmp_grp_loc, grp_loc, H5_COPY_DEEP); - tmp_grp_path_set = TRUE; + tmp_grp_loc_set = TRUE; /* Hold the object's group hier. path to restore later */ /* (Part of "tracking the names properly") */ @@ -344,9 +351,9 @@ done: H5G_name_copy(obj_loc->path, &tmp_obj_path, H5_COPY_SHALLOW); } /* end if */ - /* Release cloned copy of group path */ - if(tmp_grp_path_set) - H5G_name_free(&tmp_grp_path); + /* Release cloned copy of group location */ + if(tmp_grp_loc_set) + H5G_loc_free(&tmp_grp_loc); FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_traverse_slink() */ @@ -447,13 +454,11 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, H5O_loc_t obj_oloc; /* Object found */ H5G_name_t obj_path; /* Path for object found */ H5G_loc_t obj_loc; /* Location of object */ - H5O_link_t *cb_lnk=NULL; /* Pointer to link info for callback */ - H5G_loc_t *cb_loc=NULL; /* Pointer to object location for callback */ size_t nchars; /* component name length */ H5O_link_t lnk; /* Link information for object */ hbool_t link_valid = FALSE; /* Flag to indicate that the link information is valid */ hbool_t obj_loc_valid = FALSE; /* Flag to indicate that the object location is valid */ - hbool_t own_cb_loc=FALSE; /* Flag to indicate that callback took ownership of cb_loc */ + H5G_own_loc_t own_loc=H5G_OWN_NONE; /* Enum to indicate whether callback took ownership of locations*/ hbool_t group_copy = FALSE; /* Flag to indicate that the group entry is copied */ hbool_t last_comp = FALSE; /* Flag to indicate that a component is the last component in the name */ herr_t ret_value = SUCCEED; /* Return value */ @@ -573,6 +578,7 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, /* Set the object location, if it's a hard link set the address also */ obj_loc.oloc->file = grp_loc.oloc->file; + obj_loc.oloc->holding_file = FALSE; if(lnk.type == H5L_LINK_HARD) obj_loc.oloc->addr = lnk.u.hard.addr; obj_loc_valid = TRUE; @@ -591,8 +597,8 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, } /* end if */ /* - * If we found an external link then we should follow it. But if this - * is the last component of the name and the H5G_TARGET_ELINK bit of + * If we found a user-defined link then we should follow it. But if this + * is the last component of the name and the H5G_TARGET_UDLINK bit of * TARGET is set then we don't follow it. */ if( lnk.type >= H5L_LINK_UD_MIN && ((0 == (target & H5G_TARGET_UDLINK)) || !last_comp) ) { @@ -618,12 +624,25 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, if(H5G_traverse_mount(&obj_loc/*in,out*/) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "mount point traversal failed") } /* end if */ + + /* If the grp_loc is the only thing holding an external file open + * and obj_loc is in the same file, obj_loc should also hold the + * file open so that closing the grp_loc doesn't close the file. + */ + if(grp_loc.oloc->holding_file && grp_loc.oloc->file == obj_loc.oloc->file) + if(H5O_loc_hold_file(obj_loc.oloc) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_LINKCOUNT, FAIL, "unable to hold file open") } /* end if */ /* Check for last component in name provided */ - if(last_comp) { + if(last_comp) + { + H5O_link_t *cb_lnk; /* Pointer to link info for callback */ + H5G_loc_t *cb_loc; /* Pointer to object location for callback */ + /* Set callback parameters appropriately, based on link being found */ if(lookup_status < 0) { + HDassert(!obj_loc_valid); cb_lnk = NULL; cb_loc = NULL; } /* end if */ @@ -632,11 +651,8 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, cb_loc = &obj_loc; } /* end else */ - /* Operator routine will take care of object location, succeed or fail */ - obj_loc_valid = FALSE; - /* Call 'operator' routine */ - if((op)(&grp_loc, H5G_comp_g, cb_lnk, cb_loc, op_data, &own_cb_loc) < 0) + if((op)(&grp_loc, H5G_comp_g, cb_lnk, cb_loc, op_data, &own_loc) < 0) HGOTO_ERROR(H5E_SYM, H5E_CALLBACK, FAIL, "traversal operator failed") HGOTO_DONE(SUCCEED) @@ -669,6 +685,14 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, /* Close new group */ if(H5O_close(obj_loc.oloc) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTINIT, FAIL, "unable to close") + + /* If the parent group was holding the file open, the + * newly-created group should, as well. + */ + if(grp_loc.oloc->holding_file) + if(H5O_loc_hold_file(obj_loc.oloc) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_LINKCOUNT, FAIL, "unable to hold file open") + } /* end if */ else HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "component not found") @@ -688,50 +712,37 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, name += nchars; } /* end while */ + /* Call 'operator' routine */ /* If we've fallen through to here, the name must be something like just '.' * and we should issue the callback on that. -QAK + * Since we don't have a group location or a link to the object we pass in + * NULL. */ - cb_loc = &grp_loc; - - /* Reset "group copied" flag (cb_loc will be freed automatically unless the - * callback takes ownership of it) */ HDassert(group_copy); - group_copy = FALSE; - - - /* Call 'operator' routine */ - if((op)(&grp_loc, ".", NULL, cb_loc, op_data, &own_cb_loc) < 0) + if((op)(NULL, ".", NULL, &grp_loc, op_data, &own_loc) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTNEXT, FAIL, "traversal operator failed") + /* If the callback took ownership of the object location, it actually has + * ownership of grp_loc. It shouldn't have tried to take ownership of + * the "group location", which was NULL. */ + HDassert(!(own_loc & H5G_OWN_GRP_LOC)); + if(own_loc & H5G_OWN_OBJ_LOC) + own_loc |= H5G_OWN_GRP_LOC; + HGOTO_DONE(SUCCEED) done: - /* If the operator routine didn't take ownership of the location we - * passed it (or if there was an error), free it. If it's in a new - * file, also try to close its file. */ - if(!own_cb_loc && cb_loc) - { - if(cb_loc->oloc->file != grp_loc.oloc->file) - { - if(H5F_try_close(cb_loc->oloc->file) < 0) - HDONE_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "unable to close external file opened during traversal") - } - H5G_loc_free(cb_loc); - } + /* If there's been an error, the callback doesn't really get ownership of + * any location and we should close them both */ + if(ret_value < 0) + own_loc = H5G_OWN_NONE; - /* If the object location is still valid (usually in an error situation), reset it */ - if(obj_loc_valid) - { - if(ret_value < 0) - { - H5F_try_close(obj_loc.oloc->file); - } + /* Free all open locations. This also closes any open external files. */ + if(obj_loc_valid && !(own_loc & H5G_OWN_OBJ_LOC)) H5G_loc_free(&obj_loc); - } - if(ret_value < 0 && grp_loc.oloc->file) - { - H5F_try_close(grp_loc.oloc->file); - } + if(group_copy && !(own_loc & H5G_OWN_GRP_LOC)) + H5G_loc_free(&grp_loc); + /* If there's valid information in the link, reset it */ if(link_valid) { #ifdef H5_GROUP_REVISION @@ -743,9 +754,6 @@ done: lnk.name = H5MM_xfree(lnk.name); #endif /* H5_GROUP_REVISION */ } /* end if */ - /* If we copied something into the group location, free it */ - if(group_copy) - H5G_loc_free(&grp_loc); FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_traverse_real() */ diff --git a/src/H5L.c b/src/H5L.c index ed73380..e3e2313 100644 --- a/src/H5L.c +++ b/src/H5L.c @@ -90,7 +90,7 @@ typedef struct { /* Private prototypes */ static herr_t H5L_link_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5L_create_real(H5G_loc_t *link_loc, const char *link_name, H5G_name_t *obj_path, H5F_t *obj_file, H5O_link_t *lnk, hid_t lcpl_id, hid_t lapl_id, hid_t dxpl_id); @@ -101,12 +101,12 @@ static herr_t H5L_create_soft(const char *target_path, H5G_loc_t *cur_loc, const char *cur_name, hid_t lcpl_id, hid_t lapl_id, hid_t dxpl_id); static herr_t H5L_linkval_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5L_linkval(H5G_loc_t *loc, const char *name, size_t size, char *buf/*out*/, hid_t lapl_id, hid_t dxpl_id); static herr_t H5L_unlink_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5L_unlink(H5G_loc_t *loc, const char *name, hid_t lapl_id, hid_t dxpl_id); static herr_t H5L_move(H5G_loc_t *src_loc, const char *src_name, @@ -114,13 +114,13 @@ static herr_t H5L_move(H5G_loc_t *src_loc, const char *src_name, hid_t lcpl_id, hid_t lapl_id, hid_t dxpl_id); static herr_t H5L_move_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5L_move_dest_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static herr_t H5L_get_linfo_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t *lnk, H5G_loc_t UNUSED *obj_loc, void *_udata/*in,out*/, - hbool_t *own_obj_loc/*out*/); + H5G_own_loc_t *own_loc/*out*/); static int H5L_find_class_idx(H5L_link_t id); @@ -1050,11 +1050,10 @@ H5L_link(H5G_loc_t *new_loc, const char *new_name, H5G_loc_t *obj_loc, HDassert(obj_loc); HDassert(new_name && *new_name); - /* Check that the object is not being hard linked into a different file */ - if(NULL == (file = H5G_insertion_file(new_loc, new_name, dxpl_id))) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "unable to identify insertion file") - if(obj_loc->oloc->file != file) - HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "cannot link an object from another file") + /* The link callback will check that the object isn't being hard linked + * into a different file, so we don't need to do it here (there could be + * external links along the path). + */ /* Construct link information for eventual insertion */ lnk.type = H5L_LINK_HARD; @@ -1083,7 +1082,7 @@ done: */ static herr_t H5L_link_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t UNUSED *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5L_trav_ud3_t *udata = (H5L_trav_ud3_t *)_udata; /* User data passed in */ H5G_t *grp=NULL; /* H5G_t for this group, opened to pass to user callback */ @@ -1173,7 +1172,7 @@ done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5L_link_cb() */ @@ -1307,7 +1306,10 @@ H5L_create_hard(H5G_loc_t *cur_loc, const char *cur_name, char *norm_cur_name = NULL; /* Pointer to normalized current name */ H5F_t *link_file = NULL; /* Pointer to file to link to */ H5O_link_t lnk; /* Link to insert */ - H5O_loc_t obj_oloc; /* Location of object to link to */ + H5G_loc_t obj_loc; /* Location of object to link to */ + H5G_name_t path; /* obj_loc's path*/ + H5O_loc_t oloc; /* obj_loc's oloc */ + hbool_t loc_valid = FALSE; herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5L_create_hard) @@ -1326,14 +1328,18 @@ H5L_create_hard(H5G_loc_t *cur_loc, const char *cur_name, lnk.type = H5L_LINK_HARD; /* Get object location for object pointed to */ - if(H5G_obj_find(cur_loc, norm_cur_name, H5G_TARGET_NORMAL, NULL, &obj_oloc, lapl_id, dxpl_id) < 0) + obj_loc.path = &path; + obj_loc.oloc = &oloc; + H5G_loc_reset(&obj_loc); + if(H5G_loc_find(cur_loc, norm_cur_name, &obj_loc, lapl_id, dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "source object not found") + loc_valid = TRUE; /* Construct link information for eventual insertion */ - lnk.u.hard.addr = obj_oloc.addr; + lnk.u.hard.addr = obj_loc.oloc->addr; /* Set destination's file information */ - link_file = obj_oloc.file; + link_file = obj_loc.oloc->file; /* Create actual link to the object. Pass in NULL for the path, since this * function shouldn't change an object's user path. */ @@ -1342,6 +1348,12 @@ H5L_create_hard(H5G_loc_t *cur_loc, const char *cur_name, HGOTO_ERROR(H5E_LINK, H5E_CANTINIT, FAIL, "unable to create new link to object") done: + /* Free the object header location */ + if(loc_valid) { + if(H5G_loc_free(&obj_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "unable to free location") + } + /* Free the normalized path name */ if(norm_cur_name) H5MM_xfree(norm_cur_name); @@ -1469,7 +1481,7 @@ done: */ static herr_t H5L_linkval_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t *lnk, - H5G_loc_t UNUSED *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t UNUSED *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5L_trav_ud5_t *udata = (H5L_trav_ud5_t *)_udata; /* User data passed in */ const H5L_link_class_t *link_class; /* User-defined link class */ @@ -1509,7 +1521,7 @@ H5L_linkval_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5L_linkval_cb() */ @@ -1570,7 +1582,7 @@ done: */ static herr_t H5L_unlink_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t UNUSED *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5G_t *grp=NULL; /* H5G_t for this group, opened to pass to user callback */ hid_t grp_id = FAIL; /* Id for this group (passed to user callback */ @@ -1637,7 +1649,7 @@ done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5L_unlink_cb() */ @@ -1703,7 +1715,7 @@ done: */ static herr_t H5L_move_dest_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5L_trav_ud10_t *udata = (H5L_trav_ud10_t *)_udata; /* User data passed in */ H5RS_str_t *dst_name_r = NULL; /* Ref-counted version of dest name */ @@ -1794,7 +1806,7 @@ done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; if(dst_name_r) H5RS_decr(dst_name_r); @@ -1818,7 +1830,7 @@ done: */ static herr_t H5L_move_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5L_trav_ud4_t *udata = (H5L_trav_ud4_t *)_udata; /* User data passed in */ H5L_trav_ud10_t udata_out; /* User data for H5L_move_dest_cb traversal */ @@ -1890,7 +1902,7 @@ done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5L_move_cb() */ @@ -2002,7 +2014,7 @@ done: */ static herr_t H5L_get_linfo_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H5O_link_t *lnk, - H5G_loc_t *obj_loc, void *_udata/*in,out*/, hbool_t *own_obj_loc/*out*/) + H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/) { H5L_trav_ud1_t *udata = (H5L_trav_ud1_t *)_udata; /* User data passed in */ H5L_linkinfo_t *linfo = udata->linfo; @@ -2059,7 +2071,7 @@ H5L_get_linfo_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const done: /* Indicate that this callback didn't take ownership of the group * * location for the object */ - *own_obj_loc = FALSE; + *own_loc = H5G_OWN_NONE; FUNC_LEAVE_NOAPI(ret_value) } /* end H5L_get_linfo_cb() */ diff --git a/src/H5Lexternal.c b/src/H5Lexternal.c index 312028a..612c4c6 100644 --- a/src/H5Lexternal.c +++ b/src/H5Lexternal.c @@ -52,7 +52,6 @@ static hid_t H5L_extern_traverse(const char * link_name, hid_t cur_group, void * char *obj_name; char *prefix; size_t fname_len; - htri_t result; hbool_t fname_alloc = FALSE; hid_t ret_value = -1; diff --git a/src/H5O.c b/src/H5O.c index 878e73a..efe1ee3 100644 --- a/src/H5O.c +++ b/src/H5O.c @@ -259,7 +259,7 @@ H5Oopen(hid_t loc_id, const char *name, hid_t lapl_id) H5G_loc_t obj_loc; /* Location used to open group */ H5G_name_t obj_path; /* Opened object group hier. path */ H5O_loc_t obj_oloc; /* Opened object object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Entry at 'name' found */ hid_t ret_value = FAIL; FUNC_ENTER_API(H5Oopen, FAIL) @@ -279,15 +279,17 @@ H5Oopen(hid_t loc_id, const char *name, hid_t lapl_id) /* Find the object's location */ if(H5G_loc_find(&loc, name, &obj_loc/*out*/, lapl_id, H5AC_dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "group not found") - ent_found = TRUE; + loc_found = TRUE; if((ret_value = H5O_open_by_loc(&obj_loc, H5AC_dxpl_id)) < 0) HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, FAIL, "unable to open object") done: if(ret_value < 0) { - if(ent_found) - H5G_name_free(&obj_path); + if(loc_found) { + if(H5G_loc_free(&obj_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } } /* end if */ FUNC_LEAVE_API(ret_value) @@ -337,7 +339,7 @@ H5Oopen_by_addr(hid_t loc_id, haddr_t addr) H5G_loc_t obj_loc; /* Location used to open group */ H5G_name_t obj_path; /* Opened object group hier. path */ H5O_loc_t obj_oloc; /* Opened object object location */ - hbool_t ent_found = FALSE; /* Entry at 'name' found */ + hbool_t loc_found = FALSE; /* Location at 'name' found */ hid_t ret_value = FAIL; FUNC_ENTER_API(H5Oopen_by_addr, FAIL) @@ -362,8 +364,10 @@ H5Oopen_by_addr(hid_t loc_id, haddr_t addr) done: if(ret_value < 0) { - if(ent_found) - H5G_name_free(&obj_path); + if(loc_found) { + if(H5G_loc_free(&obj_loc) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTRELEASE, FAIL, "can't free location") + } } /* end if */ FUNC_LEAVE_API(ret_value) @@ -846,6 +850,9 @@ H5O_close(H5O_loc_t *loc) HGOTO_ERROR(H5E_OHDR, H5E_CANTCLOSEFILE, FAIL, "problem attempting file close") } /* end if */ + if(H5O_loc_free(loc) < 0) + HGOTO_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "problem attempting to free location") + done: FUNC_LEAVE_NOAPI(ret_value) } /* end H5O_close() */ @@ -4404,7 +4411,11 @@ H5O_loc_copy(H5O_loc_t *dst, const H5O_loc_t *src, H5_copy_depth_t depth) /* Deep copy the names */ if(depth == H5_COPY_DEEP) { - /* Nothing currently */ + /* If the original entry has holding open the file, this one should + * hold it open, too. + */ + if(src->holding_file) + dst->file->nopen_objs++; ; } else if(depth == H5_COPY_SHALLOW) { /* Discarding 'const' qualifier OK - QAK */ @@ -4416,6 +4427,81 @@ H5O_loc_copy(H5O_loc_t *dst, const H5O_loc_t *src, H5_copy_depth_t depth) /*------------------------------------------------------------------------- + * Function: H5O_loc_hold_file + * + * Purpose: Have this object header hold a file open until it is + * released. + * + * Return: Success: Non-negative + * Failure: Negative + * + * Programmer: James Laird + * Wednesday, August 16, 2006 + * + *------------------------------------------------------------------------- + */ +herr_t +H5O_loc_hold_file(H5O_loc_t *loc) +{ + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5O_loc_hold_file) + + /* Check arguments */ + HDassert(loc); + HDassert(loc->file); + + /* If this location is not already holding its file open, do so. */ + if(!loc->holding_file) + { + loc->file->nopen_objs++; + loc->holding_file = TRUE; + } + + FUNC_LEAVE_NOAPI(SUCCEED) +} /* end H5O_loc_hold_file() */ + + +/*------------------------------------------------------------------------- + * Function: H5O_loc_free + * + * Purpose: Release resources used by this object header location. + * Not to be confused with H5O_close; this is used on + * locations that don't correspond to open objects. + * + * Return: Success: Non-negative + * Failure: Negative + * + * Programmer: James Laird + * Wednesday, August 16, 2006 + * + *------------------------------------------------------------------------- + */ +herr_t +H5O_loc_free(H5O_loc_t *loc) +{ + herr_t ret_value = SUCCEED; + + FUNC_ENTER_NOAPI_NOINIT(H5O_loc_free) + + /* Check arguments */ + HDassert(loc); + + /* If this location is holding its file open try to close the file. */ + if(loc->holding_file) + { + loc->file->nopen_objs--; + loc->holding_file = FALSE; + if(loc->file->nopen_objs <= 0) { + if(H5F_try_close(loc->file) < 0) + HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEFILE, FAIL, "can't close file"); + } + } + +done: + FUNC_LEAVE_NOAPI(ret_value) +} /* end H5O_loc_free() */ + + +/*------------------------------------------------------------------------- * Function: H5O_copy_mesg_file * * Purpose: Copies a message to file. If MESG is is the null pointer then a null diff --git a/src/H5Oprivate.h b/src/H5Oprivate.h index 07d2468..4a6a0c5 100644 --- a/src/H5Oprivate.h +++ b/src/H5Oprivate.h @@ -65,6 +65,8 @@ typedef struct H5O_t H5O_t; typedef struct H5O_loc_t { H5F_t *file; /* File that object header is located within */ haddr_t addr; /* File address of object header */ + hbool_t holding_file; /* True if this object header has incremented + * its file's count of open objects. */ } H5O_loc_t; /* Settings/flags for copying an object */ @@ -383,6 +385,8 @@ H5_DLL void *H5O_link_copy(const void *_mesg, void *_dest, unsigned update_flags */ H5_DLL herr_t H5O_loc_reset(H5O_loc_t *loc); H5_DLL herr_t H5O_loc_copy(H5O_loc_t *dst, const H5O_loc_t *src, H5_copy_depth_t depth); +H5_DLL herr_t H5O_loc_hold_file(H5O_loc_t *loc); +H5_DLL herr_t H5O_loc_free(H5O_loc_t *loc); /* Layout operators */ H5_DLL size_t H5O_layout_meta_size(const H5F_t *f, const void *_mesg); diff --git a/src/H5Tcommit.c b/src/H5Tcommit.c index cccee9e..8527c96 100644 --- a/src/H5Tcommit.c +++ b/src/H5Tcommit.c @@ -79,6 +79,10 @@ H5Tcommit(hid_t loc_id, const char *name, hid_t type_id) { H5G_loc_t loc; H5G_loc_t type_loc; + H5G_loc_t insertion_loc; /* Loc of group in which to create object */ + H5G_name_t insert_path; /* Path of group in which to create object */ + H5O_loc_t insert_oloc; /* oloc of group in which to create object */ + hbool_t insert_loc_valid = FALSE; /* Is insertion_loc valid? */ H5F_t *file; H5T_t *type = NULL; hbool_t uncommit = FALSE; /* TRUE if H5T_commit needs to be undone */ @@ -96,9 +100,14 @@ H5Tcommit(hid_t loc_id, const char *name, hid_t type_id) if(NULL == (type = H5I_object_verify(type_id, H5I_DATATYPE))) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "not a datatype") - /* Find the insertion file */ - if(NULL == (file = H5G_insertion_file(&loc, name, H5AC_dxpl_id))) - HGOTO_ERROR(H5E_SYM, H5E_CANTINIT, FAIL, "unable to find insertion point") + /* What file is the datatype being added to? */ + insertion_loc.path = &insert_path; + insertion_loc.oloc = &insert_oloc; + H5G_loc_reset(&insertion_loc); + if(H5G_insertion_loc(&loc, name, &insertion_loc, H5AC_dxpl_id) < 0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to locate insertion point") + insert_loc_valid=TRUE; + file = insertion_loc.oloc->file; /* Record the type's state so that we can revert to it if linking fails */ old_state = type->shared->state; @@ -118,6 +127,10 @@ H5Tcommit(hid_t loc_id, const char *name, hid_t type_id) } done: + if(insert_loc_valid) { + if(H5G_loc_free(&insertion_loc) < 0) + HDONE_ERROR(H5E_OHDR, H5E_CANTRELEASE, FAIL, "unable to free location") + } /* If the datatype was committed but couldn't be linked, we need to return it to the state it was in * before it was committed. */ if(TRUE == uncommit) @@ -455,7 +468,7 @@ done: H5T_close(type); else { if(obj_found && H5F_addr_defined(type_loc.oloc->addr)) - H5G_name_free(type_loc.path); + H5G_loc_free(&type_loc); } /* end else */ } /* end if */ @@ -536,7 +549,7 @@ done: H5T_close(type); else { if(obj_found && H5F_addr_defined(type_loc.oloc->addr)) - H5G_name_free(type_loc.path); + H5G_loc_free(&type_loc); } /* end else */ } /* end if */ @@ -637,6 +650,10 @@ done: if(dt) { if(shared_fo == NULL) /* Need to free shared fo */ H5FL_FREE(H5T_shared_t, dt->shared); + + H5O_loc_free(&(dt->oloc)); + H5G_name_free(&(dt->path)); + H5FL_FREE(H5T_t, dt); } /* end if */ diff --git a/test/links.c b/test/links.c index 1f4b43b..2cbb282 100644 --- a/test/links.c +++ b/test/links.c @@ -1570,22 +1570,31 @@ external_link_root(hid_t fapl) /* Close external object (lets first file close) */ if(H5Gclose(gid) < 0) TEST_ERROR; - /* Close second file */ - if(H5Fclose(fid)<0) TEST_ERROR; - + /* Create a new object using H5Gcreate through the external link + * directly + */ + if((gid = H5Gcreate(fid, "ext_link/newer_group", (size_t)0)) < 0) TEST_ERROR - /* Open first file again and check on object created */ + /* Close file and group */ + if(H5Gclose(gid) < 0) TEST_ERROR; + if(H5Fclose(fid)<0) TEST_ERROR; + + /* Open first file again and check on objects created */ if((fid = H5Fopen(filename1, H5F_ACC_RDONLY, H5P_DEFAULT)) < 0) TEST_ERROR - /* Open object created through external link */ + /* Open objects created through external link */ if((gid = H5Gopen(fid, "new_group")) < 0) TEST_ERROR; + if((gid2 = H5Gopen(fid, "newer_group")) < 0) TEST_ERROR; - /* Check name */ + /* Check names */ if((name_len = H5Iget_name( gid, objname, (size_t)NAME_BUF_SIZE )) < 0) TEST_ERROR if(HDstrcmp(objname, "/new_group")) TEST_ERROR + if((name_len = H5Iget_name( gid2, objname, (size_t)NAME_BUF_SIZE )) < 0) TEST_ERROR + if(HDstrcmp(objname, "/newer_group")) TEST_ERROR - /* Close opened object */ - if(H5Gclose(gid) < 0) TEST_ERROR; + /* Close opened objects */ + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Gclose(gid2) < 0) TEST_ERROR /* Close first file */ if(H5Fclose(fid)<0) TEST_ERROR; @@ -3152,6 +3161,234 @@ error: /*------------------------------------------------------------------------- + * Function: external_link_closing + * + * Purpose: Test that files are closed correctly when traversing + * external links. + * + * Return: Success: 0 + * Failure: -1 + * + * Programmer: James Laird + * Wednesday, August 16, 2006 + * + *------------------------------------------------------------------------- + */ +static int +external_link_closing(hid_t fapl) +{ + hid_t fid1 = (-1), fid2 = (-1), fid3 = (-1), fid4=(-1); + hid_t gid=(-1), tid=(-1), tid2=(-1), sid=(-1), did=(-1); + hid_t lcpl_id=(-1); + hsize_t dims[2]; + char filename1[NAME_BUF_SIZE], + filename2[NAME_BUF_SIZE], + filename3[NAME_BUF_SIZE], + filename4[NAME_BUF_SIZE], /* Names of files to externally link across */ + buf[NAME_BUF_SIZE]; /* misc. buffer */ + H5L_linkinfo_t li; + H5G_stat_t sb; + hobj_ref_t obj_ref; + + TESTING("that external files are closed during traversal"); + + /* In this test, external links will go from file1 to file2 and from + * file2 to file3. + * Test that all functions that can traverse external files close + * the files they open. + * Test that providing unusual paths containing external links can't + * make HDF5 forget to close a file it opened. + */ + + /* Set up filenames */ + h5_fixname(FILENAME[3], fapl, filename1, sizeof filename1); + h5_fixname(FILENAME[4], fapl, filename2, sizeof filename2); + h5_fixname(FILENAME[5], fapl, filename3, sizeof filename3); + h5_fixname(FILENAME[6], fapl, filename4, sizeof filename4); + + /* Create four files */ + if((fid1 = H5Fcreate(filename1, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((fid2 = H5Fcreate(filename2, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((fid3 = H5Fcreate(filename3, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((fid4 = H5Fcreate(filename4, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + + /* Create a dataspace and a datatype so we can create/commit a dataset/datatype in the files */ + dims[0] = 2; + dims[1] = 2; + if((sid = H5Screate_simple(2, dims, NULL)) < 0) TEST_ERROR + if((tid = H5Tcopy(H5T_NATIVE_INT)) < 0) TEST_ERROR + if((tid2 = H5Tcopy(H5T_NATIVE_INT)) < 0) TEST_ERROR + + /* Create external links from each file to the next */ + if(H5Lcreate_external(filename2, "/", fid1, "elink", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lcreate_external(filename3, "/", fid2, "elink", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lcreate_external(filename4, "/", fid3, "elink", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + + /* Close all files but the first */ + if(H5Fclose(fid4) < 0) TEST_ERROR + if(H5Fclose(fid3) < 0) TEST_ERROR + if(H5Fclose(fid2) < 0) TEST_ERROR + + /* Test creating each kind of object */ + if((gid = H5Gcreate(fid1, "elink/elink/elink/group1", 0)) < 0) TEST_ERROR + if(H5Tcommit(fid1, "elink/elink/elink/type1", tid) < 0) TEST_ERROR + if((did = H5Dcreate(fid1, "elink/elink/elink/dataset1", tid2, sid, H5P_DEFAULT)) < 0) TEST_ERROR + /* Close objects */ + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Tclose(tid) < 0) TEST_ERROR + if(H5Dclose(did) < 0) TEST_ERROR + + /* Test that getting info works */ + if(H5Lget_linkinfo(fid1, "elink/elink/elink/type1", &li, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lget_linkinfo(fid1, "elink/elink/elink", &li, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Gget_objinfo(fid1, "elink/elink/elink/type1", TRUE, &sb) < 0) TEST_ERROR + if(H5Gget_objinfo(fid1, "elink/elink/elink", TRUE, &sb) < 0) TEST_ERROR + if(H5Gget_objinfo(fid1, "elink/elink/elink", FALSE, &sb) < 0) TEST_ERROR + + /* Test move */ + if(H5Lmove(fid1, "elink/elink/elink/group1", fid1, + "elink/elink/elink/group1_moved", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + /* Open file 4 so we can do some fancy things */ + if((fid4 = H5Fopen(filename4, H5F_ACC_RDWR, fapl)) < 0) TEST_ERROR + if(H5Lmove(fid1, "elink/elink/elink/type1", fid4, + "type1_moved", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lmove(fid4, "dataset1", fid1, + "elink/elink/elink/dataset1_moved", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + /* Close file 4 again */ + if(H5Fclose(fid4) < 0) TEST_ERROR + + /* Test copy (as of this test, it uses the same code as move) */ + if(H5Lcopy(fid1, "elink/elink/elink", fid1, + "elink/elink/elink_copied", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lcopy(fid1, "elink/elink/elink", fid1, + "elink/elink/elink/elink_copied2", H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + + /* Test H5Gset and get comment */ + if(H5Gset_comment(fid1, "elink/elink/elink/group1_moved", "comment") < 0) TEST_ERROR + if(H5Gget_comment(fid1, "elink/elink/elink/group1_moved", sizeof(buf), buf) < 0) TEST_ERROR + + /* Test H5*open */ + if((gid = H5Gopen(fid1, "elink/elink/elink/group1_moved")) < 0) TEST_ERROR + if((tid = H5Topen(fid1, "elink/elink/elink/type1_moved")) < 0) TEST_ERROR + if((did = H5Dopen(fid1, "elink/elink/elink/dataset1_moved")) < 0) TEST_ERROR + /* Close objects */ + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Tclose(tid) < 0) TEST_ERROR + if(H5Dclose(did) < 0) TEST_ERROR + + /* Test H5*open_expand */ + if((gid = H5Gopen_expand(fid1, "elink/elink/elink/group1_moved", H5P_DEFAULT)) < 0) TEST_ERROR + if((tid = H5Topen_expand(fid1, "elink/elink/elink/type1_moved", H5P_DEFAULT)) < 0) TEST_ERROR + if((did = H5Dopen_expand(fid1, "elink/elink/elink/dataset1_moved", H5P_DEFAULT)) < 0) TEST_ERROR + /* Close objects */ + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Tclose(tid) < 0) TEST_ERROR + if(H5Dclose(did) < 0) TEST_ERROR + + /* Test H5Oopen */ + if((did = H5Oopen(fid1, "elink/elink/elink/dataset1_moved", H5P_DEFAULT)) < 0) TEST_ERROR + if(H5Dclose(did) < 0) TEST_ERROR + + /* Test H5Fmount */ + if((gid = H5Gcreate(fid1, "elink/elink/elink/mnt", 0)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + H5E_BEGIN_TRY { + if(H5Fmount(fid1, "elink/elink/elink/mnt", fid1, H5P_DEFAULT) >= 0) TEST_ERROR + if(H5Funmount(fid1, "elink/elink/elink/mnt") >= 0) TEST_ERROR + } H5E_END_TRY + + /* Test H5Rcreate */ + if(H5Rcreate(&obj_ref, fid1, "elink/elink/elink/type1_moved", H5R_OBJECT, (-1)) < 0) TEST_ERROR + + /* Test unlink */ + if(H5Lunlink(fid1, "elink/elink/elink/group1_moved", H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lunlink(fid1, "elink/elink/elink/type1_moved", H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lunlink(fid1, "elink/elink/elink/dataset1_moved", H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lunlink(fid1, "elink/elink/elink_copied", H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lunlink(fid1, "elink/elink/elink/elink_copied2", H5P_DEFAULT) < 0) TEST_ERROR + + /* We've tested that the various functions above don't leave files open. + * Now test that we can't confuse HDF5 by giving unusual paths with external links + */ + /* Create an external link that points to another external link */ + if((fid2 = H5Fopen(filename2, H5F_ACC_RDWR, fapl)) < 0) TEST_ERROR + if(H5Lcreate_external(filename3, "/elink", fid2, "elink2", + H5P_DEFAULT, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Fclose(fid2) < 0) TEST_ERROR + + /* Do an external link traversal that recursively calls another external link. */ + if((gid = H5Gcreate(fid1, "elink/elink2/group2", 0)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + + /* Create two more groups so that the last three elements in the path are + * all within the same external file + */ + if((gid = H5Gcreate(fid1, "elink/elink2/group2/group3", 0)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + if((gid = H5Gcreate(fid1, "elink/elink2/group2/group3/group4", 0)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Gget_objinfo(fid1, "elink/elink2/group2/group3/group4", TRUE, &sb) < 0) TEST_ERROR + +#ifdef H5_GROUP_REVISION + /* Add a few regular groups and a soft link in file2 using intermediate group creation */ + if((lcpl_id = H5Pcreate(H5P_LINK_CREATE)) < 0) TEST_ERROR + if(H5Pset_create_intermediate_group(lcpl_id, TRUE) < 0) TEST_ERROR + if(H5Lcreate_soft("/elink2", fid1, "elink/file2group1/file2group2/slink", + lcpl_id, H5P_DEFAULT) < 0) TEST_ERROR + + /* Try to traverse this path. There are three soft traversals in a row; + * slink points to (file2)/elink2, which points to (file3)/elink, which + * points to file 4. + */ + if((gid = H5Gcreate(fid1, "elink/file2group1/file2group2/slink/group3", 0)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Lget_linkinfo(fid1, "elink/file2group1/file2group2/slink/group3", &li, H5P_DEFAULT) < 0) TEST_ERROR + + /* Some simpler tests */ + if((gid = H5Gcreate(fid1, "elink/file2group3", 0)) < 0) TEST_ERROR + if(H5Gclose(gid) < 0) TEST_ERROR + if(H5Lget_linkinfo(fid1, "elink/file2group3", &li, H5P_DEFAULT) < 0) TEST_ERROR + if(H5Lget_linkinfo(fid1, "elink/elink", &li, H5P_DEFAULT) < 0) TEST_ERROR + +#endif /* H5_GROUP_REVISION */ + + /* Close file1, the only file that should still be open */ + if(H5Fclose(fid1) < 0) TEST_ERROR + + /* Re-create each file. If they are hanging open, these creates will fail */ + if((fid1 = H5Fcreate(filename1, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((fid2 = H5Fcreate(filename2, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((fid3 = H5Fcreate(filename3, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if((fid4 = H5Fcreate(filename4, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + + /* Cleanup */ + if(H5Sclose(sid) < 0) TEST_ERROR + if(H5Tclose(tid2) < 0) TEST_ERROR + if(H5Fclose(fid4) < 0) TEST_ERROR + if(H5Fclose(fid3) < 0) TEST_ERROR + if(H5Fclose(fid2) < 0) TEST_ERROR + if(H5Fclose(fid1) < 0) TEST_ERROR + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY { + H5Gclose(gid); + H5Tclose(tid); + H5Dclose(did); + H5Sclose(sid); + H5Tclose(tid2); + H5Fclose(fid4); + H5Fclose(fid3); + H5Fclose(fid2); + H5Fclose(fid1); + } H5E_END_TRY; + return -1; +} + + +/*------------------------------------------------------------------------- * Function: ext_link_endian * * Purpose: Check that external links work properly when they are @@ -3167,7 +3404,7 @@ error: *------------------------------------------------------------------------- */ static int -ext_link_endian(hid_t fapl) +external_link_endian(hid_t fapl) { hid_t fid = (-1); /* File ID */ hid_t gid = (-1), gid2 = (-1); /* Group IDs */ @@ -4720,6 +4957,56 @@ linkinfo(hid_t fapl) } /* end ud_hard_links() */ +/*------------------------------------------------------------------------- + * Function: check_all_closed + * + * Purpose: External links and some UD links open files. To make sure + * that all such files got closed correctly, try to create + * each of them. + * + * If the files are still open, this will fail (indicating that + * some other test made a mistake). + * + * Return: Success: 0 + * Failure: -1 + * + * Programmer: James Laird + * Thursday, August 17, 2006 + * + *------------------------------------------------------------------------- + */ +static int +check_all_closed(hid_t fapl) +{ + hid_t fid=-1; + char filename[NAME_BUF_SIZE]; + int x; + + TESTING("that all files were closed correctly") + + /* Some of the external or UD link tests may have failed to close + * an external file properly. + * To check this, try to create every file used in this test. If + * a file is already open, creating it will fail. + */ + for(x=0; FILENAME[x] != NULL; x++) + { + h5_fixname(FILENAME[x], fapl, filename, sizeof filename); + + if((fid = H5Fcreate(filename, H5F_ACC_TRUNC, H5P_DEFAULT, fapl)) < 0) TEST_ERROR + if(H5Fclose(fid) < 0) TEST_ERROR + } + + PASSED(); + return 0; + + error: + H5E_BEGIN_TRY { + H5Fclose(fid); + } H5E_END_TRY; + return -1; +} + /*------------------------------------------------------------------------- * Function: main @@ -4789,7 +5076,8 @@ main(void) #ifdef H5_GROUP_REVISION nerrors += external_link_ride(fapl) < 0 ? 1 : 0; #endif /* H5_GROUP_REVISION */ - nerrors += ext_link_endian(fapl) < 0 ? 1 : 0; + nerrors += external_link_closing(fapl) < 0 ? 1 : 0; + nerrors += external_link_endian(fapl) < 0 ? 1 : 0; /* These tests assume that external links are a form of UD links, * so assume that everything that passed for external links @@ -4803,6 +5091,8 @@ main(void) nerrors += lapl_nlinks(fapl) < 0 ? 1 : 0; nerrors += linkinfo(fapl) < 0 ? 1 : 0; + nerrors += check_all_closed(fapl) < 0 ? 1 : 0; + /* Results */ if (nerrors) { printf("***** %d LINK TEST%s FAILED! *****\n", -- cgit v0.12