While doing a verification pass over code that is safety-critical - and is supposed to be executed in an embedded device - I saw some warnings emitted by both GCC and Clang. The warnings are easy to reproduce, since the code in question comes from the open-source BSD libc:
$ wget https://raw.githubusercontent.com/freebsd/\
freebsd/af3e10e5a78d3af8cef6088748978c6c612757f0/lib/libc/stdlib/qsort.c
$ gcc -c -Wall -Wsign-compare -DI_AM_QSORT_R -Wall qsort.c
qsort.c: In function 'qsort_r':
qsort.c:45:24: warning: comparison between signed and unsigned
integer expressions [-Wsign-compare]
#define MIN(a, b) ((a) < (b) ? a : b)
^
qsort.c:186:6: note: in expansion of macro 'MIN'
r = MIN(pd - pc, pn - pd - es);
^
qsort.c:45:34: warning: signed and unsigned type in conditional
expression [-Wsign-compare]
#define MIN(a, b) ((a) < (b) ? a : b)
^
qsort.c:186:6: note: in expansion of macro 'MIN'
r = MIN(pd - pc, pn - pd - es);
To understand these warnings (emitted from both GCC and CLang)...
pd
, pc
and pn
are pointerses
is size_t
(that is, unsigned
)It could be argued that C's rules for handling comparisons between signed and unsigned entities can be adequately expressed as DON'T, FOR THE LOVE OF GOD [1].
But in this case, the BSD implementation of qsort
compares...
ptrdiff_t
)es
- which is unsigned.Why does subtracting an unsigned
value from a ptrdiff_t
one (that is, a signed one) result in an unsigned
one? That's something you can read about in the post referenced above. Suffice to say, that for word-sized entities, any expression of signed OPERATOR unsigned
results in an unsigned
type.
So, to cut a long story short, for GCC and CLang to stop complaining, the lines would have to be changed from...
r = MIN(pd - pc, pn - pd - es);
to either:
r = MIN((unsigned)(pd - pc), pn - pd - es)
or to:
r = MIN(pd - pc, (signed)(pn - pd - es));
The question is... what is the right patch?
[1] "Why Not Mix Signed and Unsigned Values in C/C++?", http://blog.regehr.org/archives/268
Clearly I am on my own here :-)
My answer to the question comes from a very simplistic way of looking at things...
If you apply the unsigned cast on the left, and re-compile, the object file does not change (verified via MD5 sums). If you instead cast the right side (to signed), the object files DOES change - which means this change deviates (at assembly level, at least - for both amd64 and sparcs) from what the original code was doing.
Short answer: I proceed with casting to unsigned (size_t
). That's also what Apple seems to have done (thanks for the pointer, @alk).