From: "Michael S. Tsirkin" <mst@redhat.com>
To: Ani Sinha <anisinha@redhat.com>
Cc: "Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Richard Henderson" <richard.henderson@linaro.org>,
"Eduardo Habkost" <eduardo@habkost.net>,
"Peter Xu" <peterx@redhat.com>,
"Jason Wang" <jasowang@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Philippe Mathieu-Daude" <philmd@linaro.org>,
"Daniel P . Berrangé" <berrange@redhat.com>,
qemu-devel@nongnu.org
Subject: Re: [PATCH v2] hw/i386: changes towards enabling -Wshadow=local
Date: Tue, 3 Oct 2023 08:28:47 -0400 [thread overview]
Message-ID: <20231003082821-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20231003054306.4372-1-anisinha@redhat.com>
On Tue, Oct 03, 2023 at 11:13:06AM +0530, Ani Sinha wrote:
> Code changes that addresses all compiler complaints coming from enabling
> -Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
> other local variables or parameters. These makes the code confusing and/or adds
> bugs that are difficult to catch.
>
> CC: Markus Armbruster <armbru@redhat.com>
> CC: Philippe Mathieu-Daude <philmd@linaro.org>
> CC: mst@redhat.com
> Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
feel free to move with the rest of these changes.
> ---
> hw/i386/acpi-microvm.c | 4 ++--
> hw/i386/intel_iommu.c | 8 ++++----
> hw/i386/pc.c | 1 -
> hw/i386/x86.c | 2 --
> 4 files changed, 6 insertions(+), 9 deletions(-)
>
> changelog:
> v2: addressed suggestion from mst.
>
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index a075360d85..6ddcfb0419 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -55,8 +55,8 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>
> bus = sysbus_get_default();
> QTAILQ_FOREACH(kid, &bus->children, sibling) {
> - DeviceState *dev = kid->child;
> - Object *obj = object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MMIO);
> + Object *obj = object_dynamic_cast(OBJECT(kid->child),
> + TYPE_VIRTIO_MMIO);
>
> if (obj) {
> VirtIOMMIOProxy *mmio = VIRTIO_MMIO(obj);
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index c0ce896668..2c832ab68b 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -3744,7 +3744,7 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
> /* Unmap the whole range in the notifier's scope. */
> static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
> {
> - hwaddr size, remain;
> + hwaddr total, remain;
> hwaddr start = n->start;
> hwaddr end = n->end;
> IntelIOMMUState *s = as->iommu_state;
> @@ -3765,7 +3765,7 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
> }
>
> assert(start <= end);
> - size = remain = end - start + 1;
> + total = remain = end - start + 1;
>
> while (remain >= VTD_PAGE_SIZE) {
> IOMMUTLBEvent event;
> @@ -3793,10 +3793,10 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n)
> trace_vtd_as_unmap_whole(pci_bus_num(as->bus),
> VTD_PCI_SLOT(as->devfn),
> VTD_PCI_FUNC(as->devfn),
> - n->start, size);
> + n->start, total);
>
> map.iova = n->start;
> - map.size = size - 1; /* Inclusive */
> + map.size = total - 1; /* Inclusive */
> iova_tree_remove(as->iova_tree, map);
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3db0743f31..e7a233e886 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1116,7 +1116,6 @@ void pc_memory_init(PCMachineState *pcms,
>
> if (machine->device_memory) {
> uint64_t *val = g_malloc(sizeof(*val));
> - PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> uint64_t res_mem_end = machine->device_memory->base;
>
> if (!pcmc->broken_reserved_end) {
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index f034df8bf6..b3d054889b 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -365,8 +365,6 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
>
> cpu_slot = x86_find_cpu_slot(MACHINE(x86ms), cpu->apic_id, &idx);
> if (!cpu_slot) {
> - MachineState *ms = MACHINE(x86ms);
> -
> x86_topo_ids_from_apicid(cpu->apic_id, &topo_info, &topo_ids);
> error_setg(errp,
> "Invalid CPU [socket: %u, die: %u, core: %u, thread: %u] with"
> --
> 2.42.0
next prev parent reply other threads:[~2023-10-03 12:29 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-03 5:43 [PATCH v2] hw/i386: changes towards enabling -Wshadow=local Ani Sinha
2023-10-03 9:05 ` Ani Sinha
2023-10-03 12:28 ` Michael S. Tsirkin [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-09-26 5:52 Ani Sinha
2023-09-27 15:25 ` Michael S. Tsirkin
2023-09-28 3:44 ` Ani Sinha
2023-10-02 10:47 ` Michael S. Tsirkin
2023-10-03 5:51 ` Ani Sinha
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=20231003082821-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=anisinha@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=eduardo@habkost.net \
--cc=jasowang@redhat.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.com \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).