[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] osdep: Fix ROUND_UP(64-bit, 32-bit)
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] osdep: Fix ROUND_UP(64-bit, 32-bit) |
Date: |
Thu, 14 Sep 2017 06:59:56 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
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: address@hidden
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> +#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
signature.asc
Description: OpenPGP digital signature