Re: [PATCH v3 03/16] x86/boot: add cmdline to struct boot_domain

[Alejandro Vallejo <agarciav@amd.com>]

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