[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 10:19:19 +0200 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 |
On 12/10/2016 09:54, Gonglei (Arei) wrote:
>
>> -----Original Message-----
>> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo
>> Bonzini
>> Sent: Wednesday, October 12, 2016 3:41 PM
>> To: Gonglei (Arei); Michael Tokarev; address@hidden
>> Cc: address@hidden; Herongguang (Stephen)
>> Subject: Re: [PATCH] mmap-alloc: check before use for ptr pointer
>>
>>
>>
>> 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)));
>>
>
> Eh, align is used at the beginning of the qemu_ram_mmap() ;)
Yes, but the assertion is there because align is passed to QEMU_ALIGN_UP.
Paolo