Skip to content

Commit

Permalink
Fix some const cast and stack/static object size warnings (HDFGroup#1700
Browse files Browse the repository at this point in the history 
)

* Fix various warnings

* Move HDfree_const to H5private.h for wider use

* Print output from all ranks in parallel tests on allocation failure

* Move const pointer freeing macro to h5test.h for now
  • Loading branch information
@jhendersonHDF
jhendersonHDF committed May 2, 2022
1 parent 445a9f6 commit a615a2b
Show file tree
Hide file tree
Showing 18 changed files with 268 additions and 95 deletions.
6 changes: 3 additions & 3 deletions src/H5Dmpio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ static herr_t H5D__link_chunk_collective_io(H5D_io_info_t *io_info, const H5D_ty
static herr_t H5D__link_chunk_filtered_collective_io(H5D_io_info_t *io_info, const H5D_type_info_t *type_info,
H5D_chunk_map_t *fm, int mpi_rank, int mpi_size);
static herr_t H5D__inter_collective_io(H5D_io_info_t *io_info, const H5D_type_info_t *type_info,
const H5S_t *file_space, const H5S_t *mem_space);
H5S_t *file_space, H5S_t *mem_space);
static herr_t H5D__final_collective_io(H5D_io_info_t *io_info, const H5D_type_info_t *type_info,
hsize_t nelmts, MPI_Datatype mpi_file_type, MPI_Datatype mpi_buf_type);
static herr_t H5D__sort_chunk(H5D_io_info_t *io_info, const H5D_chunk_map_t *fm,
Expand Down Expand Up @@ -2372,8 +2372,8 @@ H5D__multi_chunk_filtered_collective_io(H5D_io_info_t *io_info, const H5D_type_i
*-------------------------------------------------------------------------
*/
static herr_t
H5D__inter_collective_io(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, const H5S_t *file_space,
const H5S_t *mem_space)
H5D__inter_collective_io(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, H5S_t *file_space,
H5S_t *mem_space)
{
int mpi_buf_count; /* # of MPI types */
hbool_t mbt_is_derived = FALSE;
Expand Down
9 changes: 4 additions & 5 deletions src/H5Shyper.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ static herr_t H5S__hyper_iter_get_seq_list_opt(H5S_sel_iter_t *iter, size_t max
static herr_t H5S__hyper_iter_get_seq_list_single(H5S_sel_iter_t *iter, size_t maxseq, size_t maxelem,
size_t *nseq, size_t *nelem, hsize_t *off, size_t *len);
static herr_t H5S__hyper_proj_int_build_proj(H5S_hyper_project_intersect_ud_t *udata);
static herr_t H5S__hyper_proj_int_iterate(const H5S_hyper_span_info_t *ss_span_info,
static herr_t H5S__hyper_proj_int_iterate(H5S_hyper_span_info_t * ss_span_info,
const H5S_hyper_span_info_t *sis_span_info, hsize_t count,
unsigned depth, H5S_hyper_project_intersect_ud_t *udata);
static void H5S__hyper_get_clip_diminfo(hsize_t start, hsize_t stride, hsize_t *count, hsize_t *block,
Expand Down Expand Up @@ -11331,9 +11331,8 @@ sis_span_info unsigned depth; IN: Depth of iteration (in terms of rank)
REVISION LOG
--------------------------------------------------------------------------*/
static herr_t
H5S__hyper_proj_int_iterate(const H5S_hyper_span_info_t *ss_span_info,
const H5S_hyper_span_info_t *sis_span_info, hsize_t count, unsigned depth,
H5S_hyper_project_intersect_ud_t *udata)
H5S__hyper_proj_int_iterate(H5S_hyper_span_info_t *ss_span_info, const H5S_hyper_span_info_t *sis_span_info,
hsize_t count, unsigned depth, H5S_hyper_project_intersect_ud_t *udata)
{
const H5S_hyper_span_t *ss_span; /* Current span in source space */
const H5S_hyper_span_t *sis_span; /* Current span in source intersect space */
Expand Down Expand Up @@ -11596,7 +11595,7 @@ H5S__hyper_project_intersection(const H5S_t *src_space, const H5S_t *dst_space,
const H5S_t *src_intersect_space, H5S_t *proj_space, hbool_t share_selection)
{
H5S_hyper_project_intersect_ud_t udata; /* User data for subroutines */
const H5S_hyper_span_info_t * ss_span_info;
H5S_hyper_span_info_t * ss_span_info;
const H5S_hyper_span_info_t * ds_span_info;
H5S_hyper_span_info_t * ss_span_info_buf = NULL;
H5S_hyper_span_info_t * ds_span_info_buf = NULL;
Expand Down
93 changes: 59 additions & 34 deletions src/H5Smpio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,10 @@ static herr_t H5S__mpio_create_point_datatype(size_t elmt_size, hsize_t num_poin
static herr_t H5S__mpio_point_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new_type, int *count,
hbool_t *is_derived_type, hbool_t do_permute, hsize_t **permute_map,
hbool_t *is_permuted);
static herr_t H5S__mpio_permute_type(const H5S_t *space, size_t elmt_size, hsize_t **permute_map,
static herr_t H5S__mpio_permute_type(H5S_t *space, size_t elmt_size, hsize_t **permute_map,
MPI_Datatype *new_type, int *count, hbool_t *is_derived_type);
static herr_t H5S__mpio_reg_hyper_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new_type,
int *count, hbool_t *is_derived_type);
static herr_t H5S__mpio_reg_hyper_type(H5S_t *space, size_t elmt_size, MPI_Datatype *new_type, int *count,
hbool_t *is_derived_type);
static herr_t H5S__mpio_span_hyper_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new_type,
int *count, hbool_t *is_derived_type);
static herr_t H5S__release_datatype(H5S_mpio_mpitype_list_t *type_list);
Expand All @@ -95,6 +95,9 @@ static herr_t H5S__obtain_datatype(H5S_hyper_span_info_t *spans, const hsize_t *
/* Declare a free list to manage the H5S_mpio_mpitype_node_t struct */
H5FL_DEFINE_STATIC(H5S_mpio_mpitype_node_t);

/* Declare a free list to manage dataspace selection iterators */
H5FL_EXTERN(H5S_sel_iter_t);

/*-------------------------------------------------------------------------
* Function: H5S__mpio_all_type
*
Expand Down Expand Up @@ -501,17 +504,19 @@ H5S__mpio_point_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new_typ
*-------------------------------------------------------------------------
*/
static herr_t
H5S__mpio_permute_type(const H5S_t *space, size_t elmt_size, hsize_t **permute, MPI_Datatype *new_type,
int *count, hbool_t *is_derived_type)
H5S__mpio_permute_type(H5S_t *space, size_t elmt_size, hsize_t **permute, MPI_Datatype *new_type, int *count,
hbool_t *is_derived_type)
{
MPI_Aint * disp = NULL; /* Datatype displacement for each point*/
H5S_sel_iter_t sel_iter; /* Selection iteration info */
hbool_t sel_iter_init = FALSE; /* Selection iteration info has been initialized */
hssize_t snum_points; /* Signed number of elements in selection */
hsize_t num_points; /* Number of points in the selection */
size_t max_elem; /* Maximum number of elements allowed in sequences */
hsize_t u; /* Local index variable */
herr_t ret_value = SUCCEED; /* Return value */
MPI_Aint * disp = NULL; /* Datatype displacement for each point*/
H5S_sel_iter_t *sel_iter = NULL; /* Selection iteration info */
hbool_t sel_iter_init = FALSE; /* Selection iteration info has been initialized */
hssize_t snum_points; /* Signed number of elements in selection */
hsize_t num_points; /* Number of points in the selection */
hsize_t * off = NULL;
size_t * len = NULL;
size_t max_elem; /* Maximum number of elements allowed in sequences */
hsize_t u; /* Local index variable */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_STATIC

Expand All @@ -527,8 +532,18 @@ H5S__mpio_permute_type(const H5S_t *space, size_t elmt_size, hsize_t **permute,
if (NULL == (disp = (MPI_Aint *)H5MM_malloc(sizeof(MPI_Aint) * num_points)))
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, "can't allocate array of displacements")

/* Allocate arrays to hold sequence offsets and lengths */
if (NULL == (off = H5MM_malloc(H5D_IO_VECTOR_SIZE * sizeof(*off))))
HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, FAIL, "can't allocate sequence offsets array")
if (NULL == (len = H5MM_malloc(H5D_IO_VECTOR_SIZE * sizeof(*len))))
HGOTO_ERROR(H5E_RESOURCE, H5E_CANTALLOC, FAIL, "can't allocate sequence lengths array")

/* Allocate a selection iterator for iterating over the dataspace */
if (NULL == (sel_iter = H5FL_MALLOC(H5S_sel_iter_t)))
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, "couldn't allocate dataspace selection iterator")

/* Initialize selection iterator */
if (H5S_select_iter_init(&sel_iter, space, elmt_size, 0) < 0)
if (H5S_select_iter_init(sel_iter, space, elmt_size, 0) < 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTINIT, FAIL, "unable to initialize selection iterator")
sel_iter_init = TRUE; /* Selection iteration info has been initialized */

Expand All @@ -538,14 +553,12 @@ H5S__mpio_permute_type(const H5S_t *space, size_t elmt_size, hsize_t **permute,
/* Loop, while elements left in selection */
u = 0;
while (max_elem > 0) {
hsize_t off[H5D_IO_VECTOR_SIZE]; /* Array to store sequence offsets */
size_t len[H5D_IO_VECTOR_SIZE]; /* Array to store sequence lengths */
size_t nelem; /* Number of elements used in sequences */
size_t nseq; /* Number of sequences generated */
size_t curr_seq; /* Current sequence being worked on */
size_t nelem; /* Number of elements used in sequences */
size_t nseq; /* Number of sequences generated */
size_t curr_seq; /* Current sequence being worked on */

/* Get the sequences of bytes */
if (H5S_SELECT_ITER_GET_SEQ_LIST(&sel_iter, (size_t)H5D_IO_VECTOR_SIZE, max_elem, &nseq, &nelem, off,
if (H5S_SELECT_ITER_GET_SEQ_LIST(sel_iter, (size_t)H5D_IO_VECTOR_SIZE, max_elem, &nseq, &nelem, off,
len) < 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, FAIL, "sequence length generation failed")

Expand Down Expand Up @@ -602,9 +615,14 @@ H5S__mpio_permute_type(const H5S_t *space, size_t elmt_size, hsize_t **permute,

done:
/* Release selection iterator */
if (sel_iter_init)
if (H5S_SELECT_ITER_RELEASE(&sel_iter) < 0)
if (sel_iter) {
if (sel_iter_init && H5S_SELECT_ITER_RELEASE(sel_iter) < 0)
HDONE_ERROR(H5E_DATASPACE, H5E_CANTRELEASE, FAIL, "unable to release selection iterator")
sel_iter = H5FL_FREE(H5S_sel_iter_t, sel_iter);
}

H5MM_free(len);
H5MM_free(off);

/* Free memory */
if (disp)
Expand Down Expand Up @@ -634,11 +652,11 @@ H5S__mpio_permute_type(const H5S_t *space, size_t elmt_size, hsize_t **permute,
*-------------------------------------------------------------------------
*/
static herr_t
H5S__mpio_reg_hyper_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new_type, int *count,
H5S__mpio_reg_hyper_type(H5S_t *space, size_t elmt_size, MPI_Datatype *new_type, int *count,
hbool_t *is_derived_type)
{
H5S_sel_iter_t sel_iter; /* Selection iteration info */
hbool_t sel_iter_init = FALSE; /* Selection iteration info has been initialized */
H5S_sel_iter_t *sel_iter = NULL; /* Selection iteration info */
hbool_t sel_iter_init = FALSE; /* Selection iteration info has been initialized */

struct dim { /* less hassle than malloc/free & ilk */
hssize_t start;
Expand Down Expand Up @@ -668,32 +686,37 @@ H5S__mpio_reg_hyper_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new
HDassert(sizeof(MPI_Aint) >= sizeof(elmt_size));

bigio_count = H5_mpi_get_bigio_count();

/* Allocate a selection iterator for iterating over the dataspace */
if (NULL == (sel_iter = H5FL_MALLOC(H5S_sel_iter_t)))
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTALLOC, FAIL, "couldn't allocate dataspace selection iterator")

/* Initialize selection iterator */
if (H5S_select_iter_init(&sel_iter, space, elmt_size, 0) < 0)
if (H5S_select_iter_init(sel_iter, space, elmt_size, 0) < 0)
HGOTO_ERROR(H5E_DATASPACE, H5E_CANTINIT, FAIL, "unable to initialize selection iterator")
sel_iter_init = TRUE; /* Selection iteration info has been initialized */

/* Abbreviate args */
diminfo = sel_iter.u.hyp.diminfo;
diminfo = sel_iter->u.hyp.diminfo;
HDassert(diminfo);

/* Make a local copy of the dimension info so we can operate with them */

/* Check if this is a "flattened" regular hyperslab selection */
if (sel_iter.u.hyp.iter_rank != 0 && sel_iter.u.hyp.iter_rank < space->extent.rank) {
if (sel_iter->u.hyp.iter_rank != 0 && sel_iter->u.hyp.iter_rank < space->extent.rank) {
/* Flattened selection */
rank = sel_iter.u.hyp.iter_rank;
rank = sel_iter->u.hyp.iter_rank;
#ifdef H5S_DEBUG
if (H5DEBUG(S))
HDfprintf(H5DEBUG(S), "%s: Flattened selection\n", FUNC);
#endif
for (u = 0; u < rank; ++u) {
H5_CHECK_OVERFLOW(diminfo[u].start, hsize_t, hssize_t)
d[u].start = (hssize_t)diminfo[u].start + sel_iter.u.hyp.sel_off[u];
d[u].start = (hssize_t)diminfo[u].start + sel_iter->u.hyp.sel_off[u];
d[u].strid = diminfo[u].stride;
d[u].block = diminfo[u].block;
d[u].count = diminfo[u].count;
d[u].xtent = sel_iter.u.hyp.size[u];
d[u].xtent = sel_iter->u.hyp.size[u];

#ifdef H5S_DEBUG
if (H5DEBUG(S)) {
Expand Down Expand Up @@ -951,9 +974,11 @@ H5S__mpio_reg_hyper_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new

done:
/* Release selection iterator */
if (sel_iter_init)
if (H5S_SELECT_ITER_RELEASE(&sel_iter) < 0)
if (sel_iter) {
if (sel_iter_init && H5S_SELECT_ITER_RELEASE(sel_iter) < 0)
HDONE_ERROR(H5E_DATASPACE, H5E_CANTRELEASE, FAIL, "unable to release selection iterator")
sel_iter = H5FL_FREE(H5S_sel_iter_t, sel_iter);
}

#ifdef H5S_DEBUG
if (H5DEBUG(S))
Expand Down Expand Up @@ -1364,7 +1389,7 @@ H5S__obtain_datatype(H5S_hyper_span_info_t *spans, const hsize_t *down, size_t e
*-------------------------------------------------------------------------
*/
herr_t
H5S_mpio_space_type(const H5S_t *space, size_t elmt_size, MPI_Datatype *new_type, int *count,
H5S_mpio_space_type(H5S_t *space, size_t elmt_size, MPI_Datatype *new_type, int *count,
hbool_t *is_derived_type, hbool_t do_permute, hsize_t **permute_map, hbool_t *is_permuted)
{
herr_t ret_value = SUCCEED; /* Return value */
Expand Down
2 changes: 1 addition & 1 deletion src/H5Sprivate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ H5_DLL herr_t H5S_select_iter_release(H5S_sel_iter_t *sel_iter);
H5_DLL herr_t H5S_sel_iter_close(H5S_sel_iter_t *sel_iter);

#ifdef H5_HAVE_PARALLEL
H5_DLL herr_t H5S_mpio_space_type(const H5S_t *space, size_t elmt_size,
H5_DLL herr_t H5S_mpio_space_type(H5S_t *space, size_t elmt_size,
/* out: */ MPI_Datatype *new_type, int *count, hbool_t *is_derived_type,
hbool_t do_permute, hsize_t **permute_map, hbool_t *is_permuted);
#endif /* H5_HAVE_PARALLEL */
Expand Down
4 changes: 2 additions & 2 deletions src/H5Tconv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3406,8 +3406,8 @@ H5T__conv_vlen(hid_t src_id, hid_t dst_id, H5T_cdata_t *cdata, size_t nelmts, si
/* For nested VL case, free leftover heap objects from the deeper level if the
* length of new data elements is shorter than the old data elements.*/
if (nested && seq_len < bg_seq_len) {
const uint8_t *tmp;
size_t u;
uint8_t *tmp;
size_t u;

/* Sanity check */
HDassert(write_to_file);
Expand Down
2 changes: 1 addition & 1 deletion src/H5Tpkg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ typedef herr_t (*H5T_vlen_setnull_func_t)(H5VL_object_t *file, void *_vl, void *
typedef herr_t (*H5T_vlen_read_func_t)(H5VL_object_t *file, void *_vl, void *buf, size_t len);
typedef herr_t (*H5T_vlen_write_func_t)(H5VL_object_t *file, const H5T_vlen_alloc_info_t *vl_alloc_info,
void *_vl, void *buf, void *_bg, size_t seq_len, size_t base_size);
typedef herr_t (*H5T_vlen_delete_func_t)(H5VL_object_t *file, const void *_vl);
typedef herr_t (*H5T_vlen_delete_func_t)(H5VL_object_t *file, void *_vl);

/* VL datatype callbacks */
typedef struct H5T_vlen_class_t {
Expand Down
9 changes: 9 additions & 0 deletions src/H5Tref.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,14 @@ H5T__ref_mem_write(H5VL_object_t *src_file, const void *src_buf, size_t src_size
*
*-------------------------------------------------------------------------
*/
/*
* It is the correct thing to do to have the reference buffer
* be const-qualified here, but the VOL "blob specific" routine
* needs a non-const pointer since an operation might modify
* the "blob". Disable this warning here (at least temporarily)
* for this reason.
*/
H5_GCC_CLANG_DIAG_OFF("cast-qual")
static herr_t
H5T__ref_disk_isnull(const H5VL_object_t *src_file, const void *src_buf, hbool_t *isnull)
{
Expand Down Expand Up @@ -742,6 +750,7 @@ H5T__ref_disk_isnull(const H5VL_object_t *src_file, const void *src_buf, hbool_t
done:
FUNC_LEAVE_NOAPI(ret_value)
} /* end H5T__ref_disk_isnull() */
H5_GCC_CLANG_DIAG_ON("cast-qual")

/*-------------------------------------------------------------------------
* Function: H5T__ref_disk_setnull
Expand Down
16 changes: 8 additions & 8 deletions src/H5Tvlen.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ static herr_t H5T__vlen_disk_setnull(H5VL_object_t *file, void *_vl, void *_bg);
static herr_t H5T__vlen_disk_read(H5VL_object_t *file, void *_vl, void *_buf, size_t len);
static herr_t H5T__vlen_disk_write(H5VL_object_t *file, const H5T_vlen_alloc_info_t *vl_alloc_info, void *_vl,
void *_buf, void *_bg, size_t seq_len, size_t base_size);
static herr_t H5T__vlen_disk_delete(H5VL_object_t *file, const void *_vl);
static herr_t H5T__vlen_disk_delete(H5VL_object_t *file, void *_vl);

/*********************/
/* Public Variables */
Expand Down Expand Up @@ -955,9 +955,9 @@ static herr_t
H5T__vlen_disk_write(H5VL_object_t *file, const H5T_vlen_alloc_info_t H5_ATTR_UNUSED *vl_alloc_info,
void *_vl, void *buf, void *_bg, size_t seq_len, size_t base_size)
{
uint8_t * vl = (uint8_t *)_vl; /* Pointer to the user's hvl_t information */
const uint8_t *bg = (const uint8_t *)_bg; /* Pointer to the old data hvl_t */
herr_t ret_value = SUCCEED; /* Return value */
uint8_t *vl = (uint8_t *)_vl; /* Pointer to the user's hvl_t information */
uint8_t *bg = (uint8_t *)_bg; /* Pointer to the old data hvl_t */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_STATIC

Expand Down Expand Up @@ -995,10 +995,10 @@ H5T__vlen_disk_write(H5VL_object_t *file, const H5T_vlen_alloc_info_t H5_ATTR_UN
*-------------------------------------------------------------------------
*/
static herr_t
H5T__vlen_disk_delete(H5VL_object_t *file, const void *_vl)
H5T__vlen_disk_delete(H5VL_object_t *file, void *_vl)
{
const uint8_t *vl = (const uint8_t *)_vl; /* Pointer to the user's hvl_t information */
herr_t ret_value = SUCCEED; /* Return value */
uint8_t *vl = (uint8_t *)_vl; /* Pointer to the user's hvl_t information */
herr_t ret_value = SUCCEED; /* Return value */

FUNC_ENTER_STATIC

Expand All @@ -1014,7 +1014,7 @@ H5T__vlen_disk_delete(H5VL_object_t *file, const void *_vl)

/* Delete object, if length > 0 */
if (seq_len > 0)
if (H5VL_blob_specific(file, (void *)vl, H5VL_BLOB_DELETE) < 0) /* Casting away 'const' OK -QAK */
if (H5VL_blob_specific(file, vl, H5VL_BLOB_DELETE) < 0)
HGOTO_ERROR(H5E_DATATYPE, H5E_CANTREMOVE, FAIL, "unable to delete blob")
} /* end if */

Expand Down
9 changes: 9 additions & 0 deletions test/h5test.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,15 @@ H5TEST_DLLVAR MPI_Info h5_io_info_g; /* MPI INFO object for IO */
extern "C" {
#endif

/*
* Ugly hack to cast away const for freeing const-qualified pointers.
* Should only be used sparingly, where the alternative (like keeping
* an equivalent non-const pointer around) is far messier.
*/
#ifndef h5_free_const
#define h5_free_const(mem) HDfree((void *)(uintptr_t)mem)
#endif

/* Generally useful testing routines */
H5TEST_DLL void h5_clean_files(const char *base_name[], hid_t fapl);
H5TEST_DLL int h5_cleanup(const char *base_name[], hid_t fapl);
Expand Down
Loading

0 comments on commit a615a2b

Please sign in to comment.