wintrust: Sign-compare warnings fix
Andrew.Talbot at talbotville.com
Thu Dec 11 01:35:12 CST 2008
Juan Lang wrote:
> Hi Andy,
> - if (pbEncoded + 1 > cbEncoded)
> + if (pbEncoded + 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.
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
Thanks for the feedback.
More information about the wine-devel