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.