From 9c31ce9a7b2b98416c0bfd90bccc277693341083 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Wed, 10 Sep 2003 13:33:48 -0500 Subject: [svn-r7458] Purpose: Bug fix. Description: Correct bug where a file opened twice, once with read-write permission and once with read-only permission would cause closing the file with the read-only file ID to fail because it was trying to flush information out of the file. Solution: Check the permissions on file IDs that are being closed and only flush when the particular file ID was opened with write permission. Platforms tested: FreeBSD 4.9 (sleipnir) too small to need h5committest --- release_docs/RELEASE.txt | 3 +++ src/H5F.c | 47 +++++++++++++++++++------------------ test/tfile.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 22 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 12f3900..d688f91 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -56,6 +56,9 @@ Bug Fixes since HDF5-1.6.0 release Library ------- + - Corrected bug when opening a file twice with read-only permission + for one open and then closing the read-only access file ID would + generate an error. QAK - 2003/09/10 - Corrected bug in repeated calls to H5Pget_access_plist() which would incorrectly manage reference counts of internal information and eventually blow up. QAK - 2003/09/02 diff --git a/src/H5F.c b/src/H5F.c index a99c3e7..44e56fb 100644 --- a/src/H5F.c +++ b/src/H5F.c @@ -3102,36 +3102,39 @@ H5F_close(H5F_t *f) H5AC_debug(f); H5F_istore_stats(f, FALSE); + /* Only try to flush the file if it was opened with write access */ + if(f->intent&H5F_ACC_RDWR) { #ifdef H5_HAVE_FPHDF5 - /* - * We only want the captain to perform the flush of the metadata - * to the file. - */ - if (!H5FD_is_fphdf5_driver(f->shared->lf) || - H5FD_fphdf5_is_captain(f->shared->lf)) { + /* + * We only want the captain to perform the flush of the metadata + * to the file. + */ + if (!H5FD_is_fphdf5_driver(f->shared->lf) || + H5FD_fphdf5_is_captain(f->shared->lf)) { #endif /* H5_HAVE_FPHDF5 */ - /* Flush and destroy all caches */ - if (H5F_flush(f, H5AC_dxpl_id, H5F_SCOPE_LOCAL, - H5F_FLUSH_INVALIDATE | H5F_FLUSH_CLOSING) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache"); + /* Flush and destroy all caches */ + if (H5F_flush(f, H5AC_dxpl_id, H5F_SCOPE_LOCAL, + H5F_FLUSH_INVALIDATE | H5F_FLUSH_CLOSING) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache"); #ifdef H5_HAVE_FPHDF5 - } else { - /* - * If this isn't the captain process, flush but only clear - * the flags. - */ - if (H5F_flush(f, H5AC_dxpl_id, H5F_SCOPE_LOCAL, - H5F_FLUSH_INVALIDATE | H5F_FLUSH_CLOSING | H5F_FLUSH_CLEAR_ONLY) < 0) - HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache"); + } else { + /* + * If this isn't the captain process, flush but only clear + * the flags. + */ + if (H5F_flush(f, H5AC_dxpl_id, H5F_SCOPE_LOCAL, + H5F_FLUSH_INVALIDATE | H5F_FLUSH_CLOSING | H5F_FLUSH_CLEAR_ONLY) < 0) + HGOTO_ERROR(H5E_CACHE, H5E_CANTFLUSH, FAIL, "unable to flush cache"); - } + } - /* Let's all meet up now... */ - if (H5FD_is_fphdf5_driver(f->shared->lf)) - MPI_Barrier(H5FP_SAP_BARRIER_COMM); + /* Let's all meet up now... */ + if (H5FD_is_fphdf5_driver(f->shared->lf)) + MPI_Barrier(H5FP_SAP_BARRIER_COMM); #endif /* H5_HAVE_FPHDF5 */ + } /* end if */ } /* end if */ /* diff --git a/test/tfile.c b/test/tfile.c index 45d6ca7..eb3c6b5 100644 --- a/test/tfile.c +++ b/test/tfile.c @@ -913,6 +913,65 @@ test_obj_count_and_id(hid_t fid1, hid_t fid2, hid_t did, hid_t gid1, /**************************************************************** ** +** test_file_perm(): low-level file test routine. +** This test verifies that a file can be opened for both +** read-only and read-write access and things will be handled +** appropriately. +** +*****************************************************************/ +static void +test_file_perm(void) +{ + hid_t file; /* File opened with read-write permission */ + hid_t filero; /* Same file opened with read-only permission */ + hid_t dspace; /* Dataspace ID */ + hid_t dset; /* Dataset ID */ + herr_t ret; + + /* Output message about test being performed */ + MESSAGE(5, ("Testing Low-Level File Permissions\n")); + + dspace = H5Screate(H5S_SCALAR); + CHECK(dspace, FAIL, "H5Screate"); + + /* Create the file (with read-write permission) */ + file = H5Fcreate(FILE2, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT); + CHECK(file, FAIL, "H5Fcreate"); + + /* Create a dataset with the read-write file handle */ + dset = H5Dcreate(file, F2_DSET, H5T_NATIVE_INT, dspace, H5P_DEFAULT); + CHECK(dset, FAIL, "H5Dcreate"); + + ret = H5Dclose(dset); + CHECK(ret, FAIL, "H5Dclose"); + + /* Open the file (with read-only permission) */ + filero = H5Fopen(FILE2, H5F_ACC_RDONLY, H5P_DEFAULT); + CHECK(filero, FAIL, "H5Fopen"); + + /* Create a dataset with the read-only file handle (should fail) */ + H5E_BEGIN_TRY { + dset = H5Dcreate(filero, F2_DSET, H5T_NATIVE_INT, dspace, H5P_DEFAULT); + } H5E_END_TRY; + VERIFY(dset, FAIL, "H5Dcreate"); + if(dset!=FAIL) { + ret = H5Dclose(dset); + CHECK(ret, FAIL, "H5Dclose"); + } /* end if */ + + ret = H5Fclose(filero); + CHECK(ret, FAIL, "H5Fclose"); + + ret = H5Fclose(file); + CHECK(ret, FAIL, "H5Fclose"); + + ret = H5Sclose(dspace); + CHECK(ret, FAIL, "H5Sclose"); + +} /* end test_file_perm() */ + +/**************************************************************** +** ** test_file(): Main low-level file I/O test routine. ** ****************************************************************/ @@ -927,6 +986,7 @@ test_file(void) #ifndef H5_NO_SHARED_WRITING test_file_close(); /* Test file close behavior */ #endif /* H5_NO_SHARED_WRITING */ + test_file_perm(); /* Test file access permissions */ } /* test_file() */ -- cgit v0.12