Message ID | 1397828484-21771-5-git-send-email-batuzovk@ispras.ru |
---|---|
State | New |
Headers | show |
Am 18.04.2014 15:41, schrieb Kirill Batuzov: > PortioList is an abstraction used for construction of MemoryRegionPortioList > from MemoryRegionPortio. It is not needed later, so there is no need to > allocate it dynamically. Also portio_list_destroy should be called to free > memory allocated in portio_list_init. > > This change spans several target platforms. The following testcases cover all > changed lines: > qemu-system-ppc -M prep > qemu-system-i386 -vga qxl > qemu-system-i386 -M isapc -soundhw adlib -device ib700,id=watchdog0,bus=isa.0 > > Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru> Your changes look correct, but I'm not sure about our future plans with this API. CC'ing some people to help review this. Regards, Andreas > --- > hw/audio/adlib.c | 7 ++++--- > hw/display/qxl.c | 9 +++++---- > hw/display/vga.c | 16 +++++++++------- > hw/dma/i82374.c | 7 ++++--- > hw/isa/isa-bus.c | 7 ++++--- > hw/ppc/prep.c | 7 ++++--- > hw/watchdog/wdt_ib700.c | 7 ++++--- > 7 files changed, 34 insertions(+), 26 deletions(-) > > Coding style in adlib.c slightly differs from QEMU coding style. > Particularly there are spaces between function name and parenthesis. I decided > to not mix coding styles, but as a result checkpatch.pl complains on this > patch. > > diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c > index 28eed81..e43d990 100644 > --- a/hw/audio/adlib.c > +++ b/hw/audio/adlib.c > @@ -293,7 +293,7 @@ static MemoryRegionPortio adlib_portio_list[] = { > static void adlib_realizefn (DeviceState *dev, Error **errp) > { > AdlibState *s = ADLIB(dev); > - PortioList *port_list = g_new(PortioList, 1); > + PortioList port_list; > struct audsettings as; > > if (glob_adlib) { > @@ -349,8 +349,9 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) > > adlib_portio_list[0].offset = s->port; > adlib_portio_list[1].offset = s->port + 8; > - portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib"); > - portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0); > + portio_list_init (&port_list, OBJECT(s), adlib_portio_list, s, "adlib"); > + portio_list_add (&port_list, isa_address_space_io(&s->parent_obj), 0); > + portio_list_destroy (&port_list); > } > > static Property adlib_properties[] = { > diff --git a/hw/display/qxl.c b/hw/display/qxl.c > index 47bbf1f..7a361c7 100644 > --- a/hw/display/qxl.c > +++ b/hw/display/qxl.c > @@ -2055,7 +2055,7 @@ static int qxl_init_primary(PCIDevice *dev) > { > PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev); > VGACommonState *vga = &qxl->vga; > - PortioList *qxl_vga_port_list = g_new(PortioList, 1); > + PortioList qxl_vga_port_list; > int rc; > > qxl->id = 0; > @@ -2064,10 +2064,11 @@ static int qxl_init_primary(PCIDevice *dev) > vga_common_init(vga, OBJECT(dev)); > vga_init(vga, OBJECT(dev), > pci_address_space(dev), pci_address_space_io(dev), false); > - portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, > + portio_list_init(&qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, > vga, "vga"); > - portio_list_set_flush_coalesced(qxl_vga_port_list); > - portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > + portio_list_set_flush_coalesced(&qxl_vga_port_list); > + portio_list_add(&qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); > + portio_list_destroy(&qxl_vga_port_list); > > vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); > qemu_spice_display_init_common(&qxl->ssd); > diff --git a/hw/display/vga.c b/hw/display/vga.c > index 063319d..72feafd 100644 > --- a/hw/display/vga.c > +++ b/hw/display/vga.c > @@ -2351,8 +2351,8 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, > { > MemoryRegion *vga_io_memory; > const MemoryRegionPortio *vga_ports, *vbe_ports; > - PortioList *vga_port_list = g_new(PortioList, 1); > - PortioList *vbe_port_list = g_new(PortioList, 1); > + PortioList vga_port_list; > + PortioList vbe_port_list; > > qemu_register_reset(vga_reset, s); > > @@ -2367,13 +2367,15 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, > 1); > memory_region_set_coalescing(vga_io_memory); > if (init_vga_ports) { > - portio_list_init(vga_port_list, obj, vga_ports, s, "vga"); > - portio_list_set_flush_coalesced(vga_port_list); > - portio_list_add(vga_port_list, address_space_io, 0x3b0); > + portio_list_init(&vga_port_list, obj, vga_ports, s, "vga"); > + portio_list_set_flush_coalesced(&vga_port_list); > + portio_list_add(&vga_port_list, address_space_io, 0x3b0); > + portio_list_destroy(&vga_port_list); > } > if (vbe_ports) { > - portio_list_init(vbe_port_list, obj, vbe_ports, s, "vbe"); > - portio_list_add(vbe_port_list, address_space_io, 0x1ce); > + portio_list_init(&vbe_port_list, obj, vbe_ports, s, "vbe"); > + portio_list_add(&vbe_port_list, address_space_io, 0x1ce); > + portio_list_destroy(&vbe_port_list); > } > } > > diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c > index dc7a767..f27ce25 100644 > --- a/hw/dma/i82374.c > +++ b/hw/dma/i82374.c > @@ -137,11 +137,12 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp) > { > ISAi82374State *isa = I82374(dev); > I82374State *s = &isa->state; > - PortioList *port_list = g_new(PortioList, 1); > + PortioList port_list; > > - portio_list_init(port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); > - portio_list_add(port_list, isa_address_space_io(&isa->parent_obj), > + portio_list_init(&port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); > + portio_list_add(&port_list, isa_address_space_io(&isa->parent_obj), > isa->iobase); > + portio_list_destroy(&port_list); > > i82374_realize(s, errp); > > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c > index 55d0100..7b88ffc 100644 > --- a/hw/isa/isa-bus.c > +++ b/hw/isa/isa-bus.c > @@ -108,15 +108,16 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start, > const MemoryRegionPortio *pio_start, > void *opaque, const char *name) > { > - PortioList *piolist = g_new(PortioList, 1); > + PortioList piolist; > > /* START is how we should treat DEV, regardless of the actual > contents of the portio array. This is how the old code > actually handled e.g. the FDC device. */ > isa_init_ioport(dev, start); > > - portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name); > - portio_list_add(piolist, isabus->address_space_io, start); > + portio_list_init(&piolist, OBJECT(dev), pio_start, opaque, name); > + portio_list_add(&piolist, isabus->address_space_io, start); > + portio_list_destroy(&piolist); > } > > static void isa_device_init(Object *obj) > diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c > index e243651..505e053 100644 > --- a/hw/ppc/prep.c > +++ b/hw/ppc/prep.c > @@ -375,7 +375,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) > CPUPPCState *env = NULL; > nvram_t nvram; > M48t59State *m48t59; > - PortioList *port_list = g_new(PortioList, 1); > + PortioList port_list; > #if 0 > MemoryRegion *xcsr = g_new(MemoryRegion, 1); > #endif > @@ -542,8 +542,9 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) > cpu = POWERPC_CPU(first_cpu); > sysctrl->reset_irq = cpu->env.irq_inputs[PPC6xx_INPUT_HRESET]; > > - portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep"); > - portio_list_add(port_list, isa_address_space_io(isa), 0x0); > + portio_list_init(&port_list, NULL, prep_portio_list, sysctrl, "prep"); > + portio_list_add(&port_list, isa_address_space_io(isa), 0x0); > + portio_list_destroy(&port_list); > > /* PowerPC control and status register group */ > #if 0 > diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c > index bc994a4..fb1ca36 100644 > --- a/hw/watchdog/wdt_ib700.c > +++ b/hw/watchdog/wdt_ib700.c > @@ -106,14 +106,15 @@ static const MemoryRegionPortio wdt_portio_list[] = { > static void wdt_ib700_realize(DeviceState *dev, Error **errp) > { > IB700State *s = IB700(dev); > - PortioList *port_list = g_new(PortioList, 1); > + PortioList port_list; > > ib700_debug("watchdog init\n"); > > s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s); > > - portio_list_init(port_list, OBJECT(s), wdt_portio_list, s, "ib700"); > - portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0); > + portio_list_init(&port_list, OBJECT(s), wdt_portio_list, s, "ib700"); > + portio_list_add(&port_list, isa_address_space_io(&s->parent_obj), 0); > + portio_list_destroy(&port_list); > } > > static void wdt_ib700_reset(DeviceState *dev) >
Il 18/04/2014 12:36, Andreas Färber ha scritto: > Am 18.04.2014 15:41, schrieb Kirill Batuzov: >> PortioList is an abstraction used for construction of MemoryRegionPortioList >> from MemoryRegionPortio. It is not needed later, so there is no need to >> allocate it dynamically. Also portio_list_destroy should be called to free >> memory allocated in portio_list_init. >> >> This change spans several target platforms. The following testcases cover all >> changed lines: >> qemu-system-ppc -M prep >> qemu-system-i386 -vga qxl >> qemu-system-i386 -M isapc -soundhw adlib -device ib700,id=watchdog0,bus=isa.0 >> >> Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru> > > Your changes look correct, but I'm not sure about our future plans with > this API. CC'ing some people to help review this. Indeed, it looks strange to have a portio_list_destroy without a portio_list_del before. You can add the PortioLists to AdlibState, PCIQXLDevice etc. instead. Paolo > Regards, > Andreas > >> --- >> hw/audio/adlib.c | 7 ++++--- >> hw/display/qxl.c | 9 +++++---- >> hw/display/vga.c | 16 +++++++++------- >> hw/dma/i82374.c | 7 ++++--- >> hw/isa/isa-bus.c | 7 ++++--- >> hw/ppc/prep.c | 7 ++++--- >> hw/watchdog/wdt_ib700.c | 7 ++++--- >> 7 files changed, 34 insertions(+), 26 deletions(-) >> >> Coding style in adlib.c slightly differs from QEMU coding style. >> Particularly there are spaces between function name and parenthesis. I decided >> to not mix coding styles, but as a result checkpatch.pl complains on this >> patch. >> >> diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c >> index 28eed81..e43d990 100644 >> --- a/hw/audio/adlib.c >> +++ b/hw/audio/adlib.c >> @@ -293,7 +293,7 @@ static MemoryRegionPortio adlib_portio_list[] = { >> static void adlib_realizefn (DeviceState *dev, Error **errp) >> { >> AdlibState *s = ADLIB(dev); >> - PortioList *port_list = g_new(PortioList, 1); >> + PortioList port_list; >> struct audsettings as; >> >> if (glob_adlib) { >> @@ -349,8 +349,9 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) >> >> adlib_portio_list[0].offset = s->port; >> adlib_portio_list[1].offset = s->port + 8; >> - portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib"); >> - portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0); >> + portio_list_init (&port_list, OBJECT(s), adlib_portio_list, s, "adlib"); >> + portio_list_add (&port_list, isa_address_space_io(&s->parent_obj), 0); >> + portio_list_destroy (&port_list); >> } >> >> static Property adlib_properties[] = { >> diff --git a/hw/display/qxl.c b/hw/display/qxl.c >> index 47bbf1f..7a361c7 100644 >> --- a/hw/display/qxl.c >> +++ b/hw/display/qxl.c >> @@ -2055,7 +2055,7 @@ static int qxl_init_primary(PCIDevice *dev) >> { >> PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev); >> VGACommonState *vga = &qxl->vga; >> - PortioList *qxl_vga_port_list = g_new(PortioList, 1); >> + PortioList qxl_vga_port_list; >> int rc; >> >> qxl->id = 0; >> @@ -2064,10 +2064,11 @@ static int qxl_init_primary(PCIDevice *dev) >> vga_common_init(vga, OBJECT(dev)); >> vga_init(vga, OBJECT(dev), >> pci_address_space(dev), pci_address_space_io(dev), false); >> - portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, >> + portio_list_init(&qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, >> vga, "vga"); >> - portio_list_set_flush_coalesced(qxl_vga_port_list); >> - portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); >> + portio_list_set_flush_coalesced(&qxl_vga_port_list); >> + portio_list_add(&qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); >> + portio_list_destroy(&qxl_vga_port_list); >> >> vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); >> qemu_spice_display_init_common(&qxl->ssd); >> diff --git a/hw/display/vga.c b/hw/display/vga.c >> index 063319d..72feafd 100644 >> --- a/hw/display/vga.c >> +++ b/hw/display/vga.c >> @@ -2351,8 +2351,8 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, >> { >> MemoryRegion *vga_io_memory; >> const MemoryRegionPortio *vga_ports, *vbe_ports; >> - PortioList *vga_port_list = g_new(PortioList, 1); >> - PortioList *vbe_port_list = g_new(PortioList, 1); >> + PortioList vga_port_list; >> + PortioList vbe_port_list; >> >> qemu_register_reset(vga_reset, s); >> >> @@ -2367,13 +2367,15 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, >> 1); >> memory_region_set_coalescing(vga_io_memory); >> if (init_vga_ports) { >> - portio_list_init(vga_port_list, obj, vga_ports, s, "vga"); >> - portio_list_set_flush_coalesced(vga_port_list); >> - portio_list_add(vga_port_list, address_space_io, 0x3b0); >> + portio_list_init(&vga_port_list, obj, vga_ports, s, "vga"); >> + portio_list_set_flush_coalesced(&vga_port_list); >> + portio_list_add(&vga_port_list, address_space_io, 0x3b0); >> + portio_list_destroy(&vga_port_list); >> } >> if (vbe_ports) { >> - portio_list_init(vbe_port_list, obj, vbe_ports, s, "vbe"); >> - portio_list_add(vbe_port_list, address_space_io, 0x1ce); >> + portio_list_init(&vbe_port_list, obj, vbe_ports, s, "vbe"); >> + portio_list_add(&vbe_port_list, address_space_io, 0x1ce); >> + portio_list_destroy(&vbe_port_list); >> } >> } >> >> diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c >> index dc7a767..f27ce25 100644 >> --- a/hw/dma/i82374.c >> +++ b/hw/dma/i82374.c >> @@ -137,11 +137,12 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp) >> { >> ISAi82374State *isa = I82374(dev); >> I82374State *s = &isa->state; >> - PortioList *port_list = g_new(PortioList, 1); >> + PortioList port_list; >> >> - portio_list_init(port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); >> - portio_list_add(port_list, isa_address_space_io(&isa->parent_obj), >> + portio_list_init(&port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); >> + portio_list_add(&port_list, isa_address_space_io(&isa->parent_obj), >> isa->iobase); >> + portio_list_destroy(&port_list); >> >> i82374_realize(s, errp); >> >> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c >> index 55d0100..7b88ffc 100644 >> --- a/hw/isa/isa-bus.c >> +++ b/hw/isa/isa-bus.c >> @@ -108,15 +108,16 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start, >> const MemoryRegionPortio *pio_start, >> void *opaque, const char *name) >> { >> - PortioList *piolist = g_new(PortioList, 1); >> + PortioList piolist; >> >> /* START is how we should treat DEV, regardless of the actual >> contents of the portio array. This is how the old code >> actually handled e.g. the FDC device. */ >> isa_init_ioport(dev, start); >> >> - portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name); >> - portio_list_add(piolist, isabus->address_space_io, start); >> + portio_list_init(&piolist, OBJECT(dev), pio_start, opaque, name); >> + portio_list_add(&piolist, isabus->address_space_io, start); >> + portio_list_destroy(&piolist); >> } >> >> static void isa_device_init(Object *obj) >> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c >> index e243651..505e053 100644 >> --- a/hw/ppc/prep.c >> +++ b/hw/ppc/prep.c >> @@ -375,7 +375,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) >> CPUPPCState *env = NULL; >> nvram_t nvram; >> M48t59State *m48t59; >> - PortioList *port_list = g_new(PortioList, 1); >> + PortioList port_list; >> #if 0 >> MemoryRegion *xcsr = g_new(MemoryRegion, 1); >> #endif >> @@ -542,8 +542,9 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) >> cpu = POWERPC_CPU(first_cpu); >> sysctrl->reset_irq = cpu->env.irq_inputs[PPC6xx_INPUT_HRESET]; >> >> - portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep"); >> - portio_list_add(port_list, isa_address_space_io(isa), 0x0); >> + portio_list_init(&port_list, NULL, prep_portio_list, sysctrl, "prep"); >> + portio_list_add(&port_list, isa_address_space_io(isa), 0x0); >> + portio_list_destroy(&port_list); >> >> /* PowerPC control and status register group */ >> #if 0 >> diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c >> index bc994a4..fb1ca36 100644 >> --- a/hw/watchdog/wdt_ib700.c >> +++ b/hw/watchdog/wdt_ib700.c >> @@ -106,14 +106,15 @@ static const MemoryRegionPortio wdt_portio_list[] = { >> static void wdt_ib700_realize(DeviceState *dev, Error **errp) >> { >> IB700State *s = IB700(dev); >> - PortioList *port_list = g_new(PortioList, 1); >> + PortioList port_list; >> >> ib700_debug("watchdog init\n"); >> >> s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s); >> >> - portio_list_init(port_list, OBJECT(s), wdt_portio_list, s, "ib700"); >> - portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0); >> + portio_list_init(&port_list, OBJECT(s), wdt_portio_list, s, "ib700"); >> + portio_list_add(&port_list, isa_address_space_io(&s->parent_obj), 0); >> + portio_list_destroy(&port_list); >> } >> >> static void wdt_ib700_reset(DeviceState *dev) >> > >
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c index 28eed81..e43d990 100644 --- a/hw/audio/adlib.c +++ b/hw/audio/adlib.c @@ -293,7 +293,7 @@ static MemoryRegionPortio adlib_portio_list[] = { static void adlib_realizefn (DeviceState *dev, Error **errp) { AdlibState *s = ADLIB(dev); - PortioList *port_list = g_new(PortioList, 1); + PortioList port_list; struct audsettings as; if (glob_adlib) { @@ -349,8 +349,9 @@ static void adlib_realizefn (DeviceState *dev, Error **errp) adlib_portio_list[0].offset = s->port; adlib_portio_list[1].offset = s->port + 8; - portio_list_init (port_list, OBJECT(s), adlib_portio_list, s, "adlib"); - portio_list_add (port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_init (&port_list, OBJECT(s), adlib_portio_list, s, "adlib"); + portio_list_add (&port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_destroy (&port_list); } static Property adlib_properties[] = { diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 47bbf1f..7a361c7 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -2055,7 +2055,7 @@ static int qxl_init_primary(PCIDevice *dev) { PCIQXLDevice *qxl = DO_UPCAST(PCIQXLDevice, pci, dev); VGACommonState *vga = &qxl->vga; - PortioList *qxl_vga_port_list = g_new(PortioList, 1); + PortioList qxl_vga_port_list; int rc; qxl->id = 0; @@ -2064,10 +2064,11 @@ static int qxl_init_primary(PCIDevice *dev) vga_common_init(vga, OBJECT(dev)); vga_init(vga, OBJECT(dev), pci_address_space(dev), pci_address_space_io(dev), false); - portio_list_init(qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, + portio_list_init(&qxl_vga_port_list, OBJECT(dev), qxl_vga_portio_list, vga, "vga"); - portio_list_set_flush_coalesced(qxl_vga_port_list); - portio_list_add(qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); + portio_list_set_flush_coalesced(&qxl_vga_port_list); + portio_list_add(&qxl_vga_port_list, pci_address_space_io(dev), 0x3b0); + portio_list_destroy(&qxl_vga_port_list); vga->con = graphic_console_init(DEVICE(dev), 0, &qxl_ops, qxl); qemu_spice_display_init_common(&qxl->ssd); diff --git a/hw/display/vga.c b/hw/display/vga.c index 063319d..72feafd 100644 --- a/hw/display/vga.c +++ b/hw/display/vga.c @@ -2351,8 +2351,8 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, { MemoryRegion *vga_io_memory; const MemoryRegionPortio *vga_ports, *vbe_ports; - PortioList *vga_port_list = g_new(PortioList, 1); - PortioList *vbe_port_list = g_new(PortioList, 1); + PortioList vga_port_list; + PortioList vbe_port_list; qemu_register_reset(vga_reset, s); @@ -2367,13 +2367,15 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space, 1); memory_region_set_coalescing(vga_io_memory); if (init_vga_ports) { - portio_list_init(vga_port_list, obj, vga_ports, s, "vga"); - portio_list_set_flush_coalesced(vga_port_list); - portio_list_add(vga_port_list, address_space_io, 0x3b0); + portio_list_init(&vga_port_list, obj, vga_ports, s, "vga"); + portio_list_set_flush_coalesced(&vga_port_list); + portio_list_add(&vga_port_list, address_space_io, 0x3b0); + portio_list_destroy(&vga_port_list); } if (vbe_ports) { - portio_list_init(vbe_port_list, obj, vbe_ports, s, "vbe"); - portio_list_add(vbe_port_list, address_space_io, 0x1ce); + portio_list_init(&vbe_port_list, obj, vbe_ports, s, "vbe"); + portio_list_add(&vbe_port_list, address_space_io, 0x1ce); + portio_list_destroy(&vbe_port_list); } } diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c index dc7a767..f27ce25 100644 --- a/hw/dma/i82374.c +++ b/hw/dma/i82374.c @@ -137,11 +137,12 @@ static void i82374_isa_realize(DeviceState *dev, Error **errp) { ISAi82374State *isa = I82374(dev); I82374State *s = &isa->state; - PortioList *port_list = g_new(PortioList, 1); + PortioList port_list; - portio_list_init(port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); - portio_list_add(port_list, isa_address_space_io(&isa->parent_obj), + portio_list_init(&port_list, OBJECT(isa), i82374_portio_list, s, "i82374"); + portio_list_add(&port_list, isa_address_space_io(&isa->parent_obj), isa->iobase); + portio_list_destroy(&port_list); i82374_realize(s, errp); diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c index 55d0100..7b88ffc 100644 --- a/hw/isa/isa-bus.c +++ b/hw/isa/isa-bus.c @@ -108,15 +108,16 @@ void isa_register_portio_list(ISADevice *dev, uint16_t start, const MemoryRegionPortio *pio_start, void *opaque, const char *name) { - PortioList *piolist = g_new(PortioList, 1); + PortioList piolist; /* START is how we should treat DEV, regardless of the actual contents of the portio array. This is how the old code actually handled e.g. the FDC device. */ isa_init_ioport(dev, start); - portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name); - portio_list_add(piolist, isabus->address_space_io, start); + portio_list_init(&piolist, OBJECT(dev), pio_start, opaque, name); + portio_list_add(&piolist, isabus->address_space_io, start); + portio_list_destroy(&piolist); } static void isa_device_init(Object *obj) diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index e243651..505e053 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -375,7 +375,7 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) CPUPPCState *env = NULL; nvram_t nvram; M48t59State *m48t59; - PortioList *port_list = g_new(PortioList, 1); + PortioList port_list; #if 0 MemoryRegion *xcsr = g_new(MemoryRegion, 1); #endif @@ -542,8 +542,9 @@ static void ppc_prep_init(QEMUMachineInitArgs *args) cpu = POWERPC_CPU(first_cpu); sysctrl->reset_irq = cpu->env.irq_inputs[PPC6xx_INPUT_HRESET]; - portio_list_init(port_list, NULL, prep_portio_list, sysctrl, "prep"); - portio_list_add(port_list, isa_address_space_io(isa), 0x0); + portio_list_init(&port_list, NULL, prep_portio_list, sysctrl, "prep"); + portio_list_add(&port_list, isa_address_space_io(isa), 0x0); + portio_list_destroy(&port_list); /* PowerPC control and status register group */ #if 0 diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c index bc994a4..fb1ca36 100644 --- a/hw/watchdog/wdt_ib700.c +++ b/hw/watchdog/wdt_ib700.c @@ -106,14 +106,15 @@ static const MemoryRegionPortio wdt_portio_list[] = { static void wdt_ib700_realize(DeviceState *dev, Error **errp) { IB700State *s = IB700(dev); - PortioList *port_list = g_new(PortioList, 1); + PortioList port_list; ib700_debug("watchdog init\n"); s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s); - portio_list_init(port_list, OBJECT(s), wdt_portio_list, s, "ib700"); - portio_list_add(port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_init(&port_list, OBJECT(s), wdt_portio_list, s, "ib700"); + portio_list_add(&port_list, isa_address_space_io(&s->parent_obj), 0); + portio_list_destroy(&port_list); } static void wdt_ib700_reset(DeviceState *dev)
PortioList is an abstraction used for construction of MemoryRegionPortioList from MemoryRegionPortio. It is not needed later, so there is no need to allocate it dynamically. Also portio_list_destroy should be called to free memory allocated in portio_list_init. This change spans several target platforms. The following testcases cover all changed lines: qemu-system-ppc -M prep qemu-system-i386 -vga qxl qemu-system-i386 -M isapc -soundhw adlib -device ib700,id=watchdog0,bus=isa.0 Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru> --- hw/audio/adlib.c | 7 ++++--- hw/display/qxl.c | 9 +++++---- hw/display/vga.c | 16 +++++++++------- hw/dma/i82374.c | 7 ++++--- hw/isa/isa-bus.c | 7 ++++--- hw/ppc/prep.c | 7 ++++--- hw/watchdog/wdt_ib700.c | 7 ++++--- 7 files changed, 34 insertions(+), 26 deletions(-) Coding style in adlib.c slightly differs from QEMU coding style. Particularly there are spaces between function name and parenthesis. I decided to not mix coding styles, but as a result checkpatch.pl complains on this patch.