Skip to content

Another round of warning cleanups on master #9903

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

Merged
merged 12 commits into from
Jan 24, 2022

Conversation

bwbarrett
Copy link
Member

These were all warnings on Ubuntu 20.04 (AARCH64) with its GCC 9.3.0.

@bwbarrett bwbarrett requested a review from awlauria January 20, 2022 22:47
@@ -545,13 +545,17 @@ ompi_report_comm_methods(int called_from_location) // 1 = from init, 2 = from fi
fp = fopen(mca_hook_comm_method_fakefile, "r");
for (i=0; i<nleaderranks; ++i) {
for (k=0; k<nleaderranks; ++k) {
fscanf(fp, "%d", &setting);
if (fscanf(fp, "%d", &setting) != 1) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't there be an fclose(fp) before the return?

Copy link
Member Author

Choose a reason for hiding this comment

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

I debated about the right thing to do here; it's a hook for testing, so one could even argue that just calling opal_output and then abort is the best option. But I changed the returns to break to pop out of the loop, which is probably good enough (and close to what we were doing before this change).

@ibm-ompi
Copy link

The IBM CI (GNU/Scale) build failed! Please review the log, linked below.

Gist: https://gist.github.com/1e6f3c08e91cdde8bdd798ea3a63d902

@ibm-ompi
Copy link

The IBM CI (XL) build failed! Please review the log, linked below.

Gist: https://gist.github.com/9db9122a75d288452203ee4e76966c94

@bwbarrett bwbarrett force-pushed the cleanup/warnings-suck branch from 2a49c18 to a6cde26 Compare January 20, 2022 23:16
@ibm-ompi
Copy link

The IBM CI (PGI) build failed! Please review the log, linked below.

Gist: https://gist.github.com/3b5b2fc4707643ec258a0ec4fea5d322

Copy link
Member

@gpaulsen gpaulsen left a comment

Choose a reason for hiding this comment

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

Just a few change requests.
Thanks.

@bwbarrett bwbarrett force-pushed the cleanup/warnings-suck branch from a6cde26 to dbdd6db Compare January 21, 2022 04:07
Type was only used in a debug print, which meant a warning in
non-debug builds.  Make the debug print dereference slightly
more complicated to remove the assignment of the unused variable
(and therefore the warning).

Signed-off-by: Brian Barrett <[email protected]>
With the removal of the runtime code from the OMPI code base, there
are no longer any users of the daemon init code.  Rather than
fix the warnings in the code, simply remove it.

Signed-off-by: Brian Barrett <[email protected]>
Remove unused function warning for the unused size_t max function.

Remove unused variable length array argument in the header.  While
the existance of the variable length array argument itself isn't
a problem, that header is included in the middle of another structure,
which is a no-no.  Since we don't use the variable length array
member, simplest solution is to remove it.

Copy the same compare and swap type enforcement for the 8 byte CAS
case as other similar cases (both 4 byte CAS and other atomics).

Signed-off-by: Brian Barrett <[email protected]>
realpath() can fail and return NULL.  Recent versions of Glibc,
including the one included in Ubuntu 20.04 on ARM, tag realpath
such that the compiler will emit a warning if the return code
is ignored.  In addition to fixing a real issue, this patch
silences an ignored return value warnig.

Signed-off-by: Brian Barrett <[email protected]>
Fix both a real (but unlikely) error and warning with the return code
of write().  Recent versions of Glibc tag the write() function to raise
a warning if the return value is ignored.  The issue is real, so do
a slightly better job of handling the issue and silence the warning.

Signed-off-by: Brian Barrett <[email protected]>
Glibc tags posix_memalign to generate a warning if the return value
is ignored.  In recent systems, this seems completely uncalled for,
since addr must not be touched.  But older systems did not enforce
that behavior, so reset addr and move on.

Signed-off-by: Brian Barrett <[email protected]>
Fix warnings in non-debug builds about unused variables by folding
into debug areas.

Signed-off-by: Brian Barrett <[email protected]>
A couple places where using asprintf and not paying attention to return
values was causing warnings.  Use opal_asprintf instead, which has known
behavior with the return pointer on failure and so we can safely ignore
return values.

Signed-off-by: Brian Barrett <[email protected]>
Don't use assert to check return values in this (not performance
critical) code, but always check and abort on failure.  Fixes
a warning about unread variable, but also is the right way to handle
error checking.

Signed-off-by: Brian Barrett <[email protected]>
We do pass NULL as a value for modifier (although
not when directive is NULL), which was causing
compiler warnings on modern GCC.  Handle the bozo
case to prevent said warnings.

Signed-off-by: Brian Barrett <[email protected]>
@bwbarrett bwbarrett force-pushed the cleanup/warnings-suck branch from dbdd6db to 03a2513 Compare January 21, 2022 18:52
@bwbarrett bwbarrett requested a review from gpaulsen January 21, 2022 21:11
Copy link
Member

@gpaulsen gpaulsen left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks.

@bwbarrett bwbarrett merged commit 053ce66 into open-mpi:master Jan 24, 2022
@bwbarrett bwbarrett deleted the cleanup/warnings-suck branch January 24, 2022 16:37
@awlauria awlauria mentioned this pull request Jan 27, 2022
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.

5 participants