[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 02/10] hw: do not pass NULL to memory_region_
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v5 02/10] hw: do not pass NULL to memory_region_init from instance_init |
Date: |
Thu, 1 Oct 2015 11:49:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 01/10/15 10:59, Markus Armbruster wrote:
> From: Paolo Bonzini <address@hidden>
>
> This causes the region to outlive the object, because it attaches the
> region to /machine. This is not nice for the "realize" method, but
> much worse for "instance_init" because it can cause dangling pointers
> after a simple object_new/object_unref pair.
>
> Reported-by: Markus Armbruster <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> Message-Id: <address@hidden>
> Reviewed-by: Peter Maydell <address@hidden>
> Tested-by: Markus Armbruster <address@hidden>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> hw/arm/pxa2xx.c | 2 +-
> hw/display/cg3.c | 4 ++--
> hw/display/tcx.c | 2 +-
> hw/misc/arm_integrator_debug.c | 2 +-
> hw/misc/macio/cuda.c | 2 +-
> hw/misc/macio/macio.c | 6 +++---
> hw/pcmcia/pxa2xx.c | 6 +++---
> 7 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
> index 164260a..79d22d9 100644
> --- a/hw/arm/pxa2xx.c
> +++ b/hw/arm/pxa2xx.c
> @@ -1958,7 +1958,7 @@ static void pxa2xx_fir_instance_init(Object *obj)
> PXA2xxFIrState *s = PXA2XX_FIR(obj);
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> - memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s,
> + memory_region_init_io(&s->iomem, obj, &pxa2xx_fir_ops, s,
> "pxa2xx-fir", 0x1000);
> sysbus_init_mmio(sbd, &s->iomem);
> sysbus_init_irq(sbd, &s->irq);
> diff --git a/hw/display/cg3.c b/hw/display/cg3.c
> index d2a0d97..e309fbe 100644
> --- a/hw/display/cg3.c
> +++ b/hw/display/cg3.c
> @@ -280,12 +280,12 @@ static void cg3_initfn(Object *obj)
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> CG3State *s = CG3(obj);
>
> - memory_region_init_ram(&s->rom, NULL, "cg3.prom", FCODE_MAX_ROM_SIZE,
> + memory_region_init_ram(&s->rom, obj, "cg3.prom", FCODE_MAX_ROM_SIZE,
> &error_fatal);
> memory_region_set_readonly(&s->rom, true);
> sysbus_init_mmio(sbd, &s->rom);
>
> - memory_region_init_io(&s->reg, NULL, &cg3_reg_ops, s, "cg3.reg",
> + memory_region_init_io(&s->reg, obj, &cg3_reg_ops, s, "cg3.reg",
> CG3_REG_SIZE);
> sysbus_init_mmio(sbd, &s->reg);
> }
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 4635800..bf119bc 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -944,7 +944,7 @@ static void tcx_initfn(Object *obj)
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> TCXState *s = TCX(obj);
>
> - memory_region_init_ram(&s->rom, NULL, "tcx.prom", FCODE_MAX_ROM_SIZE,
> + memory_region_init_ram(&s->rom, OBJECT(s), "tcx.prom",
> FCODE_MAX_ROM_SIZE,
> &error_fatal);
> memory_region_set_readonly(&s->rom, true);
> sysbus_init_mmio(sbd, &s->rom);
I'd still use "obj" instead of "OBJECT(s)", even if the rest of the
function is doing it differently ... but I guess we can also clean that
up later ...
So anyway, patch looks fine to me:
Reviewed-by: Thomas Huth <address@hidden>
- [Qemu-devel] [PATCH v5 00/10] Fix device introspection regressions, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 09/10] qdev: Protect device-list-properties against broken devices, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 10/10] Revert "qdev: Use qdev_get_device_class() for -device <type>, help", Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 01/10] memory: allow destroying a non-empty MemoryRegion, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 03/10] macio: move DBDMA_init from instance_init to realize, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 02/10] hw: do not pass NULL to memory_region_init from instance_init, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v5 02/10] hw: do not pass NULL to memory_region_init from instance_init,
Thomas Huth <=
- [Qemu-devel] [PATCH v5 05/10] libqtest: Clean up unused QTestState member sigact_old, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 07/10] device-introspect-test: New, covering device introspection, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 06/10] libqtest: New hmp() & friends, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 04/10] tests: Fix how qom-test is run, Markus Armbruster, 2015/10/08
- [Qemu-devel] [PATCH v5 08/10] qmp: Fix device-list-properties not to crash for abstract device, Markus Armbruster, 2015/10/08