[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH] mmap-alloc: check before use for ptr pointer
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-trivial] [PATCH] mmap-alloc: check before use for ptr pointer |
Date: |
Wed, 12 Oct 2016 09:40:34 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 12/10/2016 09:37, Gonglei (Arei) wrote:
>> -----Original Message-----
>> From: Michael Tokarev [mailto:address@hidden
>> Sent: Wednesday, October 12, 2016 2:46 PM
>> Subject: Re: [PATCH] mmap-alloc: check before use for ptr pointer
>>
>> 12.10.2016 05:05, Gonglei wrote:
>>> If ptr mmap failed, we don't need to do a superfluous
>>> calculation for offset variable by ptr (MAP_FAILED).
>>
>> What's the point? There's no problem in extra calculation
>> if mmap failed, yes, but do we really care? As of it is now,
>> it is more compact and readable, and works. I think.
>>
>
> I just think it's more reasonable because the ptr is checked after
> used. Why do we need a extra calculation?
Another reasonable objection (that however your patch doesn't fix) is
that align is being used before the assertion:
assert(!(align & (align - 1)));
Paolo
>
> Regards,
> -Gonglei
>
>> Thanks,
>>
>> /mjt
>>
>>> Signed-off-by: Gonglei <address@hidden>
>>> ---
>>> util/mmap-alloc.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
>>> index 5a85aa3..577862b 100644
>>> --- a/util/mmap-alloc.c
>>> +++ b/util/mmap-alloc.c
>>> @@ -61,13 +61,15 @@ void *qemu_ram_mmap(int fd, size_t size, size_t
>> align, bool shared)
>>> #else
>>> void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS |
>> MAP_PRIVATE, -1, 0);
>>> #endif
>>> - size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
>>> + size_t offset;
>>> void *ptr1;
>>>
>>> if (ptr == MAP_FAILED) {
>>> return MAP_FAILED;
>>> }
>>>
>>> + offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
>>> +
>>> /* Make sure align is a power of 2 */
>>> assert(!(align & (align - 1)));
>>> /* Always align to host page size */
>>>
>
>
>