Re: [Qemu-devel] [PULL 10/10] hw/arm: versal: Add a virtual Xilinx Versal board

[Peter Maydell]

On Fri, 2 Nov 2018 at 17:24, Peter Maydell <address@hidden> wrote:
>
> From: "Edgar E. Iglesias" <address@hidden>
>
> Add a virtual Xilinx Versal board.
>
> This board is based on the Xilinx Versal SoC. The exact
> details of what peripherals are attached to this board
> will remain in control of QEMU. QEMU will generate an
> FDT on the fly for Linux and other software to auto-discover
> peripherals.
>
> Signed-off-by: Edgar E. Iglesias <address@hidden>
> Message-id: address@hidden
> Reviewed-by: Peter Maydell <address@hidden>
> Signed-off-by: Peter Maydell <address@hidden>

Hi Edgar -- I've been running the clang-7 leak sanitizer
on "make check", and it spotted a couple of minor memory
leaks in the versal board code:

> +static void fdt_add_gic_nodes(VersalVirt *s)
> +{
> +    char *nodename;
> +
> +    nodename = g_strdup_printf("/address@hidden", MM_GIC_APU_DIST_MAIN);
> +    qemu_fdt_add_subnode(s->fdt, nodename);
> +    qemu_fdt_setprop_cell(s->fdt, nodename, "phandle", s->phandle.gic);
> +    qemu_fdt_setprop_cells(s->fdt, nodename, "interrupts",
> +                           GIC_FDT_IRQ_TYPE_PPI, VERSAL_GIC_MAINT_IRQ,
> +                           GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> +    qemu_fdt_setprop(s->fdt, nodename, "interrupt-controller", NULL, 0);
> +    qemu_fdt_setprop_sized_cells(s->fdt, nodename, "reg",
> +                                 2, MM_GIC_APU_DIST_MAIN,
> +                                 2, MM_GIC_APU_DIST_MAIN_SIZE,
> +                                 2, MM_GIC_APU_REDIST_0,
> +                                 2, MM_GIC_APU_REDIST_0_SIZE);
> +    qemu_fdt_setprop_cell(s->fdt, nodename, "#interrupt-cells", 3);
> +    qemu_fdt_setprop_string(s->fdt, nodename, "compatible", "arm,gic-v3");

nodename is allocated but never freed here.

> +}


> +#define NUM_VIRTIO_TRANSPORT 32
> +static void create_virtio_regions(VersalVirt *s)
> +{
> +    int virtio_mmio_size = 0x200;
> +    int i;
> +
> +    for (i = 0; i < NUM_VIRTIO_TRANSPORT; i++) {
> +        char *name = g_strdup_printf("virtio%d", i);;
> +        hwaddr base = MM_TOP_RSVD + i * virtio_mmio_size;
> +        int irq = VERSAL_RSVD_HIGH_IRQ_FIRST + i;
> +        MemoryRegion *mr;
> +        DeviceState *dev;
> +        qemu_irq pic_irq;
> +
> +        pic_irq = qdev_get_gpio_in(DEVICE(&s->soc.fpd.apu.gic), irq);
> +        dev = qdev_create(NULL, "virtio-mmio");
> +        object_property_add_child(OBJECT(&s->soc), name, OBJECT(dev),
> +                                  &error_fatal);
> +        qdev_init_nofail(dev);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic_irq);
> +        mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> +        memory_region_add_subregion(&s->soc.mr_ps, base, mr);
> +        sysbus_create_simple("virtio-mmio", base, pic_irq);

The body of this loop allocates name but forgets to free it.

> +    }

thanks
-- PMM