Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.07.2025 14:37, Alejandro Vallejo wrote:
> On Tue Jul 22, 2025 at 2:18 PM CEST, Jan Beulich wrote:
>> On 22.07.2025 13:59, Alejandro Vallejo wrote:
>>> Reduce the scope of every variable so they are reinitialised. "iommu",
>>> for instance, isn't being cleared, so the wrong flags may make it to
>>> domains that should not have them.
>>
>> Yet "for instance" isn't quite right, is it? "iommu" is the only one where
>> the (re)init was misplaced. The other two ...
> 
> We do strive for minimal scope where possible. But you're right "for instance"
> might be misleading in suggesting there's more bugs than one.
> 
> I'm happy to have "for instance" removed, leaving the rest as-is, if that 
> works
> for you.

Except that "every" isn't quite right either. Nor is "they".

Jan

>>> --- a/xen/common/device-tree/dom0less-build.c
>>> +++ b/xen/common/device-tree/dom0less-build.c
>>> @@ -826,14 +826,14 @@ static int __init construct_domU(struct kernel_info 
>>> *kinfo,
>>>  void __init create_domUs(void)
>>>  {
>>>      struct dt_device_node *node;
>>> -    const char *dom0less_iommu;
>>> -    bool iommu = false;
>>> -    const struct dt_device_node *cpupool_node,
>>> -                                *chosen = dt_find_node_by_path("/chosen");
>>> +    const struct dt_device_node *chosen = dt_find_node_by_path("/chosen");
>>>  
>>>      BUG_ON(chosen == NULL);
>>>      dt_for_each_child_node(chosen, node)
>>>      {
>>> +        const char *dom0less_iommu;
>>> +        bool iommu = false;
>>> +        const struct dt_device_node *cpupool_node;
>>
>> ... had no initializer, and also don't gain any. So they must both be
>> set inside the loop. (Irrespective, the scope reduction is a good thing
>> imo.)
>>
>> Jan
> 
> Cheers,
> Alejandro