Re: [Qemu-devel] [PATCH] osdep: Fix ROUND_UP(64-bit, 32-bit)

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]

On 09/14/2017 03:44 AM, Laszlo Ersek wrote:
> On 09/13/17 23:03, Eric Blake wrote:
>> When using bit-wise operations that exploit the power-of-two
>> nature of the second argument of ROUND_UP(), we still need to
>> ensure that the mask is as wide as the first argument (done
>> by using addition of 0 to force proper arithmetic promotion).
>> Unpatched, ROUND_UP(2ULL*1024*1024*1024*1024, 512) produces 0,
>> instead of the intended 2TiB.
>>
>> Broken since its introduction in commit 292c8e50 (v1.5.0).
>>
>> CC: qemu-trivial@nongnu.org
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>

>> +#define ROUND_UP(n, d) (((n) + (d) - 1) & -((n) - (n) + (d)))
>>  #endif
> 
> Another way to widen the mask as necessary would be:
> 
>   (((n) + (d) - 1) & -(0 ? (n) : (d)))

Oh, I like that even better!


>>  #ifndef DIV_ROUND_UP
>> -#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
>> +#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
>>  #endif
> 
> This looks like an independent whitespace fix; should it be in this patch?

checkpatch complained about the pre-patch spacing in ROUND_UP, and
DIV_ROUND_UP had the same issue 2 lines later.  But you're right that
it's not strictly necessary (or that I should at least call it out in
the commit message).

v2 coming up.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]