mirror/hdf5
2
0
Fork 0
mirror of https://github.com/HDFGroup/hdf5.git synced 2025-03-31 17:10:47 +08:00
Code Issues Packages Projects Releases Wiki Activity

Try to address most of the issues raised by Dana in the code review

Browse Source
This commit is contained in:
Richard Warren 2017-10-11 13:40:54 -04:00
parent 9849d61344
commit 157398107e
2 changed files with 101 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
src/H5Fsuper.c
View File

@ -21,15 +21,15 @@
/***********/
/* Headers */
/***********/
#include "H5private.h" /* Generic Functions */
#include "H5private.h" /* Generic Functions */
#include "H5ACprivate.h" /* Metadata cache */
#include "H5Eprivate.h" /* Error handling */
#include "H5Eprivate.h" /* Error handling */
#include "H5Fpkg.h" /* File access */
#include "H5FDprivate.h" /* File drivers */
#include "H5Iprivate.h" /* IDs */
#include "H5FDprivate.h" /* File drivers */
#include "H5Iprivate.h" /* IDs */
#include "H5MFprivate.h" /* File memory management */
#include "H5MMprivate.h" /* Memory management */
#include "H5Pprivate.h" /* Property lists */
#include "H5MMprivate.h" /* Memory management */
#include "H5Pprivate.h" /* Property lists */
#include "H5SMprivate.h" /* Shared Object Header Messages */
@ -158,7 +158,7 @@ H5F_super_ext_open(H5F_t *f, haddr_t ext_addr, H5O_loc_t *ext_ptr)
/* Open the superblock extension object header */
if(H5O_open(ext_ptr) < 0)
HGOTO_ERROR(H5E_OHDR, H5E_CANTOPENOBJ, FAIL, "unable to open superblock extension")
HGOTO_ERROR(H5E_OHDR, H5E_CANTOPENOBJ, FAIL, "unable to open superblock extension")
done:
FUNC_LEAVE_NOAPI(ret_value)
@ -224,12 +224,12 @@ done:
/*-------------------------------------------------------------------------
* Function: H5F__update_super_ext_driver_msg
*
* Purpose: Update the superblock extension file driver info message if
* we are using a V 2 superblock. Observe that the function
* is a NO-OP if the file driver info message does not exist.
* Purpose: Update the superblock extension file driver info message if
* we are using a V 2 superblock. Observe that the function
* is a NO-OP if the file driver info message does not exist.
* This is necessary, as the function is called whenever the
* EOA is updated, and were it to create the file driver info
* message, it would find itself in an infinite recursion.
* EOA is updated, and were it to create the file driver info
* message, it would find itself in an infinite recursion.
*
* Return: Success: SUCCEED
* Failure: FAIL
@ -267,7 +267,7 @@ H5F__update_super_ext_driver_msg(H5F_t *f, hid_t dxpl_id)
/* Check for driver info */
H5_CHECKED_ASSIGN(driver_size, size_t, H5FD_sb_size(f->shared->lf), hsize_t);
/* Nothing to do unless there is both driver info and
/* Nothing to do unless there is both driver info and
* the driver info superblock extension message has
* already been created.
*/
@ -330,13 +330,13 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
unsigned sblock_flags = H5AC__NO_FLAGS_SET; /* flags used in superblock unprotect call */
haddr_t super_addr; /* Absolute address of superblock */
haddr_t eof; /* End of file address */
unsigned rw_flags; /* Read/write permissions for file */
hbool_t skip_eof_check = FALSE; /* Whether to skip checking the EOF value */
unsigned rw_flags; /* Read/write permissions for file */
hbool_t skip_eof_check = FALSE; /* Whether to skip checking the EOF value */
herr_t ret_value = SUCCEED; /* Return value */
#ifdef H5_HAVE_PARALLEL
int mpi_rank = 0, mpi_size = 1;
int mpi_result;
#endif /* H5_HAVE_PARALLEL */
#endif /* H5_HAVE_PARALLEL */
FUNC_ENTER_PACKAGE_TAG(meta_dxpl_id, H5AC__SUPERBLOCK_TAG, FAIL)
@ -365,7 +365,7 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "Can't get MPI rank")
if((mpi_size = H5F_mpi_get_size(f)) < 0)
HGOTO_ERROR(H5E_PAGEBUF, H5E_CANTGET, FAIL, "can't retrieve MPI communicator size")
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "can't retrieve MPI communicator size")
}
/* If we are an MPI application with at least two processes, the
@ -380,14 +380,14 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
if ( mpi_size > 1 ) {
MPI_Comm this_comm = MPI_COMM_NULL;
if ( mpi_rank == 0 ) {
if(H5FD_locate_signature(&fdio_info, &super_addr) < 0)
HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "unable to locate file signature")
if ( mpi_rank == 0 ) {
if (H5FD_locate_signature(&fdio_info, &super_addr) < 0)
HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "unable to locate file signature")
}
HDassert(H5F_HAS_FEATURE(f, H5FD_FEAT_HAS_MPI));
if ( MPI_COMM_NULL == (this_comm = H5F_mpi_get_comm(f)) )
HGOTO_ERROR(H5E_VFL, H5E_CANTGET, FAIL, "can't get MPI communicator")
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "can't get MPI communicator")
if ( MPI_SUCCESS !=
(mpi_result = MPI_Bcast(&super_addr,sizeof(super_addr), MPI_BYTE, 0, this_comm)))
@ -395,14 +395,14 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
}
else {
/* Locate the signature as per per the serial library */
#endif /* H5_HAVE_PARALLEL */
#endif /* H5_HAVE_PARALLEL */
if(H5FD_locate_signature(&fdio_info, &super_addr) < 0)
HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "unable to locate file signature")
if (H5FD_locate_signature(&fdio_info, &super_addr) < 0)
HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "unable to locate file signature")
#ifdef H5_HAVE_PARALLEL
}
#endif /* H5_HAVE_PARALLEL */
#endif /* H5_HAVE_PARALLEL */
if(HADDR_UNDEF == super_addr)
HGOTO_ERROR(H5E_FILE, H5E_NOTHDF5, FAIL, "file signature not found")
@ -453,12 +453,12 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
HGOTO_ERROR(H5E_FILE, H5E_CANTPROTECT, FAIL, "unable to load superblock")
if(H5F_INTENT(f) & H5F_ACC_SWMR_WRITE)
if(sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3)
HGOTO_ERROR(H5E_FILE, H5E_CANTPROTECT, FAIL, "invalid superblock version for SWMR_WRITE")
if(sblock->super_vers < HDF5_SUPERBLOCK_VERSION_3)
HGOTO_ERROR(H5E_FILE, H5E_CANTPROTECT, FAIL, "invalid superblock version for SWMR_WRITE")
/* Enable all latest version support when file has v3 superblock */
if(sblock->super_vers >= HDF5_SUPERBLOCK_VERSION_3)
f->shared->latest_flags |= H5F_LATEST_ALL_FLAGS;
f->shared->latest_flags |= H5F_LATEST_ALL_FLAGS;
/* Pin the superblock in the cache */
if(H5AC_pin_protected_entry(sblock) < 0)
@ -558,15 +558,15 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
* been flushed to disk by the SWMR writer process.
*/
if(H5F_INTENT(f) & H5F_ACC_SWMR_READ) {
/*
* When the file is opened for SWMR read access, skip the check if:
* --the file is already marked for SWMR writing and
* --the file has version 3 superblock for SWMR support
*/
if((sblock->status_flags & H5F_SUPER_SWMR_WRITE_ACCESS) &&
/*
* When the file is opened for SWMR read access, skip the check if:
* --the file is already marked for SWMR writing and
* --the file has version 3 superblock for SWMR support
*/
if((sblock->status_flags & H5F_SUPER_SWMR_WRITE_ACCESS) &&
(sblock->status_flags & H5F_SUPER_WRITE_ACCESS) &&
sblock->super_vers >= HDF5_SUPERBLOCK_VERSION_3)
skip_eof_check = TRUE;
skip_eof_check = TRUE;
} /* end if */
if(!skip_eof_check && initial_read) {
if(HADDR_UNDEF == (eof = H5FD_get_eof(f->shared->lf, H5FD_MEM_DEFAULT)))
@ -640,7 +640,7 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
H5O_loc_t ext_loc; /* "Object location" for superblock extension */
H5O_btreek_t btreek; /* v1 B-tree 'K' value message from superblock extension */
H5O_drvinfo_t drvinfo; /* Driver info message from superblock extension */
size_t u; /* Local index variable */
size_t u; /* Local index variable */
htri_t status; /* Status for message existing */
/* Sanity check - superblock extension should only be defined for
@ -661,7 +661,7 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
} /* end if */
/* Open the superblock extension */
if(H5F_super_ext_open(f, sblock->ext_addr, &ext_loc) < 0)
if(H5F_super_ext_open(f, sblock->ext_addr, &ext_loc) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTOPENOBJ, FAIL, "unable to open file's superblock extension")
/* Check for the extension having a 'driver info' message */
@ -684,8 +684,8 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
/* Reset driver info message */
H5O_msg_reset(H5O_DRVINFO_ID, &drvinfo);
HDassert(FALSE == f->shared->drvinfo_sb_msg_exists);
f->shared->drvinfo_sb_msg_exists = TRUE;
HDassert(FALSE == f->shared->drvinfo_sb_msg_exists);
f->shared->drvinfo_sb_msg_exists = TRUE;
} /* end else */
} /* end if */
@ -811,37 +811,37 @@ H5F__super_read(H5F_t *f, hid_t meta_dxpl_id, hid_t raw_dxpl_id, hbool_t initial
} /* end if not marked "unknown" */
} /* end if */
/* Check for the extension having a 'metadata cache image' message */
/* Check for the extension having a 'metadata cache image' message */
if((status = H5O_msg_exists(&ext_loc, H5O_MDCI_MSG_ID, meta_dxpl_id)) < 0)
HGOTO_ERROR(H5E_FILE, H5E_EXISTS, FAIL, "unable to read object header")
if(status) {
hbool_t rw = ((rw_flags & H5AC__READ_ONLY_FLAG) == 0);
H5O_mdci_t mdci_msg;
hbool_t rw = ((rw_flags & H5AC__READ_ONLY_FLAG) == 0);
H5O_mdci_t mdci_msg;
/* if the metadata cache image superblock extension message exists,
/* if the metadata cache image superblock extension message exists,
* read its contents and pass the data on to the metadata cache.
* Given this data, the cache will load and decode the metadata
* cache image block, decoded it and load its contents into the
* the cache on the test protect call.
* cache image block, decoded it and load its contents into the
* the cache on the test protect call.
*
* Further, if the file is opened R/W, the metadata cache will
* delete the metadata cache image superblock extension and free
* the cache image block. Don't do this now as f->shared
* is not fully setup, which complicates matters.
* delete the metadata cache image superblock extension and free
* the cache image block. Don't do this now as f->shared
* is not fully setup, which complicates matters.
*/
/* Retrieve the 'metadata cache image message' structure */
if(NULL == H5O_msg_read(&ext_loc, H5O_MDCI_MSG_ID, &mdci_msg, meta_dxpl_id))
if(NULL == H5O_msg_read(&ext_loc, H5O_MDCI_MSG_ID, &mdci_msg, meta_dxpl_id))
HGOTO_ERROR(H5E_FILE, H5E_CANTGET, FAIL, "unable to get metadata cache image message")
/* Indicate to the cache that there's an image to load on first protect call */
if(H5AC_load_cache_image_on_next_protect(f, mdci_msg.addr, mdci_msg.size, rw) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTLOAD, FAIL, "call to H5AC_load_cache_image_on_next_protect failed");
HGOTO_ERROR(H5E_FILE, H5E_CANTLOAD, FAIL, "call to H5AC_load_cache_image_on_next_protect failed");
} /* end if */
/* Close superblock extension */
if(H5F_super_ext_close(f, &ext_loc, meta_dxpl_id, FALSE) < 0)
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEOBJ, FAIL, "unable to close file's superblock extension")
HGOTO_ERROR(H5E_FILE, H5E_CANTCLOSEOBJ, FAIL, "unable to close file's superblock extension")
} /* end if */
/* Update the driver info if VFD indicated to do so */

104
testpar/t_pread.c
View File

@ -87,17 +87,13 @@ static int test_parallel_read(MPI_Comm comm, int mpi_rank, int group);
*
* Failure: 1
*
* Programmer: Richard Warren
* 10/1/17
*
* Modifications:
*
*-------------------------------------------------------------------------
*/
static int
generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
{
FILE *header;
FILE *header = NULL;
const char *fcn_name = "generate_test_file()";
const char *failure_mssg = NULL;
const char *group_filename = NULL;
@ -113,12 +109,12 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
hsize_t i;
hsize_t offset;
hsize_t dims[1] = {0};
hid_t file_id;
hid_t memspace;
hid_t filespace;
hid_t fapl_id;
hid_t dxpl_id;
hid_t dset_id;
hid_t file_id = -1;
hid_t memspace = -1;
hid_t filespace = -1;
hid_t fapl_id = -1;
hid_t dxpl_id = -1;
hid_t dset_id = -1;
float nextValue;
float *data_slice = NULL;
@ -155,17 +151,17 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
else if ( group_id == 0 ) { /* Test 2 group 0 */
file_index = 3;
}
else { /* Test 2 group 1 */
else { /* Test 2 group 1 */
file_index = 6;
}
/* The 'group_filename' is just a temp variable and
/* The 'group_filename' is just a temp variable and
* is used to call into the h5_fixname function. No
* need to worry that we reassign it for each file!
*/
*/
HDassert((group_filename = FILENAMES[file_index]));
/* Assign the 'data_filename' */
/* Assign the 'data_filename' */
if ( h5_fixname(group_filename, H5P_DEFAULT, data_filename,
sizeof(data_filename)) == NULL ) {
pass = FALSE;
@ -177,7 +173,7 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
HDassert( (group_filename = FILENAMES[file_index+1]) );
/* Assign the 'reloc_data_filename' */
/* Assign the 'reloc_data_filename' */
if ( h5_fixname(group_filename, H5P_DEFAULT, reloc_data_filename,
sizeof(reloc_data_filename)) == NULL ) {
@ -190,7 +186,7 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
HDassert( (group_filename = FILENAMES[file_index+2]) );
/* Assing the 'prolog_filename' */
/* Assign the 'prolog_filename' */
if ( h5_fixname(group_filename, H5P_DEFAULT, prolog_filename,
sizeof(prolog_filename)) == NULL ) {
pass = FALSE;
@ -211,7 +207,7 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
for(i=0; i<COUNT; i++) {
data_slice[i] = nextValue;
nextValue += 1;
nextValue += 1;
}
}
@ -297,42 +293,42 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
}
/* close file, etc. */
if ( pass ) {
if ( pass || (dset_id != -1)) {
if ( H5Dclose(dset_id) < 0 ) {
pass = false;
failure_mssg = "H5Dclose(dset_id) failed.\n";
}
}
if ( pass ) {
if ( pass || (memspace != -1) ) {
if ( H5Sclose(memspace) < 0 ) {
pass = false;
failure_mssg = "H5Sclose(memspace) failed.\n";
}
}
if ( pass ) {
if ( pass || (filespace != -1) ) {
if ( H5Sclose(filespace) < 0 ) {
pass = false;
failure_mssg = "H5Sclose(filespace) failed.\n";
}
}
if ( pass ) {
if ( pass || (file_id != -1) ) {
if ( H5Fclose(file_id) < 0 ) {
pass = false;
failure_mssg = "H5Fclose(file_id) failed.\n";
}
}
if ( pass ) {
if ( pass || (dxpl_id != -1) ) {
if ( H5Pclose(dxpl_id) < 0 ) {
pass = false;
failure_mssg = "H5Pclose(dxpl_id) failed.\n";
}
}
if ( pass ) {
if ( pass || (fapl_id != -1) ) {
if ( H5Pclose(fapl_id) < 0 ) {
pass = false;
failure_mssg = "H5Pclose(fapl_id) failed.\n";
@ -361,10 +357,10 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
if (group_id == 0)
text_to_write = random_hdf5_text;
else
else
text_to_write = hitchhiker_quote;
bytes_to_write = strlen(text_to_write);
bytes_to_write = strlen(text_to_write);
if ( pass ) {
if ( (header = HDfopen(prolog_filename, "w+")) == NULL ) {
@ -382,7 +378,7 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
}
}
if ( pass ) {
if ( pass || (header != NULL) ) {
if ( HDfclose(header) != 0 ) {
pass = FALSE;
failure_mssg = "HDfclose() failed.\n";
@ -395,16 +391,14 @@ generate_test_file( MPI_Comm comm, int mpi_rank, int group_id )
HDsprintf(cmd, "../tools/src/h5jam/h5jam -i %s -u %s -o %s",
data_filename, prolog_filename, reloc_data_filename);
if ( system(cmd) != 0 ) {
if ( system(cmd) != 0 ) {
pass = FALSE;
failure_mssg = "invocation of h5jam failed.\n";
}
}
if ( pass ) {
HDremove(prolog_filename);
HDremove(data_filename);
}
HDremove(prolog_filename);
HDremove(data_filename);
}
/* collect results from other processes.
@ -492,10 +486,10 @@ test_parallel_read(MPI_Comm comm, int mpi_rank, int group_id)
int global_failures = 0;
int group_size;
int group_rank;
hid_t fapl_id;
hid_t file_id;
hid_t dset_id;
hid_t memspace = -1;
hid_t fapl_id = -1;
hid_t file_id = -1;
hid_t dset_id = -1;
hid_t memspace = -1;
hid_t filespace = -1;
hsize_t i;
hsize_t offset;
@ -542,11 +536,11 @@ test_parallel_read(MPI_Comm comm, int mpi_rank, int group_id)
*/
if ( pass ) {
if ( comm == MPI_COMM_WORLD ) /* test 1 */
if ( comm == MPI_COMM_WORLD ) /* test 1 */
group_filename = FILENAMES[1];
else if ( group_id == 0 ) /* test 2 group 0 */
else if ( group_id == 0 ) /* test 2 group 0 */
group_filename = FILENAMES[4];
else /* test 2 group 1 */
else /* test 2 group 1 */
group_filename = FILENAMES[7];
HDassert(group_filename);
@ -645,35 +639,35 @@ test_parallel_read(MPI_Comm comm, int mpi_rank, int group_id)
}
/* close file, etc. */
if ( pass ) {
if ( pass || (dset_id != -1) ) {
if ( H5Dclose(dset_id) < 0 ) {
pass = false;
failure_mssg = "H5Dclose(dset_id) failed.\n";
}
}
if ( pass ) {
if ( pass || (memspace != -1) ) {
if ( H5Sclose(memspace) < 0 ) {
pass = false;
failure_mssg = "H5Sclose(memspace) failed.\n";
}
}
if ( pass ) {
if ( pass || (filespace != -1) ) {
if ( H5Sclose(filespace) < 0 ) {
pass = false;
failure_mssg = "H5Sclose(filespace) failed.\n";
}
}
if ( pass ) {
if ( pass || (file_id != -1) ) {
if ( H5Fclose(file_id) < 0 ) {
pass = false;
failure_mssg = "H5Fclose(file_id) failed.\n";
}
}
if ( pass ) {
if ( pass || (fapl_id != -1) ) {
if ( H5Pclose(fapl_id) < 0 ) {
pass = false;
failure_mssg = "H5Pclose(fapl_id) failed.\n";
@ -764,17 +758,17 @@ main( int argc, char **argv)
if ( (MPI_Init(&argc, &argv)) != MPI_SUCCESS) {
HDfprintf(stderr, "FATAL: Unable to initialize MPI\n");
exit(1);
HDexit(FAIL);
}
if ( (MPI_Comm_rank(MPI_COMM_WORLD, &mpi_rank)) != MPI_SUCCESS) {
HDfprintf(stderr, "FATAL: MPI_Comm_rank returned an error\n");
exit(2);
HDexit(FAIL);
}
if ( (MPI_Comm_size(MPI_COMM_WORLD, &mpi_size)) != MPI_SUCCESS) {
HDfprintf(stderr, "FATAL: MPI_Comm_size returned an error\n");
exit(2);
HDexit(FAIL);
}
H5open();
@ -835,12 +829,12 @@ main( int argc, char **argv)
which_group = (mpi_rank < split_size ? 0 : 1);
if ( (MPI_Comm_split(MPI_COMM_WORLD,
which_group,
0,
&group_comm)) != MPI_SUCCESS) {
which_group,
0,
&group_comm)) != MPI_SUCCESS) {
HDfprintf(stderr, "FATAL: MPI_Comm_split returned an error\n");
exit(2);
HDexit(FAIL);
}
nerrs += generate_test_file( group_comm, mpi_rank, which_group );
@ -870,7 +864,7 @@ main( int argc, char **argv)
finish:
if ((group_comm != MPI_COMM_NULL) &&
(MPI_Comm_free(&group_comm)) != MPI_SUCCESS) {
(MPI_Comm_free(&group_comm)) != MPI_SUCCESS) {
HDfprintf(stderr, "MPI_Comm_free failed!\n");
}
@ -895,12 +889,14 @@ finish:
}
/* close HDF5 library */
H5close();
if (H5close() != SUCCEED) {
HDfprintf(stdout, "H5close() failed. (Ignoring)\n");
}
/* MPI_Finalize must be called AFTER H5close which may use MPI calls */
MPI_Finalize();
/* cannot just return (nerrs) because exit code is limited to 1byte */
return(nerrs > 0);
return((nerrs > 0) ? FAIL : SUCCEED );
} /* main() */