From 516c63299f638bf0f9ab6d58268e8edf28515ed4 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Tue, 15 Jul 2008 16:00:15 -0500 Subject: [svn-r15367] Description: Add check to avoid mounting the a file on a group twice, then the mounts are done on the same HDF5 file, but opened with separate H5Fopen calls. Also add new 'mounted' flag to the H5G_info_t struct, queried with the H5Gget_info() API call, to allow applications to detect and avoid this situation. This probably fixes Bz#1070 also, I'll check with Dan Anov (who reported a different sort of behavior, but seems to have the same underlying problem). Tested on: Mac OS X/32 10.5.4 (amazon) Linux/64 2.6 (chicago) --- src/H5Fmount.c | 6 ++- src/H5G.c | 24 ++++++++++ src/H5Gobj.c | 24 ++++++++++ src/H5Gprivate.h | 7 +++ src/H5Gpublic.h | 1 + test/mount.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 198 insertions(+), 3 deletions(-) diff --git a/src/H5Fmount.c b/src/H5Fmount.c index 470d323..c96c920 100644 --- a/src/H5Fmount.c +++ b/src/H5Fmount.c @@ -135,7 +135,7 @@ H5F_mount(H5G_loc_t *loc, const char *name, H5F_t *child, HDassert(child); HDassert(TRUE == H5P_isa_class(plist_id, H5P_FILE_MOUNT)); - /* Set up dataset location to fill in */ + /* Set up group location to fill in */ mp_loc.oloc = &mp_oloc; mp_loc.path = &mp_path; H5G_loc_reset(&mp_loc); @@ -161,6 +161,10 @@ H5F_mount(H5G_loc_t *loc, const char *name, H5F_t *child, if(NULL == (mount_point = H5G_open(&mp_loc, dxpl_id))) HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "mount point not found") + /* Check if the proposed mount point group is already a mount point */ + if(H5G_MOUNTED(mount_point)) + HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "mount point is already in use") + /* Retrieve information from the mount point group */ /* (Some of which we had before but was reset in mp_loc when the group * "took over" the group location - QAK) diff --git a/src/H5G.c b/src/H5G.c index 0fa2c1c..e04402e 100644 --- a/src/H5G.c +++ b/src/H5G.c @@ -1526,6 +1526,30 @@ H5G_mount(H5G_t *grp) /*------------------------------------------------------------------------- + * Function: H5G_mounted + * + * Purpose: Retrieves the 'mounted' flag for a group + * + * Return: Non-negative on success/Negative on failure + * + * Programmer: Quincey Koziol + * Tuesday, July 15, 2008 + * + *------------------------------------------------------------------------- + */ +hbool_t +H5G_mounted(H5G_t *grp) +{ + FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5G_mounted) + + /* Check args */ + HDassert(grp && grp->shared); + + FUNC_LEAVE_NOAPI(grp->shared->mounted) +} /* end H5G_mounted() */ + + +/*------------------------------------------------------------------------- * Function: H5G_unmount * * Purpose: Resets the 'mounted' flag for a group diff --git a/src/H5Gobj.c b/src/H5Gobj.c index 8ca8c52..e04c12d 100644 --- a/src/H5Gobj.c +++ b/src/H5Gobj.c @@ -702,6 +702,10 @@ done: herr_t H5G_obj_info(H5O_loc_t *oloc, H5G_info_t *grp_info, hid_t dxpl_id) { + H5G_t *grp = NULL; /* Group to query */ + H5G_loc_t grp_loc; /* Entry of group to be queried */ + H5G_name_t grp_path; /* Group hier. path */ + H5O_loc_t grp_oloc; /* Group object location */ H5O_linfo_t linfo; /* Link info message */ herr_t ret_value = SUCCEED; /* Return value */ @@ -711,6 +715,22 @@ H5G_obj_info(H5O_loc_t *oloc, H5G_info_t *grp_info, hid_t dxpl_id) HDassert(oloc); HDassert(grp_info); + /* Set up group location to fill in */ + grp_loc.oloc = &grp_oloc; + grp_loc.path = &grp_path; + H5G_loc_reset(&grp_loc); + + /* Deep copy (duplicate) of the group location object */ + if(H5O_loc_copy(&grp_oloc, oloc, H5_COPY_DEEP) < 0) + HGOTO_ERROR(H5E_SYM, H5E_CANTCOPY, FAIL, "can't copy object location") + + /* Open the group */ + if(NULL == (grp = H5G_open(&grp_loc, dxpl_id))) + HGOTO_ERROR(H5E_FILE, H5E_MOUNT, FAIL, "mount point not found") + + /* Get information from the group */ + grp_info->mounted = H5G_MOUNTED(grp); + /* Attempt to get the link info for this group */ if(H5G_obj_get_linfo(oloc, &linfo, dxpl_id)) { /* Retrieve the information about the links */ @@ -737,6 +757,10 @@ H5G_obj_info(H5O_loc_t *oloc, H5G_info_t *grp_info, hid_t dxpl_id) } /* end else */ done: + /* Clean up resources */ + if(grp && H5G_close(grp) < 0) + HDONE_ERROR(H5E_SYM, H5E_CANTCLOSEOBJ, FAIL, "unable to close queried group") + FUNC_LEAVE_NOAPI(ret_value) } /* end H5G_obj_info() */ diff --git a/src/H5Gprivate.h b/src/H5Gprivate.h index c54f11d..ab6abc1 100644 --- a/src/H5Gprivate.h +++ b/src/H5Gprivate.h @@ -101,6 +101,12 @@ H5G_CRT_GINFO_EST_NAME_LEN \ } +/* If the module using this macro is allowed access to the private variables, access them directly */ +#ifdef H5G_PACKAGE +#define H5G_MOUNTED(G) ((G)->shared->mounted) +#else /* H5G_PACKAGE */ +#define H5G_MOUNTED(G) (H5G_mounted(G)) +#endif /* H5G_PACKAGE */ /* Type of operation being performed for call to H5G_name_replace() */ typedef enum { @@ -158,6 +164,7 @@ H5_DLL herr_t H5G_close(H5G_t *grp); 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); +H5_DLL hbool_t H5G_mounted(H5G_t *grp); H5_DLL herr_t H5G_unmount(H5G_t *grp); #ifndef H5_NO_DEPRECATED_SYMBOLS H5_DLL H5G_obj_t H5G_map_obj_type(H5O_type_t obj_type); diff --git a/src/H5Gpublic.h b/src/H5Gpublic.h index e818e72..5ba7050 100644 --- a/src/H5Gpublic.h +++ b/src/H5Gpublic.h @@ -61,6 +61,7 @@ typedef struct H5G_info_t { H5G_storage_type_t storage_type; /* Type of storage for links in group */ hsize_t nlinks; /* Number of links in group */ int64_t max_corder; /* Current max. creation order value for group */ + hbool_t mounted; /* Whether group has a file mounted on it */ } H5G_info_t; /********************/ diff --git a/test/mount.c b/test/mount.c index e6aa881..1898a36 100644 --- a/test/mount.c +++ b/test/mount.c @@ -168,9 +168,9 @@ test_basic(hid_t fapl) static int test_illegal(hid_t fapl) { - hid_t file1 = -1, file2 = -1, file3 = -1, mnt = -1; - herr_t status; + hid_t file1 = -1, file2 = -1, file3 = -1, file4 = -1, mnt = -1; char filename1[1024], filename2[1024], filename3[1024]; + herr_t status; TESTING("illegal mount operations"); h5_fixname(FILENAME[0], fapl, filename1, sizeof filename1); @@ -183,6 +183,8 @@ test_illegal(hid_t fapl) (file2 = H5Fopen(filename2, H5F_ACC_RDONLY, fapl)) < 0 || (file3 = H5Fopen(filename3, H5F_ACC_RDONLY, fapl)) < 0) FAIL_STACK_ERROR + if((file4 = H5Fopen(filename3, H5F_ACC_RDONLY, fapl)) < 0) + FAIL_STACK_ERROR /* Try mounting a file on itself */ H5E_BEGIN_TRY { @@ -212,11 +214,32 @@ test_illegal(hid_t fapl) if(H5Funmount(mnt, ".") < 0) FAIL_STACK_ERROR if(H5Gclose(mnt) < 0) FAIL_STACK_ERROR + /* + * Try mounting the same file opened twice at the same place. + * + * We have to open the mount point before we mount the first file or we'll + * end up mounting file4 at the root of file3 and the mount will succeed. + */ + if((mnt = H5Gopen2(file1, "/mnt1", H5P_DEFAULT)) < 0) FAIL_STACK_ERROR + if(H5Fmount(mnt, ".", file3, H5P_DEFAULT) < 0) FAIL_STACK_ERROR + H5E_BEGIN_TRY { + status = H5Fmount(mnt, ".", file4, H5P_DEFAULT); + } H5E_END_TRY; + if(status >= 0) { + H5_FAILED(); + puts(" Mounting same file opened twice at one mount point should have failed."); + TEST_ERROR + } /* end if */ + if(H5Funmount(mnt, ".") < 0) FAIL_STACK_ERROR + if(H5Gclose(mnt) < 0) FAIL_STACK_ERROR + + /* Close everything and return */ if(H5Fclose(file1) < 0) FAIL_STACK_ERROR if(H5Fclose(file2) < 0) FAIL_STACK_ERROR if(H5Fclose(file3) < 0) FAIL_STACK_ERROR + if(H5Fclose(file4) < 0) FAIL_STACK_ERROR PASSED(); return 0; @@ -227,12 +250,123 @@ error: H5Fclose(file1); H5Fclose(file2); H5Fclose(file3); + H5Fclose(file4); } H5E_END_TRY; return 1; } /* end test_illegal() */ /*------------------------------------------------------------------------- + * Function: test_samefile + * + * Purpose: Test opening the same file twice and then mounting another + * file on each. + * + * Return: Success: 0 + * Failure: number of errors + * + * Programmer: Quincey Koziol + * Tuesday, July 15, 2008 + * + *------------------------------------------------------------------------- + */ +static int +test_samefile(hid_t fapl) +{ + hid_t file1a = -1, file1b = -1, file2 = -1, file3 = -1; + hid_t mnt1a = -1, mnt1b = -1; + char filename1[1024], filename2[1024], filename3[1024]; + H5G_info_t grp_info; + herr_t status; + + TESTING("same file mount operations"); + h5_fixname(FILENAME[0], fapl, filename1, sizeof filename1); + h5_fixname(FILENAME[1], fapl, filename2, sizeof filename2); + h5_fixname(FILENAME[2], fapl, filename3, sizeof filename3); + + + /* Open the files */ + if((file1a = H5Fopen(filename1, H5F_ACC_RDONLY, fapl)) < 0) + FAIL_STACK_ERROR + if((file1b = H5Fopen(filename1, H5F_ACC_RDONLY, fapl)) < 0) + FAIL_STACK_ERROR + if((file2 = H5Fopen(filename2, H5F_ACC_RDONLY, fapl)) < 0) + FAIL_STACK_ERROR + if((file3 = H5Fopen(filename3, H5F_ACC_RDONLY, fapl)) < 0) + FAIL_STACK_ERROR + + /* + * Try mounting different files at the same place in each of the "top" + * files. + * + * We have to open the mount point before we mount the first file or we'll + * end up mounting file4 at the root of file3 and the mount will succeed. + */ + if((mnt1a = H5Gopen2(file1a, "/mnt1", H5P_DEFAULT)) < 0) FAIL_STACK_ERROR + if(H5Gget_info(mnt1a, &grp_info) < 0) FAIL_STACK_ERROR + if(grp_info.mounted) FAIL_PUTS_ERROR(" Group shouldn't have 'mounted' flag set.") + if((mnt1b = H5Gopen2(file1b, "/mnt1", H5P_DEFAULT)) < 0) FAIL_STACK_ERROR + if(H5Gget_info(mnt1b, &grp_info) < 0) FAIL_STACK_ERROR + if(grp_info.mounted) FAIL_PUTS_ERROR(" Group shouldn't have 'mounted' flag set.") + if(H5Fmount(mnt1a, ".", file2, H5P_DEFAULT) < 0) FAIL_STACK_ERROR + if(H5Gget_info(mnt1a, &grp_info) < 0) FAIL_STACK_ERROR + if(!grp_info.mounted) FAIL_PUTS_ERROR(" Group should have 'mounted' flag set.") + H5E_BEGIN_TRY { + status = H5Fmount(mnt1b, ".", file3, H5P_DEFAULT); + } H5E_END_TRY; + if(status >= 0) FAIL_PUTS_ERROR(" Mounting different files at one mount point should have failed.") + if(H5Funmount(mnt1a, ".") < 0) FAIL_STACK_ERROR + if(H5Gclose(mnt1a) < 0) FAIL_STACK_ERROR + if(H5Gclose(mnt1b) < 0) FAIL_STACK_ERROR + + /* + * Try mounting same files at the same place in each of the "top" + * files. + * + * We have to open the mount point before we mount the first file or we'll + * end up mounting file4 at the root of file3 and the mount will succeed. + */ + if((mnt1a = H5Gopen2(file1a, "/mnt1", H5P_DEFAULT)) < 0) FAIL_STACK_ERROR + if(H5Gget_info(mnt1a, &grp_info) < 0) FAIL_STACK_ERROR + if(grp_info.mounted) FAIL_PUTS_ERROR(" Group shouldn't have 'mounted' flag set.") + if((mnt1b = H5Gopen2(file1b, "/mnt1", H5P_DEFAULT)) < 0) FAIL_STACK_ERROR + if(H5Gget_info(mnt1b, &grp_info) < 0) FAIL_STACK_ERROR + if(grp_info.mounted) FAIL_PUTS_ERROR(" Group shouldn't have 'mounted' flag set.") + if(H5Fmount(mnt1a, ".", file2, H5P_DEFAULT) < 0) FAIL_STACK_ERROR + if(H5Gget_info(mnt1a, &grp_info) < 0) FAIL_STACK_ERROR + if(!grp_info.mounted) FAIL_PUTS_ERROR(" Group should have 'mounted' flag set.") + H5E_BEGIN_TRY { + status = H5Fmount(mnt1b, ".", file2, H5P_DEFAULT); + } H5E_END_TRY; + if(status >= 0) FAIL_PUTS_ERROR(" Mounting same files at one mount point should have failed.") + if(H5Funmount(mnt1a, ".") < 0) FAIL_STACK_ERROR + if(H5Gclose(mnt1a) < 0) FAIL_STACK_ERROR + if(H5Gclose(mnt1b) < 0) FAIL_STACK_ERROR + + + /* Close everything and return */ + if(H5Fclose(file1a) < 0) FAIL_STACK_ERROR + if(H5Fclose(file1b) < 0) FAIL_STACK_ERROR + if(H5Fclose(file2) < 0) FAIL_STACK_ERROR + if(H5Fclose(file3) < 0) FAIL_STACK_ERROR + + PASSED(); + return 0; + +error: + H5E_BEGIN_TRY { + H5Gclose(mnt1a); + H5Gclose(mnt1b); + H5Fclose(file1a); + H5Fclose(file1b); + H5Fclose(file2); + H5Fclose(file3); + } H5E_END_TRY; + return 1; +} /* end test_samefile() */ + + +/*------------------------------------------------------------------------- * Function: test_hide * * Purpose: The previous contents of the mount point is temporarily @@ -3817,6 +3951,7 @@ main(void) nerrors += test_basic(fapl); nerrors += test_illegal(fapl); + nerrors += test_samefile(fapl); nerrors += test_hide(fapl); nerrors += test_assoc(fapl); nerrors += test_mntlnk(fapl); -- cgit v0.12