Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address some warnings from casting away of const #1684

Merged
merged 1 commit into from
Apr 26, 2022
+97 −121
Merged

Address some warnings from casting away of const #1684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 31 additions & 36 deletions src/H5Dchunk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1143,8 +1143,7 @@ H5D__chunk_io_init(H5D_io_info_t *io_info, const H5D_type_info_t *type_info, hsi
fm->mem_space = NULL;

if (file_space_normalized == TRUE)
if (H5S_hyper_denormalize_offset((H5S_t *)file_space, old_offset) <
0) /* (Casting away const OK -QAK) */
if (H5S_hyper_denormalize_offset(file_space, old_offset) < 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

file_space here was already changed to not be const-qualified. Just remove the comment and cast.

HDONE_ERROR(H5E_DATASET, H5E_CANTSET, FAIL, "can't denormalize selection")

FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -1371,15 +1370,16 @@ H5D__chunk_io_init_selections(const H5D_io_info_t *io_info, const H5D_type_info_
*-------------------------------------------------------------------------
*/
void *
H5D__chunk_mem_alloc(size_t size, const H5O_pline_t *pline)
H5D__chunk_mem_alloc(size_t size, void *pline)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These routines in H5Dchunk.c didn't match the H5MM_allocate_t and H5MM_free_t function pointer typedefs, but this was hidden by casting them to such. Correct that and do the casting to H5O_pline_t in the routine. I recall this being changed from void * to H5O_pline_t * previously, but let's actually match our specification for the typedefs and not hide that.

{
void *ret_value = NULL; /* Return value */
H5O_pline_t *_pline = (H5O_pline_t *)pline;
void * ret_value = NULL; /* Return value */

FUNC_ENTER_STATIC_NOERR

HDassert(size);

if (pline && pline->nused)
if (_pline && _pline->nused)
ret_value = H5MM_malloc(size);
else
ret_value = H5FL_BLK_MALLOC(chunk, size);
Expand All @@ -1402,14 +1402,14 @@ H5D__chunk_mem_alloc(size_t size, const H5O_pline_t *pline)
*-------------------------------------------------------------------------
*/
void *
H5D__chunk_mem_xfree(void *chk, const void *_pline)
H5D__chunk_mem_xfree(void *chk, const void *pline)
{
const H5O_pline_t *pline = (const H5O_pline_t *)_pline;
const H5O_pline_t *_pline = (const H5O_pline_t *)pline;

FUNC_ENTER_STATIC_NOERR

if (chk) {
if (pline && pline->nused)
if (_pline && _pline->nused)
H5MM_xfree(chk);
else
chk = H5FL_BLK_FREE(chunk, chk);
Expand All @@ -1426,9 +1426,9 @@ H5D__chunk_mem_xfree(void *chk, const void *_pline)
*-------------------------------------------------------------------------
*/
void
H5D__chunk_mem_free(void *chk, const void *_pline)
H5D__chunk_mem_free(void *chk, void *pline)
{
(void)H5D__chunk_mem_xfree(chk, _pline);
(void)H5D__chunk_mem_xfree(chk, pline);
}

/*-------------------------------------------------------------------------
Expand Down Expand Up @@ -1580,8 +1580,7 @@ H5D__create_chunk_map_single(H5D_chunk_map_t *fm, const H5D_io_info_t
chunk_info->fspace_shared = TRUE;

/* Just point at the memory dataspace & selection */
/* (Casting away const OK -QAK) */
chunk_info->mspace = (H5S_t *)fm->mem_space;
chunk_info->mspace = fm->mem_space;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fm->mem_space already had const removed.


/* Indicate that the chunk's memory dataspace is shared */
chunk_info->mspace_shared = TRUE;
Expand Down Expand Up @@ -1856,7 +1855,6 @@ H5D__create_chunk_file_map_hyper(H5D_chunk_map_t *fm, const H5D_io_info_t
/* Iterate through each chunk in the dataset */
while (sel_points) {
/* Check for intersection of current chunk and file selection */
/* (Casting away const OK - QAK) */
if (TRUE == H5S_SELECT_INTERSECT_BLOCK(fm->file_space, coords, end)) {
H5D_chunk_info_t *new_chunk_info; /* chunk information to insert into skip list */
hsize_t chunk_points; /* Number of elements in chunk selection */
Expand Down Expand Up @@ -2015,8 +2013,7 @@ H5D__create_chunk_mem_map_hyper(const H5D_chunk_map_t *fm)
HDassert(chunk_info);

/* Just point at the memory dataspace & selection */
/* (Casting away const OK -QAK) */
chunk_info->mspace = (H5S_t *)fm->mem_space;
chunk_info->mspace = fm->mem_space;

/* Indicate that the chunk's memory space is shared */
chunk_info->mspace_shared = TRUE;
Expand Down Expand Up @@ -2138,8 +2135,7 @@ H5D__create_chunk_mem_map_1d(const H5D_chunk_map_t *fm)
HDassert(chunk_info);

/* Just point at the memory dataspace & selection */
/* (Casting away const OK -QAK) */
chunk_info->mspace = (H5S_t *)fm->mem_space;
chunk_info->mspace = fm->mem_space;

/* Indicate that the chunk's memory space is shared */
chunk_info->mspace_shared = TRUE;
Expand Down Expand Up @@ -4104,11 +4100,11 @@ H5D__chunk_cache_prune(const H5D_t *dset, size_t size)
static void *
H5D__chunk_lock(const H5D_io_info_t *io_info, H5D_chunk_ud_t *udata, hbool_t relax, hbool_t prev_unfilt_chunk)
{
const H5D_t * dset = io_info->dset; /* Local pointer to the dataset info */
const H5O_pline_t *pline =
const H5D_t *dset = io_info->dset; /* Local pointer to the dataset info */
H5O_pline_t *pline =
&(dset->shared->dcpl_cache
.pline); /* I/O pipeline info - always equal to the pline passed to H5D__chunk_mem_alloc */
const H5O_pline_t * old_pline = pline; /* Old pipeline, i.e. pipeline used to read the chunk */
H5O_pline_t * old_pline = pline; /* Old pipeline, i.e. pipeline used to read the chunk */
const H5O_layout_t *layout = &(dset->shared->layout); /* Dataset layout */
const H5O_fill_t * fill = &(dset->shared->dcpl_cache.fill); /* Fill value info */
H5D_fill_buf_info_t fb_info; /* Dataset's fill buffer info */
Expand Down Expand Up @@ -4688,18 +4684,18 @@ H5D__chunk_allocate(const H5D_io_info_t *io_info, hbool_t full_overwrite, const
coordinates) */
hsize_t max_unalloc[H5O_LAYOUT_NDIMS]; /* Last chunk in each dimension that is unallocated (in scaled
coordinates) */
hsize_t scaled[H5O_LAYOUT_NDIMS]; /* Offset of current chunk (in scaled coordinates) */
size_t orig_chunk_size; /* Original size of chunk in bytes */
size_t chunk_size; /* Actual size of chunk in bytes, possibly filtered */
unsigned filter_mask = 0; /* Filter mask for chunks that have them */
H5O_layout_t * layout = &(dset->shared->layout); /* Dataset layout */
const H5O_pline_t *pline = &(dset->shared->dcpl_cache.pline); /* I/O pipeline info */
const H5O_pline_t def_pline = H5O_CRT_PIPELINE_DEF; /* Default pipeline */
const H5O_fill_t * fill = &(dset->shared->dcpl_cache.fill); /* Fill value info */
H5D_fill_value_t fill_status; /* The fill value status */
hbool_t should_fill = FALSE; /* Whether fill values should be written */
void * unfilt_fill_buf = NULL; /* Unfiltered fill value buffer */
void ** fill_buf = NULL; /* Pointer to the fill buffer to use for a chunk */
hsize_t scaled[H5O_LAYOUT_NDIMS]; /* Offset of current chunk (in scaled coordinates) */
size_t orig_chunk_size; /* Original size of chunk in bytes */
size_t chunk_size; /* Actual size of chunk in bytes, possibly filtered */
unsigned filter_mask = 0; /* Filter mask for chunks that have them */
H5O_layout_t * layout = &(dset->shared->layout); /* Dataset layout */
H5O_pline_t * pline = &(dset->shared->dcpl_cache.pline); /* I/O pipeline info */
H5O_pline_t def_pline = H5O_CRT_PIPELINE_DEF; /* Default pipeline */
const H5O_fill_t *fill = &(dset->shared->dcpl_cache.fill); /* Fill value info */
H5D_fill_value_t fill_status; /* The fill value status */
hbool_t should_fill = FALSE; /* Whether fill values should be written */
void * unfilt_fill_buf = NULL; /* Unfiltered fill value buffer */
void ** fill_buf = NULL; /* Pointer to the fill buffer to use for a chunk */
#ifdef H5_HAVE_PARALLEL
hbool_t blocks_written = FALSE; /* Flag to indicate that chunk was actually written */
hbool_t using_mpi =
Expand Down Expand Up @@ -4804,10 +4800,9 @@ H5D__chunk_allocate(const H5D_io_info_t *io_info, hbool_t full_overwrite, const
if (should_fill) {
/* Initialize the fill value buffer */
/* (delay allocating fill buffer for VL datatypes until refilling) */
/* (casting away const OK - QAK) */
if (H5D__fill_init(&fb_info, NULL, (H5MM_allocate_t)H5D__chunk_mem_alloc, (void *)pline,
(H5MM_free_t)H5D__chunk_mem_free, (void *)pline, &dset->shared->dcpl_cache.fill,
dset->shared->type, dset->shared->type_id, (size_t)0, orig_chunk_size) < 0)
if (H5D__fill_init(&fb_info, NULL, H5D__chunk_mem_alloc, pline, H5D__chunk_mem_free, pline,
&dset->shared->dcpl_cache.fill, dset->shared->type, dset->shared->type_id,
(size_t)0, orig_chunk_size) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "can't initialize fill buffer info")
fb_info_init = TRUE;

Expand Down
10 changes: 5 additions & 5 deletions src/H5Dint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ typedef struct {

/* General stuff */
static H5D_shared_t *H5D__new(hid_t dcpl_id, hid_t dapl_id, hbool_t creating, hbool_t vl_type);
static herr_t H5D__init_type(H5F_t *file, const H5D_t *dset, hid_t type_id, const H5T_t *type);
static herr_t H5D__init_type(H5F_t *file, const H5D_t *dset, hid_t type_id, H5T_t *type);
static herr_t H5D__cache_dataspace_info(const H5D_t *dset);
static herr_t H5D__init_space(H5F_t *file, const H5D_t *dset, const H5S_t *space);
static herr_t H5D__update_oh_info(H5F_t *file, H5D_t *dset, hid_t dapl_id);
Expand Down Expand Up @@ -488,7 +488,7 @@ H5D__new(hid_t dcpl_id, hid_t dapl_id, hbool_t creating, hbool_t vl_type)
*-------------------------------------------------------------------------
*/
static herr_t
H5D__init_type(H5F_t *file, const H5D_t *dset, hid_t type_id, const H5T_t *type)
H5D__init_type(H5F_t *file, const H5D_t *dset, hid_t type_id, H5T_t *type)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we might take the input type and directly assign it to a modifiable dset->shared->type field, let's not lie about the type parameter's const-ness here.

{
htri_t relocatable; /* Flag whether the type is relocatable */
htri_t immutable; /* Flag whether the type is immutable */
Expand Down Expand Up @@ -544,8 +544,8 @@ H5D__init_type(H5F_t *file, const H5D_t *dset, hid_t type_id, const H5T_t *type)

/* Use existing datatype */
dset->shared->type_id = type_id;
dset->shared->type = (H5T_t *)type; /* (Cast away const OK - QAK) */
} /* end else */
dset->shared->type = type;
} /* end else */

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -2192,7 +2192,7 @@ H5D_oloc(H5D_t *dataset)
*-------------------------------------------------------------------------
*/
H5G_name_t *
H5D_nameof(const H5D_t *dataset)
H5D_nameof(H5D_t *dataset)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While in theory returning the name of an object should be able to accept a const qualified pointer to the object, the returned value can potentially be used in a non-const way, so until we can properly architect things, the _nameof routines for objects should just accept non-const-qualified pointers to objects. Note that this follows the same as the _oloc routines for objects.

{
/* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand Down
26 changes: 13 additions & 13 deletions src/H5Dio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,21 @@ H5D__read(H5D_t *dataset, hid_t mem_type_id, H5S_t *mem_space, H5S_t *file_space
*/
if (nelmts > 0 && TRUE == H5S_SELECT_SHAPE_SAME(mem_space, file_space) &&
H5S_GET_EXTENT_NDIMS(mem_space) != H5S_GET_EXTENT_NDIMS(file_space)) {
const void *adj_buf = NULL; /* Pointer to the location in buf corresponding */
/* to the beginning of the projected mem space. */
ptrdiff_t buf_adj = 0;

/* Attempt to construct projected dataspace for memory dataspace */
if (H5S_select_construct_projection(mem_space, &projected_mem_space,
(unsigned)H5S_GET_EXTENT_NDIMS(file_space), buf, &adj_buf,
type_info.dst_type_size) < 0)
(unsigned)H5S_GET_EXTENT_NDIMS(file_space),
type_info.dst_type_size, &buf_adj) < 0)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the read case, having H5S_select_construct_projection always take const-qualified pointers to buffers and try to return const-qualified pointers to potentially adjusted buffers is problematic. The non-const read buffer goes in as const-qualified and comes back as const-qualified, then we cast away the const since the buffer should be modifiable.

Rather than having H5S_select_construct_projection try to do the adjusting of buffers, just return an adjustment amount and let the higher level do the adjusting of buffers, which works fine for both the read and write case.

HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to construct projected memory dataspace")
HDassert(projected_mem_space);
HDassert(adj_buf);

/* Adjust the buffer by the given amount */
buf = (void *)(((uint8_t *)buf) + buf_adj);

/* Switch to using projected memory dataspace & adjusted buffer */
mem_space = projected_mem_space;
buf = (void *)adj_buf; /* Casting away 'const' OK -QAK */
} /* end if */
} /* end if */

/* Retrieve dataset properties */
/* <none needed in the general case> */
Expand Down Expand Up @@ -407,20 +407,20 @@ H5D__write(H5D_t *dataset, hid_t mem_type_id, H5S_t *mem_space, H5S_t *file_spac
*/
if (nelmts > 0 && TRUE == H5S_SELECT_SHAPE_SAME(mem_space, file_space) &&
H5S_GET_EXTENT_NDIMS(mem_space) != H5S_GET_EXTENT_NDIMS(file_space)) {
const void *adj_buf = NULL; /* Pointer to the location in buf corresponding */
/* to the beginning of the projected mem space. */
ptrdiff_t buf_adj = 0;

/* Attempt to construct projected dataspace for memory dataspace */
if (H5S_select_construct_projection(mem_space, &projected_mem_space,
(unsigned)H5S_GET_EXTENT_NDIMS(file_space), buf, &adj_buf,
type_info.src_type_size) < 0)
(unsigned)H5S_GET_EXTENT_NDIMS(file_space),
type_info.src_type_size, &buf_adj) < 0)
HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to construct projected memory dataspace")
HDassert(projected_mem_space);
HDassert(adj_buf);

/* Adjust the buffer by the given amount */
buf = (const void *)(((const uint8_t *)buf) + buf_adj);

/* Switch to using projected memory dataspace & adjusted buffer */
mem_space = projected_mem_space;
buf = adj_buf;
} /* end if */

/* Retrieve dataset properties */
Expand Down
8 changes: 4 additions & 4 deletions src/H5Dpkg.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,8 +594,8 @@ H5_DLL herr_t H5D__select_write(const H5D_io_info_t *io_info, const H5D_type_inf
hsize_t nelmts, H5S_t *file_space, H5S_t *mem_space);

/* Functions that perform direct copying between memory buffers */
H5_DLL herr_t H5D_select_io_mem(void *dst_buf, const H5S_t *dst_space, const void *src_buf,
const H5S_t *src_space, size_t elmt_size, size_t nelmts);
H5_DLL herr_t H5D_select_io_mem(void *dst_buf, H5S_t *dst_space, const void *src_buf, H5S_t *src_space,
size_t elmt_size, size_t nelmts);

/* Functions that perform scatter-gather serial I/O operations */
H5_DLL herr_t H5D__scatter_mem(const void *_tscat_buf, H5S_sel_iter_t *iter, size_t nelmts, void *_buf);
Expand Down Expand Up @@ -641,8 +641,8 @@ H5_DLL herr_t H5D__chunk_allocate(const H5D_io_info_t *io_info, hbool_t full_ov
const hsize_t old_dim[]);
H5_DLL herr_t H5D__chunk_file_alloc(const H5D_chk_idx_info_t *idx_info, const H5F_block_t *old_chunk,
H5F_block_t *new_chunk, hbool_t *need_insert, const hsize_t *scaled);
H5_DLL void * H5D__chunk_mem_alloc(size_t size, const H5O_pline_t *pline);
H5_DLL void H5D__chunk_mem_free(void *chk, const void *_pline);
H5_DLL void * H5D__chunk_mem_alloc(size_t size, void *pline);
H5_DLL void H5D__chunk_mem_free(void *chk, void *pline);
H5_DLL void * H5D__chunk_mem_xfree(void *chk, const void *pline);
H5_DLL void * H5D__chunk_mem_realloc(void *chk, size_t size, const H5O_pline_t *pline);
H5_DLL herr_t H5D__chunk_update_old_edge_chunks(H5D_t *dset, hsize_t old_dim[]);
Expand Down
2 changes: 1 addition & 1 deletion src/H5Dprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ H5_DLL herr_t H5D_close(H5D_t *dataset);
H5_DLL herr_t H5D_mult_refresh_close(hid_t dset_id);
H5_DLL herr_t H5D_mult_refresh_reopen(H5D_t *dataset);
H5_DLL H5O_loc_t *H5D_oloc(H5D_t *dataset);
H5_DLL H5G_name_t *H5D_nameof(const H5D_t *dataset);
H5_DLL H5G_name_t *H5D_nameof(H5D_t *dataset);
H5_DLL herr_t H5D_flush_all(H5F_t *f);
H5_DLL hid_t H5D_get_create_plist(const H5D_t *dset);
H5_DLL hid_t H5D_get_access_plist(const H5D_t *dset);
Expand Down
4 changes: 2 additions & 2 deletions src/H5Dselect.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,8 @@ H5D__select_io(const H5D_io_info_t *io_info, size_t elmt_size, size_t nelmts, H5
*-------------------------------------------------------------------------
*/
herr_t
H5D_select_io_mem(void *dst_buf, const H5S_t *dst_space, const void *src_buf, const H5S_t *src_space,
size_t elmt_size, size_t nelmts)
H5D_select_io_mem(void *dst_buf, H5S_t *dst_space, const void *src_buf, H5S_t *src_space, size_t elmt_size,
size_t nelmts)
{
H5S_sel_iter_t *dst_sel_iter = NULL; /* Destination dataspace iteration info */
H5S_sel_iter_t *src_sel_iter = NULL; /* Source dataspace iteration info */
Expand Down
5 changes: 2 additions & 3 deletions src/H5Gent.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ H5G_ent_encode(const H5F_t *f, uint8_t **pp, const H5G_entry_t *ent)
*-------------------------------------------------------------------------
*/
void
H5G__ent_copy(H5G_entry_t *dst, const H5G_entry_t *src, H5_copy_depth_t depth)
H5G__ent_copy(H5G_entry_t *dst, H5G_entry_t *src, H5_copy_depth_t depth)
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Apr 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this routine should theoretically accept a const-qualified source H5G_entry_t, the pesky H5G_ent_reset call below means we're lying about the const-ness of the source parameter. Let's not do that for now, until future re-architecting. This is a common pattern in a few other spots (like H5G_name_copy), but fixing those is very involved.

{
FUNC_ENTER_PACKAGE_NOERR

Expand All @@ -318,8 +318,7 @@ H5G__ent_copy(H5G_entry_t *dst, const H5G_entry_t *src, H5_copy_depth_t depth)
;
}
else if (depth == H5_COPY_SHALLOW) {
/* Discarding 'const' qualifier OK - QAK */
H5G__ent_reset((H5G_entry_t *)src);
H5G__ent_reset(src);
} /* end if */

FUNC_LEAVE_NOAPI_VOID
Expand Down
2 changes: 1 addition & 1 deletion src/H5Gint.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ H5G_oloc(H5G_t *grp)
*-------------------------------------------------------------------------
*/
H5G_name_t *
H5G_nameof(const H5G_t *grp)
H5G_nameof(H5G_t *grp)
{
/* Use FUNC_ENTER_NOAPI_NOINIT_NOERR here to avoid performance issues */
FUNC_ENTER_NOAPI_NOINIT_NOERR
Expand Down
Loading