On Wed Apr 9, 2025 at 12:11 PM BST, Alejandro Vallejo wrote:
> On Wed Apr 9, 2025 at 7:48 AM BST, Jan Beulich wrote:
>> On 08.04.2025 18:07, Alejandro Vallejo wrote:
>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -978,10 +978,30 @@ static unsigned int __init copy_bios_e820(struct
>>> e820entry *map, unsigned int li
>>> return n;
>>> }
>>>
>>> -static struct domain *__init create_dom0(struct boot_info *bi)
>>> +static size_t __init domain_cmdline_size(
>>> + struct boot_info *bi, struct boot_domain *bd)
>>
>> const for both? And perhaps s/domain/dom0/ in the function name?
(missed this one)
Sure to the const pointers. But as the hyperlaunch effort progresses the
point is to turn all this into a more generic domain builders rather
than having dom0-specific stuff. Changing the name like that here to
adjust it in a few patches down the line doesn't seem worth the effort.
>> While this tidies the local variable, what about bd->cmdline? As it stands
>> this
>> gives the impression that you're freeing a pointer here which may still be
>> used
>> through passing bd elsewhere.
>
> That ought to have been bd->cmdline indeed.
>
Actually, it can't be. It's a "const char *", so XFREE() chokes on it.
I'll turn it into
XFREE(cmdline);
bd->cmdline = NULL;
instead.
Cheers,
Alejandro
