cryptnet: Sign-compare warning fix (try 2)

[Andrew Talbot]

Juan Lang wrote:

> Hi Andy,
> 
> +            LONG last_error;
> 
>              ret = CryptGetObjectUrl(URL_OID_CERTIFICATE_CRL_DIST_POINT,
>               rgpvContext[i], 0, NULL, &cbUrlArray, NULL, NULL, NULL);
> -            if (!ret && GetLastError() == CRYPT_E_NOT_FOUND)
> +            last_error = GetLastError();
> +            if (!ret && last_error == CRYPT_E_NOT_FOUND)
> 
> I don't get it.  How does using a temporary LONG rather than
> GetLastError directly remove the warning?  Either the compiler was
> wrong about the first warning, or it's wrong about not warning about
> this form.  I don't see the utility of this patch.  Furthermore, any
> patches that remove warnings due to sign mismatches between
> GetLastError and an error in winerror.h reflect an error on
> Microsoft's part, not on Wine's, so I don't see the point of removing
> them.
> --Juan

Hi Juan,

An error code like CRYPT_E_NOT_FOUND is, in essence, a LONG (signed).
Whereas GetLastError() returns a DWORD (unsigned, which does seem
inappropriate to me). So the compiler is correct to warn about the original
code. In the case of the assignment to the temporary, the compiler
automatically coerces the right-hand side to suit the left. Let me attempt
to illustrate with the following code.

    signed char i, j, k;

    i = j + k;

In the above code, j and k would first be automatically promoted to intS
(integer promotion), then they would be added together, the result being an
int. Finally, this result would be coerced back into being a signed char
again: no cast required. (It's up to the programmer to avoid overflow).
Thus, one can avoid a cast by using an assignment.

Regarding whether there is a point in removing such errors: I suppose, if we
want to apply -Wsign-compare for the whole codebase and have zero warnings
during a build, then I don't think we have any choice.

-- 
Andy.