On 12/04/2024 12:41 pm, Daniel P. Smith wrote:
> On 4/12/24 07:34, Daniel P. Smith wrote:
>> On 4/11/24 15:24, Andrew Cooper wrote:
>>> On 11/04/2024 4:25 pm, Daniel P. Smith wrote:
>>>> diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
>>>> index 1bcb007395ba..9b4891731b8b 100644
>>>> --- a/xen/common/gzip/gunzip.c
>>>> +++ b/xen/common/gzip/gunzip.c
>>>> @@ -102,12 +109,13 @@ __init int gzip_check(char *image, unsigned
>>>> long image_len)
>>>> __init int perform_gunzip(char *output, char *image, unsigned
>>>> long image_len)
>>>> {
>>>> + struct gzip_data gd;
>>>> int rc;
>>>
>>> By the end of this series,
>>>
>>> Reading symbols from xen-syms...
>>> (gdb) p sizeof(struct gzip_data)
>>> $1 = 2120
>>>
>>> x86 has an 8k stack and this takes 1/4 of it. Other bits of state are
>>> dynamically allocated, even in inflate.c, so I'd highly recommend doing
>>> the same for this.
>>
>> I take it you are mainly talking about crc_32_tab? Yes, I can switch
>> that to being dynamically allocated.
>
> Never mind, reading your comment on patch4 made me realize you wanted
> the instance of struct dynamically allocated. Though the answer is
> still, yes, we can dynamically allocate it.
I wrote this before realising that crc_32_tag could be shrunk.
If it's only1k on the stack, then that's a whole lot less bad, and is
perhaps ok. I guess it depends on the stack size of the other
architectures.
Still - I expect dynamically allocating would be a safer course of
action. Internal blocks are dynamically allocated already, so this is
"just" one more.
~Andrew
