Re: [Qemu-devel] [PATCH] bitops: fix types

[Peter Maydell]

On 8 July 2012 20:51, Peter Maydell <address@hidden> wrote:
> On 8 July 2012 20:39, Blue Swirl <address@hidden> wrote:
>> On Sun, Jul 8, 2012 at 7:32 PM, Peter Maydell <address@hidden> wrote:
>>> On 8 July 2012 20:12, Blue Swirl <address@hidden> wrote:
>>>> Nice. Why is it written like that, I'd use
>>>> start + length <= 64?
>>>
>>> That would fail to handle the case of start == length == INT_MAX.
>>
>>  64 - INT_MAX = 0x80000040 (maybe off by one), which should still
>> trigger assert(INT_MAX <= 0x80000040).
>
> Nope, because "64 - start" here is signed arithmetic, so 64 - INT_MAX
> underflows and gives you a negative number, and INT_MAX is not <= -ve.
> (We went through this in reviews for the initial patch.)

Sorry, my reply is confused, because your email had a structure
implying that you are asserting that 'start + length <= 64' is a
working check, but the calculation you do is for 'length <= 64 - start',
and I ended up writing garbage as a result of my failure to parse it.

For clarity (all assuming signed int parameters as the code currently
uses, since this was a question about the current code):
 * "start + length <= 64" doesn't work because signed arithmetic means
the case of start = length = INT_MAX overflows and the condition
incorrectly doesn't trigger.
 * "length <= 64 - start" does work, because there is no underflow
or overflow, and the condition correctly triggers.
 assert(INT_MAX <= 0x80000040)
 * although "assert(INT_MAX <= 0x80000041)" will trigger if written
out literally, "assert(INT_MAX <= (64 - INT_MAX))" will not, even
though 64 - INT_MAX is 0x80000041. (the literal hex constant will
be an unsigned value, so we end up doing an unsigned comparison,
whereas 64 - INT_MAX is signed and we do a signed comparison).

Isn't C great?

-- PMM