Fix some const cast and stack/static object size warnings #1700
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jhendersonHDF
requested review from
ChristopherHogan,
gnuoyd,
jrmainzer,
qkoziol,
fortnern,
soumagne,
vchoi-hdfgroup and
derobins
as code owners
jhendersonHDF
force-pushed
the
warnings_fixes
branch
from
e0fb3e4 to
9e89b81
Compare
derobins
reviewed
Apr 28, 2022
| MPI_Finalize(); | ||
|
|
||
| HDexit(EXIT_SUCCESS); | ||
|
|
||
| error: | ||
| if (data_g) | ||
| HDfree(data_g); |
There was a problem hiding this comment.
The check for data_g isn't necessary. You can call free() on a NULL pointer.
There was a problem hiding this comment.
I know; bad habit
derobins
reviewed
Apr 28, 2022
derobins
reviewed
Apr 28, 2022
test/vfd.c
Outdated
| */ | ||
| #ifndef HDfree_const | ||
| #define HDfree_const(mem) HDfree((void *)(uintptr_t)mem) | ||
| #endif |
There was a problem hiding this comment.
We should put this in H5private.h. We already have n H5MM flavor, but this could be used more widely.
There was a problem hiding this comment.
I do hate this hack, but I thought about putting it in H5private.h. I don't want to encourage people to do this, but it is kind of necessary in some cases. I can put it in H5private and leave a stern comment.
There was a problem hiding this comment.
I see your original comment now Dana, I feel like I'm more on Jordan's side on that, it was better to leave it in that particular test imo. I don't think encouraging that kind of hack is a good thing :)
derobins
reviewed
Apr 28, 2022
derobins
approved these changes
Apr 28, 2022
jhendersonHDF
force-pushed
the
warnings_fixes
branch
from
ad29770 to
dcf9e5a
Compare
soumagne
reviewed
Apr 28, 2022
| * the "blob". Disable this warning here (at least temporarily) | ||
| * for this reason. | ||
| */ | ||
| H5_GCC_CLANG_DIAG_OFF("cast-qual") |
There was a problem hiding this comment.
I feel like that could happen in several cases, we should probably fix that VOL callback to handle const and non-const
There was a problem hiding this comment.
Agreed. Just thought I'd temporarily disable it until we can do that. I'd like to eventually search for all the places we've turned things off with H5_GCC_CLANG_DIAG_OFF in the past and correctly fix things if we can, but that's for a later date.
soumagne
reviewed
Apr 28, 2022
src/H5private.h
Outdated
| */ | ||
| #ifndef HDfree_const | ||
| #define HDfree_const(mem) HDfree((void *)(uintptr_t)mem) | ||
| #endif |
There was a problem hiding this comment.
Hmm ok if that's only used in tests, I think you should leave it in tests because having that in H5private.h seems ugly
There was a problem hiding this comment.
I don't really like the way that this works, but we also do the same thing in H5MM_xfree_const. Right now it's only used in the tests so I only had it in the one test file, but moved it to H5private.h from Dana's suggestion. I can move it back if we don't really see it being useful elsewhere.
There was a problem hiding this comment.
Not being able to free const pointers is often considered to be a deficiency in the standard. If we leave it here, it shouldn't get HD as that implies it's a part of the platform-independence stuff in H5private.h. Maybe h5_free_const to go with the other tools functionality.
There was a problem hiding this comment.
Decided to move it to h5test.h for now, since the tests are where this macro will most likely be used. We can always move it back to H5private.h in the future if we use it more widely.
derobins
reviewed
Apr 28, 2022
soumagne
approved these changes
Apr 29, 2022
qkoziol
approved these changes
May 1, 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 <43805+derobins@users.noreply.github.com> Co-authored-by: David Young <dyoung@hdfgroup.org> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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 <43805+derobins@users.noreply.github.com> Co-authored-by: David Young <dyoung@hdfgroup.org> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> * Fix several warnings (#747) Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com> Co-authored-by: David Young <dyoung@hdfgroup.org> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
mkitti
added a commit
to mkitti/hdf5
that referenced
this pull request
Aug 5, 2022
mkitti
added a commit
to mkitti/hdf5
that referenced
this pull request
Dec 20, 2022
derobins
pushed a commit
that referenced
this pull request
Feb 21, 2023
* Backport H5Dchunk_iter to 1.10 branch * Add some accessory files, test still needs work * Apply proper formatting, and fix compile errors * Remove const from H5D__chunk_iter as per #1700 * Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info (#2074) * Align arg types of H5D_chunk_iter_op_t with H5Dget_chunk_info * Modify chunk_info test to for unsigned / hsize_t types * Fix types in test * Add test_basic_query, helper functions to test/chunk_info.c 1_10 * H5Dchunk_iter now passes offsets in units of dataset elements, fix #1419 (#1969) * H5Dchunk_iter now passes chunk dimension scaled offsets, fix #1419 * Update docs for H5Dchunk_iter, H5Dget_chunk_info* Modified description for `H5Dchunk_iter`, `H5Dget_chunk_info`, and `H5Dget_chunk_info_by_coord` to the following * offset Logical position of the chunk’s first element in units of dataset elements * filter_mask Bitmask indicating the filters used when the chunk was written * size Chunk size in bytes, 0 if the chunk does not exist * Fix regression of #1419 * Add a note about return fail in 1.12 and older for invalid chunk index * Committing clang-format changes * Run clang-format on test/chunk_info.c --------- 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.