[U-Boot] [PATCH] Revert "part: Allocate only one legacy_mbr buffer"

[Faiz Abbas]

Hi Alexey,

On 04/09/19 6:27 PM, Alexey Brodkin wrote:
> Hi Faiz,
> 
> [snip]
> 
>>>>> I guess what you really want to do is to allocate buffer for "mbr"
>>>>> dynamically of size which is max(sizeof(legacy_mbr), dev_desc->blksz).
>>>>>
>>>>
>>>> With the assumption that blksz is always greater than
>>>> sizeof(legacy_mbr), this should work:
>>>>
>>>> ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, DIV_ROUND_UP(dev_desc->blksz,
>>>> sizeof(legacy_mbr)));
>>>
>>> No this won't work. See ALLOC_CACHE_ALIGN_BUFFER does static allocation
>>> in compile-time while blksz is set in runtime.
>>>
>>> That's why I mentioned switch to runtime allocation.
>>>
>>
>> Hmm. So the problem isn't as much about allocating too much in the
>> buffer but about using dynamically determined size for static array
>> declaration.
> 
> In fact it was a problem of huge static allocation I discovered just by
> chance building U-Boot for a very memory-limited device, see [1].
> 
>> This is being used all over this disk/part_dos.c file.
>> Shouldn't we fix all of that? Not sure how it has been working all along.
> 
> That part I don't quite understand. What's being used, where and how?
> And what's the problem with dynamic allocation of the buffer for MBR?
> 

Isn't the following line (being used in different functions in
disk/part_dos.c) also problematic because we don't know the value of
blksz at compile time?

ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);

Thanks,
Faiz