Re: [Qemu-devel] [PATCH v8 18/23] RISC-V VirtIO Machine

[Peter Maydell]

On 2 March 2018 at 13:51, Michael Clark <address@hidden> wrote:
> RISC-V machine with device-tree, 16550a UART and VirtIO MMIO.
> The following machine is implemented:
>
> - 'virt'; CLINT, PLIC, 16550A UART, VirtIO MMIO, device-tree
>
> Acked-by: Richard Henderson <address@hidden>
> Signed-off-by: Palmer Dabbelt <address@hidden>
> Signed-off-by: Michael Clark <address@hidden>

Hi; Coverity spots (CID 1390606) that in this function you
leak a little bit of memory:

> +static void riscv_virt_board_init(MachineState *machine)
> +{

> +    /* create PLIC hart topology configuration string */
> +    plic_hart_config_len = (strlen(VIRT_PLIC_HART_CONFIG) + 1) * smp_cpus;
> +    plic_hart_config = g_malloc0(plic_hart_config_len);

Here we allocate memory into plic_hart_config...

> +    for (i = 0; i < smp_cpus; i++) {
> +        if (i != 0) {
> +            strncat(plic_hart_config, ",", plic_hart_config_len);
> +        }
> +        strncat(plic_hart_config, VIRT_PLIC_HART_CONFIG, 
> plic_hart_config_len);
> +        plic_hart_config_len -= (strlen(VIRT_PLIC_HART_CONFIG) + 1);
> +    }
> +
> +    /* MMIO */
> +    s->plic = sifive_plic_create(memmap[VIRT_PLIC].base,
> +        plic_hart_config,

(and this function doesn't take ownership of the string)

> +        VIRT_PLIC_NUM_SOURCES,
> +        VIRT_PLIC_NUM_PRIORITIES,
> +        VIRT_PLIC_PRIORITY_BASE,
> +        VIRT_PLIC_PENDING_BASE,
> +        VIRT_PLIC_ENABLE_BASE,
> +        VIRT_PLIC_ENABLE_STRIDE,
> +        VIRT_PLIC_CONTEXT_BASE,
> +        VIRT_PLIC_CONTEXT_STRIDE,
> +        memmap[VIRT_PLIC].size);
> +    sifive_clint_create(memmap[VIRT_CLINT].base,
> +        memmap[VIRT_CLINT].size, smp_cpus,
> +        SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE);
> +    sifive_test_create(memmap[VIRT_TEST].base);
> +
> +    for (i = 0; i < VIRTIO_COUNT; i++) {
> +        sysbus_create_simple("virtio-mmio",
> +            memmap[VIRT_VIRTIO].base + i * memmap[VIRT_VIRTIO].size,
> +            SIFIVE_PLIC(s->plic)->irqs[VIRTIO_IRQ + i]);
> +    }
> +
> +    serial_mm_init(system_memory, memmap[VIRT_UART0].base,
> +        0, SIFIVE_PLIC(s->plic)->irqs[UART0_IRQ], 399193,
> +        serial_hds[0], DEVICE_LITTLE_ENDIAN);

...but we don't free the memory before leaving.

> +}

Not a big deal since this function is only run once, but adding
in the necessary g_free(plic_hart_config) will placate Coverity.

thanks
-- PMM