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
