Re: [Xen-devel] [PATCH v5 3/7] xen/arm: keep track of reserved-memory regions

[Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>]

Julien Grall writes:

> Hi,
>
> On 8/13/19 4:14 PM, Volodymyr Babchuk wrote:
>> Julien Grall writes:
>>> On 8/13/19 3:23 PM, Volodymyr Babchuk wrote:
>>>> Stefano Stabellini writes:
>>>>
>>>>>        {
>>>>>            device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
>>>>>            if ( !size )
>>>>>                continue;
>>>>> -        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
>>>>> -        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
>>>>> -        bootinfo.mem.nr_banks++;
>>>>> +        mem->bank[mem->nr_banks].start = start;
>>>>> +        mem->bank[mem->nr_banks].size = size;
>>>>> +        mem->nr_banks++;
>>>>>        }
>>>>>
>>>>> -    if ( bootinfo.mem.nr_banks == NR_MEM_BANKS )
>>>>> +    if ( mem->nr_banks == NR_MEM_BANKS )
>>>> Looks like you have the same off-by-one error, as in previous patch.
>>>> I can see that it was there earlier. But it is good time to fix it.
>>>
>>> I don't think there was an off-by-one error before this series. So
>>> what do you mean?
>> I explained this in patch #2. Imagine that NR_MEM_BANKS = 1 and you have
>> one memory node in the dtb. You'll fill the first element of the array
>> and mem->nr_banks will become 1. This is absolutely normal. But check
>> above will fail, which is not right.
>
> Ok. So the off-by-one error has been introduced by this series. So
> this should be fixed in patch #2 not here.
Yes, sorry. I got lost in the code.

-- 
Volodymyr Babchuk at EPAM
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel