wintrust: Sign-compare warnings fix

Andrew Talbot Andrew.Talbot at talbotville.com
Thu Dec 11 01:35:12 CST 2008


Juan Lang wrote:

> Hi Andy,
> 
> -        if (pbEncoded[1] + 1 > cbEncoded)
> +        if (pbEncoded[1] + 1U > cbEncoded)
> 
> Is this change necessary?  The resulting code is less clear than the
> original, IMO.  It's clearly a spurious warning:  a BYTE (max value
> 255) + 1 can't yield a value that overflows an unsigned int, so this
> comparison will always do what's wanted.
> 
> Same with the change here:
> -        else if (lenLen + 2 > cbEncoded)
> +        else if (lenLen + 2U > cbEncoded)
> 
> Otherwise, this patch looks fine to me.
> --Juan

Hi Juan,

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)
    {
        ...

Maybe, in this case the code actually is "wrong", since a variable that
operates in the range 0 to UINT_MAX (on a non-MSC system) can never be
strictly equal to -1, but in these cases I have resisted comparing against
"~0U", because there may be places where an internal function could/should
be rewritten not to use "minus one unsigned" as a return value, for example,
and this "fix" would tend to hide those opportunities.

I shall see how the patch fares and resubmit without the "U"s if it gets
rejected.

Thanks for the feedback.

-- 
Andy.





More information about the wine-devel mailing list