Hi Andy,
I was curious to see how this one would fly. I fully take your point, of course. If it were a good idea, the point would be to reduce the noise when looking for real sign-compare problems and without introducing a cast. In a similar vein, quite a lot of warnings are generated by code like this
if (dwSomething == -1) {
Often in these cases, there's some predefined constant that should be used in lieu of -1. You introduced such a change in the same patch: - if (SetFilePointer(pSubjectInfo->hFile, 0, NULL, SEEK_END) == -1) + if (SetFilePointer(pSubjectInfo->hFile, 0, NULL, SEEK_END) == INVALID_SET_FILE_POINTER)
This sort of change makes the code better, in my opinion, because the return type is being compared against something defined in the interface's documentation.
The case I objected to is a curious one. I had a look at K&R's type promotion rules (2nd edition, section A6.5) and I'm confused what the compiler is doing here. The if-block is:
if (pbEncoded[1] + 1 > cbEncoded)
Rewriting the parenthesized expression as types rather than variables/values yields:
unsigned char + int > unsigned int
According to K&R, the unsigned char should get promoted to unsigned int (I think), hence:
unsigned int + int > unsigned int
When an unsigned type and a signed type are together in an expression, the signed type is promoted to unsigned:
unsigned int + unsigned int > unsigned int
Thus the comparison shouldn't have a signed/unsigned mismatch.
The promotion of unsigned char to signed/unsigned int is platform-dependent, so perhaps gcc's analysis assumes it's signed? In any case, by my reading the gcc warning is simply wrong. I could be wrong of course.
Anyway, good luck with your warning-crushing war ;-) --Juan