All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "Daniel P. Smith" <dpsmith@apertussolutions.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	Michal Orzel <michal.orzel@amd.com>,
	<xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration
Date: Tue, 22 Jul 2025 14:37:08 +0200	[thread overview]
Message-ID: <DBIL7TSOVEXY.2SBQ063EVQ7GK@amd.com> (raw)
In-Reply-To: <e18e45f6-cffd-4bd9-9091-d2e5e979ffb1@suse.com>

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


  reply	other threads:[~2025-07-22 12:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-22 11:59 [PATCH 00/10] Extract bindings in dom0less code Alejandro Vallejo
2025-07-22 11:59 ` [PATCH 01/10] static-evtchn: Add missing include Alejandro Vallejo
2025-07-22 12:10   ` Jan Beulich
2025-07-22 12:41     ` Alejandro Vallejo
2025-07-22 23:22       ` Stefano Stabellini
2025-07-22 11:59 ` [PATCH 02/10] dt: Add cf_check to device-tree.c function pointers Alejandro Vallejo
2025-07-22 11:59 ` [PATCH 03/10] dom0less: Move asm-generic/dom0less-build.h -> xen/dom0less-build.h Alejandro Vallejo
2025-07-22 23:25   ` Stefano Stabellini
2025-07-22 11:59 ` [PATCH 04/10] bootfdt: Add createdomain arguments to boot_domain Alejandro Vallejo
2025-07-22 11:59 ` [PATCH 05/10] dom0less: Introduce kernel_info into the domain creation loop Alejandro Vallejo
2025-07-22 11:59 ` [PATCH 06/10] dom0less: Create llc_color_str field in boot_domain Alejandro Vallejo
2025-07-22 23:29   ` Stefano Stabellini
2025-07-22 11:59 ` [PATCH 07/10] dom0less: Turn arch_create_domUs into arch_parse_dom0less_node() Alejandro Vallejo
2025-07-22 23:33   ` Stefano Stabellini
2025-07-22 11:59 ` [PATCH 08/10] dom0less: Allow arch_parse_dom0less_node() to be fallible Alejandro Vallejo
2025-07-22 23:32   ` Stefano Stabellini
2025-07-22 11:59 ` [PATCH 09/10] dom0less: Reinitialise all variables on each loop iteration Alejandro Vallejo
2025-07-22 12:18   ` Jan Beulich
2025-07-22 12:37     ` Alejandro Vallejo [this message]
2025-07-22 12:57       ` Jan Beulich
2025-07-22 13:31         ` Alejandro Vallejo
2025-07-22 13:35           ` Jan Beulich
2025-07-22 23:36             ` Stefano Stabellini
2025-07-22 11:59 ` [PATCH 10/10] dom0less: Parse dom0less bindings into createdomain input args Alejandro Vallejo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBIL7TSOVEXY.2SBQ063EVQ7GK@amd.com \
    --to=alejandro.garciavallejo@amd.com \
    --cc=bertrand.marquis@arm.com \
    --cc=dpsmith@apertussolutions.com \
    --cc=jbeulich@suse.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.