Fix a bunch of compiler warnings in "make check"

[jsquyres]

There were many compiler warnings, making it difficult to read if
there were real issues that needed to be addressed.

Signed-off-by: Jeff Squyres jsquyres@cisco.com

[jjhursey]

IBM CI had a bad weekend (sorry about that). We should be all good now.
bot:ibm:pgi:retest
bot:ibm:xl:retest

[bosilca]

I doubt it is necessary, the shadowing is done in purpose.

[jsquyres]

The shadowing creates a bunch of warnings, such as:

1122 reduce_local.c: In function ‘build_do_ops’:
1123 reduce_local.c:84:34: warning: declaration of ‘do_ops’ shadows a global decla\
     ration [-Wshadow]
1124  build_do_ops( char* optarg, int* do_ops)
1125                              ~~~~~^~~~~~

[bosilca]

I understand what the warning is but I said I do this on purpose. By default all functions use the global variable until I need to specialize their behavior, in which case I add a param shadowing the global variable.

[bwbarrett]

I'm not a huge fan of that paradigm, as I think it's pretty easy for developers to miss and or get wrong. I certainly wouldn't want any of the developers in my org to write code like that. That said, if we are going to intentionally shadow variables, can we at least document that we're doing so intentionally? I still think it is bad form, and that we should not remove the general compiler warning, so we'll have to figure out how to ignore the warning in this file), but we at least need to tell people it was intentional.

[bosilca]

displ is a ptrdiff_t already because of a compiler complaint, (because it is computed as a difference between two pointers, while in fact is the displacement of a field in a struct).

[jsquyres]

Here's the compiler warnings it generated without updating the RHS to be (ptrdiff_t):

1024 unpack_ooo.c: In function ‘testcase’:
1025 unpack_ooo.c:154:32: warning: comparison of integer expressions of different \
     signedness: ‘ptrdiff_t’ {aka ‘long int’} and ‘size_t’ {aka ‘long unsigned int\
     ’} [-Wsign-compare]
1026                      if( (displ >= arr[i][1]) && (displ <= (arr[i][1] + arr[i\
     ][0])) ) {
1027                                 ^~
1028 unpack_ooo.c:154:56: warning: comparison of integer expressions of different \
     signedness: ‘ptrdiff_t’ {aka ‘long int’} and ‘size_t’ {aka ‘long unsigned int\
     ’} [-Wsign-compare]
1029                      if( (displ >= arr[i][1]) && (displ <= (arr[i][1] + arr[i\
     ][0])) ) {
1030                                                         ^~

I don't have an opinion on this one; do you prefer a different fix?

[bosilca]

Based on a machine understanding of a piece of code. I don't really care.

[gpaulsen]

@jsquyres Can you please rebase? This may be important.

[gpaulsen]

bot:retest
/azp run
bot:ibm:retest

[gpaulsen]

bot:ibm:retest

[jjhursey]

Any reason to hold this PR any longer? (other than it need to be rebased)

[gpaulsen]

I rebased on tip of main(no code change).

[jsquyres]

If that 3rd commit passes CI properly, you'll squash it appropriately, right? (minor pet peeve: make a commit on a PR and then make a later commit on the same PR to fix the earlier commit)

[bwbarrett]

What's the warning? The non-sized version is supposed to handle multiple sizes without warning, so if we're emitting warnings here, that's an issue we shouldn't gloss over with this particular patch.

[gpaulsen]

mellanox CI failed that it couldn't find a definition for the non-sized versions of these, and I searched around and couldn't find those definitions either, so I created the 3rd commit.
Why the other CIs didn't fail, I'm not sure.
@bwbarrett thoughts?

[gpaulsen]

I git bisected opal/include/opal/sys/atomic_stdc.h and couldn't find a definition for opal_atomic_compare_exchange_strong anywhere in the past.

[bwbarrett]

bot:aws:retest