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

Warnings fixes #1680

Merged
merged 1 commit into from
Apr 25, 2022
Merged

Warnings fixes #1680

merged 1 commit into from
Apr 25, 2022

Conversation

jhendersonHDF
Copy link
Collaborator

No description provided.

@@ -1865,7 +1865,7 @@ H5FD__mpio_vector_build_types(uint32_t count, H5FD_mem_t types[], haddr_t addrs[
if (!sub_types) {
HDassert(!sub_types_created);

if (NULL == (sub_types = (int *)HDmalloc((size_t)count * sizeof(MPI_Datatype))))
if (NULL == (sub_types = HDmalloc((size_t)count * sizeof(MPI_Datatype))))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sub_types is an MPI_Datatype *, not an int *

Copy link
Member

Choose a reason for hiding this comment

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

There's no need to cast the allocated pointers in C anyways.

@@ -2694,18 +2694,18 @@ my_isnan(dtype_t type, void *val)
if (FLT_FLOAT == type) {
float x = 0.0;
HDmemcpy(&x, val, sizeof(float));
retval = (x != x);
retval = !H5_FLT_ABS_EQUAL(x, x);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't compare floats, doubles, long doubles using != here. Use library macro to compare with an epsilon value instead.

@@ -4911,13 +4922,13 @@ test_vector_io(const char *vfd_name)

for (i = 0; i < count; i++) {

HDfree(write_bufs_0[i]);
HDfree((void *)write_bufs_0[i]);
Copy link
Collaborator Author

@jhendersonHDF jhendersonHDF Apr 24, 2022

Choose a reason for hiding this comment

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

Since H5FDwrite_vector takes a const void ** argument, the cleanest way to fix the warnings in this file is to have them actually be const void **. Unfortunately that means we will always have const warnings from freeing those pointers, unless we use our flex_ptr union hack here. For now, the explicit cast to (void *) is meant to highlight that.

@@ -1151,14 +1151,14 @@ single_rank_independent_io(void)
VRFY_G((mspace_id >= 0), "H5Screate_simple mspace_id succeeded");

/* Write data */
H5Dwrite(dset_id, H5T_NATIVE_INT, mspace_id, fspace_id, H5P_DEFAULT, data);
ret = H5Dwrite(dset_id, H5T_NATIVE_INT, mspace_id, fspace_id, H5P_DEFAULT, data);
VRFY_G((ret >= 0), "H5Dwrite succeeded");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ret had never been initialized or set in this test file, so it's very possible t_bigio might've had sporadic failures from checking an uninitialized ret.

@@ -615,13 +615,15 @@ H5D__contig_may_use_select_io(const H5D_io_info_t *io_info, H5D_io_op_type_t op_
HDassert(dataset);
HDassert(op_type == H5D_IO_OP_READ || op_type == H5D_IO_OP_WRITE);

dataset = io_info->dset;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be immediately before the HDassert(dataset) line, otherwise that assert will always be tripped.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Was building in release so missed this one! Just fixed the assertion and pushed.

@@ -312,5 +312,5 @@ h5diff_exit(int status)
/* Always exit(0), since MPI implementations do weird stuff when they
* receive a non-zero exit value. - QAK
*/
HDexit(0);
HDexit(status);
Copy link
Contributor

@qkoziol qkoziol Apr 24, 2022

Choose a reason for hiding this comment

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

This shouldn't be changed (per my comment immediately above :-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting if there are still MPI implementations that behave weird. On the other hand, just a few lines up we always set status to EXIT_SUCCESS when in parallel, and EXIT_SUCCESS should be equal to 0, so (I think?) this shouldn't be a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

Worth a try :-)

Copy link
Member

@derobins derobins Apr 24, 2022

Choose a reason for hiding this comment

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

EXIT_SUCCESS is used instead of 0/1 because the "good" and "bad" error codes are platform-specific. VMS/OpenVMS were weird about return codes, for example. I wouldn't count on EXIT_SUCCESS always being zero, even if that's a practical assumption today. That said, I would assume that MPI on a "weird" system would do the right thing with EXIT_SUCCESS.

Copy link
Contributor

@qkoziol qkoziol left a comment

Choose a reason for hiding this comment

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

Two small fixes needed, but otherwise looks good

@lrknox lrknox merged commit 2b32315 into HDFGroup:develop Apr 25, 2022
@jhendersonHDF jhendersonHDF deleted the warnings_fixes branch April 25, 2022 17:28
derobins added a commit that referenced this pull request Apr 26, 2022
* Removes unused definitions from module headers (#1624)

* Fix these Doxygen warnings #1581 (#1589)

* Fixes a typo in H5.c (#1639)

* free MPI_Group/MPI_Comm/MPI_Datatype objects (#1638)

* free MPI_Group/MPI_Comm/MPI_Datatype objects

* fix clang-format style

* Adds build and license shields to README.md (#1641)

* First stab at a Github status bar

* Adds a .tokeignore file for counting lines of code accurately

* Yanks lines of code calculation since it wildly overcounts

* not depend on doIO to free an MPI_Comm object (#1642)

* free MPI datatypes previously created (#1637)

* Retrieve MPI-IO hints used by MPI library after file open (#1636)

H5Pget_fapl_mpio() should return an MPI info object containing all the
MPI-IO hints used by the MPI library underneath, after the file is
opened. Some hints, such as cb_nodes (number of I/O aggregators), are
useful for HDF5 applications and I/O libraries built on top of HDF5.

* OESS-168: Remove clang warnings. (#1309)

* OESS-168: Remove clang warnings.

* OESS-168: Address @lrknox review.

* OESS-168: Remove clang warnings. (#1376)

* Remove H5_NO_ALIGNMENT_RESTRICTIONS (#1426)

* Do not conditionally compile code that uses a pointer dereference
and assignment to copy a potentially unaligned variable to aligned
automatic storage, or vice versa.  Instead, always use naked `memcpy(3)`s.
Disassembling the generated code reveals that the `memcpy(3)`s optimize
(`-O3`) to a single `mov` instruction for x86_64, which is not strict
about alignment.

This change reduces the size of code and scripts by 143 lines, eases
our way to cross-compilation, and avoids invoking undefined behavior.

* Committing clang-format changes

* Per discussion, use HD and add comments.

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

* Cleans up some HL library code that inappropriately returns htri_t values cast to herr_t (#1651)

* Cleans up some HL library code that inappropriately returns
htri_t values cast to herr_t

* Committing clang-format changes

* Formatted source

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

* Mirror vfd test fixes (#1629)

* Use the FAPL that was created earlier in the test (and delete an unused
variable).   This allows 'make check-vfd' to pass with --enable-mirror-vfd.

* Check for testing directory before creating, to avoid warning from bash.
Clean out .libs directory before re-using it (after a failed test), to
remove any files generated by libtool.

* Committing clang-format changes

* Increment error count on failed file open and skip tests for VFDs that need
modified filenames.

* Skip the mirror VFD for 'make check-vfd' - the mirror VFD requires networking
configuration parameters and can't be provided for an automated test that
is configured with an environment variable.

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

* Removes HDF Group paths, adds shellcheck fixes (#1656)

For more information:
  https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
  https://www.shellcheck.net/wiki/SC2230 -- which is non-standard. Use builti...
  https://www.shellcheck.net/wiki/SC2006 -- Use $(...) notation instead of le...

* HDFFV-11306 Fixed (#1657)

* HDFFV-11306,
 * Fixed it so both h5open_f and h5close_f can be called multiple times.
 * Fixed an issue with open objects remaining after h5close_f was called.
 * Added additional tests.

* comments clean-up

* Develop clang format java (#1653)

* added HDFFV-11306 entry (#1662)

* Adds the -q flag to all swmr test programs, quieting noisy output (#1665)

* Adds paths-ignore to the Github pull request workflow (#1663)

* Changes Github action `hdf5 dev CI` to `PR hdf5 dev CI` (#1666)

So the PR action name is not the same as the one in main.yml

* Replace H5detect's build-time detection of C99 integer properties with a (#1400)

* Replace H5detect's build-time detection of C99 integer properties with a
table-driven routine, `H5T__init_native_int()`, that is run at library
initialization time.

* Improve handling of copying of dynamic libraries and clean them up after (#1681)

test finishes.

* Warnings fixes (#1680)

* Clean stack size warnings in sio_engine (#1687)

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

* Minor things noticed while bringing VFD SWMR in line with develop (#1691)

* Removed dead code, weird formatting, and other badness

* Fixed remaining stack size warnings in onion VFD

* Committing clang-format changes

Co-authored-by: Allen Byrne <50328838+byrnHDF@users.noreply.github.com>
Co-authored-by: Wei-keng Liao <wkliao@users.noreply.github.com>
Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
Co-authored-by: David Young <dyoung@hdfgroup.org>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Quincey Koziol <koziol@lbl.gov>
Co-authored-by: Scot Breitenfeld <brtnfld@hdfgroup.org>
Co-authored-by: jhendersonHDF <jhenderson@hdfgroup.org>
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 <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>
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 <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>
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