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.

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

Conversation

jhendersonHDF
Copy link
Collaborator

Attempt to address some of the const cast warnings that are reasonable to fix. Most of the remaining ones are involved enough to be separate PRs on their own.

@@ -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.

@@ -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.

@@ -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.

@@ -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.

@@ -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.

(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.

@@ -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

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.

@@ -927,13 +926,15 @@ H5G__loc_set_comment_cb(H5G_loc_t H5_ATTR_UNUSED *grp_loc /*in*/, const char H5_

/* Add the new message */
if (udata->comment && *udata->comment) {
/* Casting away const OK -QAK */
comment.s = (char *)udata->comment;
if (NULL == (comment.s = HDstrdup(udata->comment)))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Short of playing games with our flexible const pointer type, the easiest way to solve this appears to be taking a copy of the object comment passed in from above, since H5O_msg_create accepts a non-const message pointer.

@lrknox lrknox merged commit d7565f3 into HDFGroup:develop Apr 26, 2022
@jhendersonHDF jhendersonHDF deleted the const_cast_fixes branch April 28, 2022 04:27
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request May 2, 2022
derobins added a commit that referenced this pull request May 3, 2022
* Warnings fixes (#1680)

* Clean stack size warnings in sio_engine (#1687)

* Fixes stack size warnings in tcoords.c (#1688)

* Address some warnings from casting away of const (#1684)

* Fixes stack size warnings in ntypes (#1695)

* Fixes stack size warnings in dtransform (#1696)

* Fixes stack size warnings in set_extent test (#1698)

* Be a bit safer with signed arithmetic, thus quieting some signed-overflow warnings from GCC (#1706)

* Avoid a signed overflow: check the range of `entry_ptr->age` before
increasing it instead of increasing it and then checking the range.
This quiets a GCC warning.

* Avoid the potential for signed overflow by rewriting expressions
`MAX(0, fwidth - n)` as `MAX(n, fwidth) - n` for various `n`.
This change quiets some GCC warnings.

* Change some local variables that cannot take sensible negative values
from signed to unsigned.  This quiets GCC warnings about potential
signed overflow.

* In a handful of instances, check the range of a signed integer before
increasing/decreasing it, just in case the increase/decrease overflows.
This quiets a handful of GCC signed-overflow warnings.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Fix object size warnings in cache.c test (#1701)

* Fix some const cast and stack/static object size warnings (#1700)

* 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

* Stop lying about H5S_t const-ness (#1209)

Hyperslabs can be reworked inside several H5S callbacks, making H5S_t
non-const in some places where it is marked const. This change switches
these incorrectly const H5S_t pointer parameters and variables to
non-const where appropriate.

* Fix a few warnings after recent H5S const-related changes (#1225)

* Adjustments for HDF5 1.12

Co-authored-by: Dana Robinson <[email protected]>
Co-authored-by: David Young <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
jhendersonHDF added a commit to jhendersonHDF/hdf5 that referenced this pull request May 8, 2022
derobins added a commit that referenced this pull request May 8, 2022
* Warnings fixes (#1680)

* Clean stack size warnings in sio_engine (#1687)

* Fixes stack size warnings in tcoords.c (#1688)

* Address some warnings from casting away of const (#1684)

* Fixes stack size warnings in dtransform (#1696)

* Fixes stack size warnings in set_extent test (#1698)

* Be a bit safer with signed arithmetic, thus quieting some signed-overflow warnings from GCC (#1706)

* Avoid a signed overflow: check the range of `entry_ptr->age` before
increasing it instead of increasing it and then checking the range.
This quiets a GCC warning.

* Avoid the potential for signed overflow by rewriting expressions
`MAX(0, fwidth - n)` as `MAX(n, fwidth) - n` for various `n`.
This change quiets some GCC warnings.

* Change some local variables that cannot take sensible negative values
from signed to unsigned.  This quiets GCC warnings about potential
signed overflow.

* In a handful of instances, check the range of a signed integer before
increasing/decreasing it, just in case the increase/decrease overflows.
This quiets a handful of GCC signed-overflow warnings.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Fix object size warnings in cache.c test (#1701)

* Fix some const cast and stack/static object size warnings (#1700)

* 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

* Fixes a bug where t_cache fails due to a string size being too small (#1720)

* Fixes a bug where t_cache fails due to a string size being too small

Recent warning reductions led to an incorrect string size being passed
to h5_fileaccess, causing the test to silently fail. In addition to
fixing the bug, the test will now fail noisily on setup failures.

* Updates the t_cache test to fail noisily on setup errors

* Fix a few Clang sanitizer warnings (#1727)

* Stop lying about H5S_t const-ness (#1209)

Hyperslabs can be reworked inside several H5S callbacks, making H5S_t
non-const in some places where it is marked const. This change switches
these incorrectly const H5S_t pointer parameters and variables to
non-const where appropriate.

* Fix a few warnings after recent H5S const-related changes (#1225)

* Adjustments for HDF5 1.10

* Hdf5 1 12 Miscellaneous warnings fixes (#1718)

* Fixes const issues in the version 2 B-trees (#1172)

The operations that were changed are fundamentally not const since the
shadow operation can modify the node structure when SWMR is in use.

* Quiets const warning in H5RS code (#1181)

* Avoid calling H5Ropen_object with a misaligned H5R_ref_t: copy the (#1171)

* Avoid calling H5Ropen_object with a misaligned H5R_ref_t: copy the
raw H5R_ref_t bytes to a heap buffer that's known to have the right
alignment.

* Committing clang-format changes

* Use an automatic H5R_ref_t instead of malloc'ing one.  Go ahead and
initialize the H5R_ref_t to all-0s so that arbitrary stack content
doesn't foul things up.  Bail out with an error if `size` exceeds
`sizeof(H5R_ref_t)`.

* Committing clang-format changes

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Miscellaneous warnings fixes

Co-authored-by: Dana Robinson <[email protected]>
Co-authored-by: David Young <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>

* Fix several warnings (#747)

Co-authored-by: Dana Robinson <[email protected]>
Co-authored-by: David Young <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants