| Message ID | 20230923023334.41537-1-anisinha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/i386: changes towards enabling -Wshadow=local | expand |
On Sat, Sep 23, 2023 at 08:03:34AM +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> > --- > hw/i386/acpi-microvm.c | 12 ++++++------ > hw/i386/intel_iommu.c | 8 ++++---- > hw/i386/pc.c | 1 - > hw/i386/x86.c | 2 -- > 4 files changed, 10 insertions(+), 13 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> With regards, Daniel
On Sat, Sep 23, 2023 at 08:03:34AM +0530, Ani Sinha wrote: > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index c0ce896668..c1fb69170f 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -3770,9 +3770,9 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) > while (remain >= VTD_PAGE_SIZE) { > IOMMUTLBEvent event; > uint64_t mask = dma_aligned_pow2_mask(start, end, s->aw_bits); > - uint64_t size = mask + 1; > + uint64_t sz = mask + 1; > > - assert(size); > + assert(sz); > > event.type = IOMMU_NOTIFIER_UNMAP; > event.entry.iova = start; > @@ -3784,8 +3784,8 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) > > memory_region_notify_iommu_one(n, &event); > > - start += size; > - remain -= size; > + start += sz; > + remain -= sz; > } > > assert(!remain); Ani, I've got a small patch for this hunk already: https://lore.kernel.org/r/20230922160410.138786-1-peterx@redhat.com Wouldn't hurt to merge both, though.. or just drop the other one. Reviewed-by: Peter Xu <peterx@redhat.com> Thanks,
> On 25-Sep-2023, at 8:42 PM, Peter Xu <peterx@redhat.com> wrote: > > On Sat, Sep 23, 2023 at 08:03:34AM +0530, Ani Sinha wrote: >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index c0ce896668..c1fb69170f 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -3770,9 +3770,9 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) >> while (remain >= VTD_PAGE_SIZE) { >> IOMMUTLBEvent event; >> uint64_t mask = dma_aligned_pow2_mask(start, end, s->aw_bits); >> - uint64_t size = mask + 1; >> + uint64_t sz = mask + 1; >> >> - assert(size); >> + assert(sz); >> >> event.type = IOMMU_NOTIFIER_UNMAP; >> event.entry.iova = start; >> @@ -3784,8 +3784,8 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) >> >> memory_region_notify_iommu_one(n, &event); >> >> - start += size; >> - remain -= size; >> + start += sz; >> + remain -= sz; >> } >> >> assert(!remain); > > Ani, > > I've got a small patch for this hunk already: > > https://lore.kernel.org/r/20230922160410.138786-1-peterx@redhat.com > > Wouldn't hurt to merge both, though.. or just drop the other one. I liked your change so kept it and removed mine. See v2. > > Reviewed-by: Peter Xu <peterx@redhat.com> > > Thanks, > > -- > Peter Xu >
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c index a075360d85..6e4f8061eb 100644 --- a/hw/i386/acpi-microvm.c +++ b/hw/i386/acpi-microvm.c @@ -78,18 +78,18 @@ static void acpi_dsdt_add_virtio(Aml *scope, hwaddr base = VIRTIO_MMIO_BASE + index * 512; hwaddr size = 512; - Aml *dev = aml_device("VR%02u", (unsigned)index); - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); - aml_append(dev, aml_name_decl("_UID", aml_int(index))); - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); + Aml *adev = aml_device("VR%02u", (unsigned)index); + aml_append(adev, aml_name_decl("_HID", aml_string("LNRO0005"))); + aml_append(adev, aml_name_decl("_UID", aml_int(index))); + aml_append(adev, aml_name_decl("_CCA", aml_int(1))); Aml *crs = aml_resource_template(); aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, AML_EXCLUSIVE, &irq, 1)); - aml_append(dev, aml_name_decl("_CRS", crs)); - aml_append(scope, dev); + aml_append(adev, aml_name_decl("_CRS", crs)); + aml_append(scope, adev); } } } diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index c0ce896668..c1fb69170f 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -3770,9 +3770,9 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) while (remain >= VTD_PAGE_SIZE) { IOMMUTLBEvent event; uint64_t mask = dma_aligned_pow2_mask(start, end, s->aw_bits); - uint64_t size = mask + 1; + uint64_t sz = mask + 1; - assert(size); + assert(sz); event.type = IOMMU_NOTIFIER_UNMAP; event.entry.iova = start; @@ -3784,8 +3784,8 @@ static void vtd_address_space_unmap(VTDAddressSpace *as, IOMMUNotifier *n) memory_region_notify_iommu_one(n, &event); - start += size; - remain -= size; + start += sz; + remain -= sz; } assert(!remain); 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"
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> --- hw/i386/acpi-microvm.c | 12 ++++++------ hw/i386/intel_iommu.c | 8 ++++---- hw/i386/pc.c | 1 - hw/i386/x86.c | 2 -- 4 files changed, 10 insertions(+), 13 deletions(-)