From ac035bf3679dc539c6b35534f310539210322779 Mon Sep 17 00:00:00 2001 From: Dana Robinson Date: Wed, 2 May 2018 10:11:49 -0700 Subject: * Fixed the error tests * Moved common functionality into helper functions --- src/H5Dio.c | 171 +++++++++++++++++++++----------------------- src/H5S.c | 53 +++++++++++++- src/H5Sprivate.h | 1 + test/testfiles/err_compat_1 | 2 +- test/testfiles/error_test_1 | 4 +- 5 files changed, 139 insertions(+), 92 deletions(-) diff --git a/src/H5Dio.c b/src/H5Dio.c index 95efa08..781bd17 100644 --- a/src/H5Dio.c +++ b/src/H5Dio.c @@ -50,7 +50,8 @@ /* Local Prototypes */ /********************/ -/* Setup/teardown routines */ +static herr_t H5D__get_offset_copy(const H5D_t *dset, const hsize_t *offset, + hsize_t **offset_copy/*out*/); static herr_t H5D__ioinfo_init(H5D_t *dset, const H5D_type_info_t *type_info, H5D_storage_t *store, H5D_io_info_t *io_info); static herr_t H5D__typeinfo_init(const H5D_t *dset, hid_t mem_type_id, @@ -80,6 +81,56 @@ H5FL_DEFINE(H5D_chunk_map_t); /*------------------------------------------------------------------------- + * Function: H5D__get_offset_copy + * + * Purpose: Gets a copy of the user's offset array that is guaraneteed + * to be suitable for use by the library. + * + * The caller must free the constructed offset array. + * + * Return: SUCCEED/FAIL + * + *------------------------------------------------------------------------- + */ +static herr_t +H5D__get_offset_copy(const H5D_t *dset, const hsize_t *offset, hsize_t **offset_copy) +{ + unsigned u; + herr_t ret_value = SUCCEED; /* Return value */ + + FUNC_ENTER_NOAPI(FAIL) + + HDassert(dset); + HDassert(offset); + HDassert(offset_copy); + + if (NULL == (*offset_copy = (hsize_t *)H5MM_calloc(H5O_LAYOUT_NDIMS * sizeof(hsize_t)))) + HGOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "unable to allocate offset_copy array") + + /* The library's chunking code requires the offset to terminate with a zero. + * So transfer the offset array to an internal offset array that we + * can properly terminate (handled via the calloc call). + */ + for (u = 0; u < dset->shared->ndims; u++) { + /* Make sure the offset doesn't exceed the dataset's dimensions */ + if (offset[u] > dset->shared->curr_dims[u]) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADTYPE, FAIL, "offset exceeds dimensions of dataset") + + /* Make sure the offset fall right on a chunk's boundary */ + if (offset[u] % dset->shared->layout.u.chunk.dim[u]) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADTYPE, FAIL, "offset doesn't fall on chunks's boundary") + + (*offset_copy)[u] = offset[u]; + } + +done: + FUNC_LEAVE_NOAPI(ret_value) + +} /* end H5D__get_offset_copy() */ + + + +/*------------------------------------------------------------------------- * Function: H5Dread * * Purpose: Reads (part of) a DSET from the file into application @@ -122,33 +173,17 @@ H5Dread(hid_t dset_id, hid_t mem_type_id, hid_t mem_space_id, H5TRACE6("e", "iiiiix", dset_id, mem_type_id, mem_space_id, file_space_id, dxpl_id, buf); - /* check arguments */ + /* Get dataset pointer */ if (NULL == (dset = (H5D_t *)H5I_object_verify(dset_id, H5I_DATASET))) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "dset_id is not a dataset ID") if (NULL == dset->oloc.file) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "dataset is not associated with a file") - if (mem_space_id < 0) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid mem_space_id") - if (file_space_id < 0) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid file_space_id") - - /* Check dataspaces */ - if (H5S_ALL != mem_space_id) { - if (NULL == (mem_space = (const H5S_t *)H5I_object_verify(mem_space_id, H5I_DATASPACE))) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "mem_space_id is not a dataspace ID") - - /* Check for valid selection */ - if (H5S_SELECT_VALID(mem_space) != TRUE) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADRANGE, FAIL, "selection + offset not within extent") - } - if (H5S_ALL != file_space_id) { - if (NULL == (file_space = (const H5S_t *)H5I_object_verify(file_space_id, H5I_DATASPACE))) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "file_space_id is not a dataspace ID") - /* Check for valid selection */ - if (H5S_SELECT_VALID(file_space) != TRUE) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADRANGE, FAIL, "selection + offset not within extent") - } + /* Get validated dataspace pointers */ + if (H5S_get_validated_dataspace(mem_space_id, &mem_space) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "could not get a validated dataspace from mem_space_id") + if (H5S_get_validated_dataspace(file_space_id, &file_space) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "could not get a validated dataspace from file_space_id") /* Get the default dataset transfer property list if the user didn't provide one */ if (H5P_DEFAULT == dxpl_id) @@ -186,9 +221,8 @@ H5Dread_chunk(hid_t dset_id, hid_t dxpl_id, const hsize_t *offset, uint32_t *fil void *buf) { H5D_t *dset = NULL; - hsize_t internal_offset[H5O_LAYOUT_NDIMS]; /* Internal copy of chunk offset */ - unsigned u; /* Local index variable */ - herr_t ret_value = SUCCEED; /* Return value */ + hsize_t *offset_copy = NULL; /* Internal copy of chunk offset */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_API(FAIL) H5TRACE5("e", "ii*h*Iu*x", dset_id, dxpl_id, offset, filters, buf); @@ -217,30 +251,19 @@ H5Dread_chunk(hid_t dset_id, hid_t dxpl_id, const hsize_t *offset, uint32_t *fil /* Set DXPL for operation */ H5CX_set_dxpl(dxpl_id); - /* The library's chunking code requires the offset to terminate with a zero. - * So transfer the offset array to an internal offset array that we - * can properly terminate (don't mess with the passed-in buffer). + /* Copy the user's offset array so we can be sure it's terminated properly. + * (we don't want to mess with the user's buffer). */ - for (u = 0; u < dset->shared->ndims; u++) { - /* Make sure the offset doesn't exceed the dataset's dimensions */ - if (offset[u] > dset->shared->curr_dims[u]) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADTYPE, FAIL, "offset exceeds dimensions of dataset") - - /* Make sure the offset fall right on a chunk's boundary */ - if (offset[u] % dset->shared->layout.u.chunk.dim[u]) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADTYPE, FAIL, "offset doesn't fall on chunks's boundary") - - internal_offset[u] = offset[u]; - } - - /* Terminate the offset with a zero */ - internal_offset[dset->shared->ndims] = 0; + if (H5D__get_offset_copy(dset, offset, &offset_copy) < 0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "failure to copy offset array") /* Read the raw chunk */ - if (H5D__chunk_direct_read(dset, internal_offset, filters, buf) < 0) + if (H5D__chunk_direct_read(dset, offset_copy, filters, buf) < 0) HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "can't read unprocessed chunk data") done: + offset_copy = (hsize_t *)H5MM_xfree(offset_copy); + FUNC_LEAVE_API(ret_value) } /* end H5Dread_chunk() */ @@ -289,33 +312,17 @@ H5Dwrite(hid_t dset_id, hid_t mem_type_id, hid_t mem_space_id, H5TRACE6("e", "iiiii*x", dset_id, mem_type_id, mem_space_id, file_space_id, dxpl_id, buf); - /* check arguments */ + /* Get dataset pointer and ensure it's associated with a file */ if (NULL == (dset = (H5D_t *)H5I_object_verify(dset_id, H5I_DATASET))) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "dset_id is not a dataset ID") if (NULL == dset->oloc.file) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "dataset is not associated with a file") - if (mem_space_id < 0) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid mem_space_id") - if (file_space_id < 0) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid file_space_id") - - /* Check dataspace selections */ - if (H5S_ALL != mem_space_id) { - if (NULL == (mem_space = (const H5S_t *)H5I_object_verify(mem_space_id, H5I_DATASPACE))) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "mem_space_id is not a dataspace ID") - - /* Check for valid selection */ - if (H5S_SELECT_VALID(mem_space) != TRUE) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADRANGE, FAIL, "memory selection + offset not within extent") - } - if (H5S_ALL != file_space_id) { - if (NULL == (file_space = (const H5S_t *)H5I_object_verify(file_space_id, H5I_DATASPACE))) - HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "file_space_id is not a dataspace ID") - /* Check for valid selection */ - if (H5S_SELECT_VALID(file_space) != TRUE) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADRANGE, FAIL, "file selection + offset not within extent") - } + /* Get validated dataspace pointers */ + if (H5S_get_validated_dataspace(mem_space_id, &mem_space) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "could not get a validated dataspace from mem_space_id") + if (H5S_get_validated_dataspace(file_space_id, &file_space) < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "could not get a validated dataspace from file_space_id") /* Get the default dataset transfer property list if the user didn't provide one */ if (H5P_DEFAULT == dxpl_id) @@ -353,10 +360,9 @@ H5Dwrite_chunk(hid_t dset_id, hid_t dxpl_id, uint32_t filters, const hsize_t *of size_t data_size, const void *buf) { H5D_t *dset = NULL; - hsize_t internal_offset[H5O_LAYOUT_NDIMS]; /* Internal copy of chunk offset */ - unsigned u; /* Local index variable */ - uint32_t data_size_32; /* Chunk data size (limited to 32-bits currently) */ - herr_t ret_value = SUCCEED; /* Return value */ + hsize_t *offset_copy = NULL; /* Internal copy of chunk offset */ + uint32_t data_size_32; /* Chunk data size (limited to 32-bits currently) */ + herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_API(FAIL) H5TRACE6("e", "iiIu*hz*x", dset_id, dxpl_id, filters, offset, data_size, buf); @@ -390,30 +396,19 @@ H5Dwrite_chunk(hid_t dset_id, hid_t dxpl_id, uint32_t filters, const hsize_t *of /* Set DXPL for operation */ H5CX_set_dxpl(dxpl_id); - /* The library's chunking code requires the offset to terminate with a zero. - * So transfer the offset array to an internal offset array that we - * can properly terminate (don't mess with the passed-in buffer). + /* Copy the user's offset array so we can be sure it's terminated properly. + * (we don't want to mess with the user's buffer). */ - for (u = 0; u < dset->shared->ndims; u++) { - /* Make sure the offset doesn't exceed the dataset's dimensions */ - if (offset[u] > dset->shared->curr_dims[u]) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADTYPE, FAIL, "offset exceeds dimensions of dataset") - - /* Make sure the offset fall right on a chunk's boundary */ - if (offset[u] % dset->shared->layout.u.chunk.dim[u]) - HGOTO_ERROR(H5E_DATASPACE, H5E_BADTYPE, FAIL, "offset doesn't fall on chunks's boundary") - - internal_offset[u] = offset[u]; - } - - /* Terminate the offset with a zero */ - internal_offset[dset->shared->ndims] = 0; + if (H5D__get_offset_copy(dset, offset, &offset_copy) < 0) + HGOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "failure to copy offset array") /* Write chunk */ - if (H5D__chunk_direct_write(dset, filters, internal_offset, data_size_32, buf) < 0) + if (H5D__chunk_direct_write(dset, filters, offset_copy, data_size_32, buf) < 0) HGOTO_ERROR(H5E_DATASET, H5E_WRITEERROR, FAIL, "can't write unprocessed chunk data") done: + offset_copy = (hsize_t *)H5MM_xfree(offset_copy); + FUNC_LEAVE_API(ret_value) } /* end H5Dwrite_chunk() */ diff --git a/src/H5S.c b/src/H5S.c index 3d09fa0..88c2f72 100644 --- a/src/H5S.c +++ b/src/H5S.c @@ -26,7 +26,6 @@ #include "H5Fprivate.h" /* Files */ #include "H5FLprivate.h" /* Free lists */ #include "H5Iprivate.h" /* IDs */ -#include "H5MMprivate.h" /* Memory management */ #include "H5Oprivate.h" /* Object headers */ #include "H5Spkg.h" /* Dataspaces */ @@ -208,6 +207,58 @@ H5S_term_package(void) FUNC_LEAVE_NOAPI(n) } /* end H5S_term_package() */ + + +/*-------------------------------------------------------------------------- + NAME + H5S_get_validiated_dataspace + PURPOSE + Get a pointer to a validated H5S_t pointer + USAGE + H5S_t *H5S_get_validated_space(dataspace_id, space) + hid_t space_id; IN: The ID of the dataspace + const H5S_t * space; OUT: A pointer to the dataspace + RETURNS + SUCCEED/FAIL + DESCRIPTION + Gets a pointer to a dataspace struct after validating it. The pointer + can be NULL (if the ID is H5S_ALL, for example). + GLOBAL VARIABLES + COMMENTS, BUGS, ASSUMPTIONS + EXAMPLES + REVISION LOG +--------------------------------------------------------------------------*/ +herr_t +H5S_get_validated_dataspace(hid_t space_id, const H5S_t **space) +{ + herr_t ret_value = SUCCEED; /* Return value */ + + FUNC_ENTER_NOAPI(FAIL) + + HDassert(space); + + if (space_id < 0) + HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid space_id (ID cannot be a negative number)") + + if (H5S_ALL == space_id) { + /* No special dataspace struct for H5S_ALL */ + *space = NULL; + } + else { + /* Get the dataspace pointer */ + if (NULL == (*space = (const H5S_t *)H5I_object_verify(space_id, H5I_DATASPACE))) + HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "space_id is not a dataspace ID") + + /* Check for valid selection */ + if (H5S_SELECT_VALID(*space) != TRUE) + HGOTO_ERROR(H5E_DATASPACE, H5E_BADRANGE, FAIL, "selection + offset not within extent") + } + +done: + FUNC_LEAVE_NOAPI(ret_value) + +} /* end H5S_get_validated_dataspace() */ + /*-------------------------------------------------------------------------- NAME diff --git a/src/H5Sprivate.h b/src/H5Sprivate.h index b4ae1ff..15ce75d 100644 --- a/src/H5Sprivate.h +++ b/src/H5Sprivate.h @@ -215,6 +215,7 @@ H5_DLL herr_t H5S_set_extent_real(H5S_t *space, const hsize_t *size); H5_DLL herr_t H5S_set_extent_simple(H5S_t *space, unsigned rank, const hsize_t *dims, const hsize_t *max); H5_DLL H5S_t *H5S_create(H5S_class_t type); +H5_DLL herr_t H5S_get_validated_dataspace(hid_t space_id, const H5S_t **space/*out*/); H5_DLL H5S_t *H5S_create_simple(unsigned rank, const hsize_t dims[/*rank*/], const hsize_t maxdims[/*rank*/]); H5_DLL herr_t H5S_set_version(H5F_t *f, H5S_t *ds); diff --git a/test/testfiles/err_compat_1 b/test/testfiles/err_compat_1 index d471e13..8190665 100644 --- a/test/testfiles/err_compat_1 +++ b/test/testfiles/err_compat_1 @@ -54,6 +54,6 @@ HDF5-DIAG: Error detected in HDF5 (version (number)) thread (IDs): #001: (file name) line (number) in test_error2(): H5Dwrite shouldn't succeed major: Error API minor: Write failed - #002: (file name) line (number) in H5Dwrite(): not a dataset + #002: (file name) line (number) in H5Dwrite(): dset_id is not a dataset ID major: Invalid arguments to routine minor: Inappropriate type diff --git a/test/testfiles/error_test_1 b/test/testfiles/error_test_1 index b0b5085..02cfed8 100644 --- a/test/testfiles/error_test_1 +++ b/test/testfiles/error_test_1 @@ -21,7 +21,7 @@ Error Test-DIAG: Error detected in Error Program (1.0) thread (IDs): Testing error API based on data I/O HDF5-DIAG: Error detected in HDF5 (version (number)) thread (IDs): - #000: (file name) line (number) in H5Dwrite(): not a dataset + #000: (file name) line (number) in H5Dwrite(): dset_id is not a dataset ID major: Invalid arguments to routine minor: Inappropriate type Error Test-DIAG: Error detected in Error Program (1.0) thread (IDs): @@ -32,7 +32,7 @@ Error Test-DIAG: Error detected in Error Program (1.0) thread (IDs): major: Error in IO minor: Error in H5Dwrite HDF5-DIAG: Error detected in HDF5 (version (number)) thread (IDs): - #002: (file name) line (number) in H5Dwrite(): not a dataset + #002: (file name) line (number) in H5Dwrite(): dset_id is not a dataset ID major: Invalid arguments to routine minor: Inappropriate type -- cgit v0.12