wintrust: Sign-compare warnings fix

Juan Lang juan.lang at gmail.com
Thu Dec 11 09:40:45 CST 2008


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



More information about the wine-devel mailing list