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

[Alejandro Vallejo <alejandro.garciavallejo@xxxxxxx>]

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.

>
>> --- 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