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 <batuz...@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)
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
