Re: [Qemu-devel] [PATCH v5 06/10] rocker: add new rocker switch device

[Stefan Hajnoczi]

On Thu, Jan 22, 2015 at 12:03:55AM -0800, address@hidden wrote:
> +        iov[iovcnt].iov_len = frag_len;
> +        iov[iovcnt].iov_base = g_malloc(frag_len);
> +        if (!iov[iovcnt].iov_base) {
> +            err = -ROCKER_ENOMEM;
> +            goto err_no_mem;
> +        }
> +
> +        if (pci_dma_read(dev, frag_addr, iov[iovcnt].iov_base,
> +                     iov[iovcnt].iov_len)) {
> +            err = -ROCKER_ENXIO;
> +            goto err_bad_io;
> +        }

iov[iovcnt].iov_base is leaked because iovcnt hasn't been incremented
yet.  The cleanup loop will not free this fragment.

> +static void rocker_test_dma_ctrl(Rocker *r, uint32_t val)
> +{
> +    PCIDevice *dev = PCI_DEVICE(r);
> +    char *buf;
> +    int i;
> +
> +    buf = malloc(r->test_dma_size);

Please use g_try_malloc()

> +
> +    if (!buf) {
> +        DPRINTF("test dma buffer alloc failed");
> +        return;
> +    }
> +
> +    switch (val) {
> +    case ROCKER_TEST_DMA_CTRL_CLEAR:
> +        memset(buf, 0, r->test_dma_size);
> +        break;
> +    case ROCKER_TEST_DMA_CTRL_FILL:
> +        memset(buf, 0x96, r->test_dma_size);
> +        break;
> +    case ROCKER_TEST_DMA_CTRL_INVERT:
> +        pci_dma_read(dev, r->test_dma_addr, buf, r->test_dma_size);
> +        for (i = 0; i < r->test_dma_size; i++) {
> +            buf[i] = ~buf[i];
> +        }
> +        break;
> +    default:
> +        DPRINTF("not test dma control val=0x%08x\n", val);
> +        return;
> +    }
> +    pci_dma_write(dev, r->test_dma_addr, buf, r->test_dma_size);
> +
> +    rocker_msix_irq(r, ROCKER_MSIX_VEC_TEST);
> +}

Where is buf freed?

> +void fp_port_set_macaddr(FpPort *port, MACAddr *macaddr)
> +{
> +/*XXX memcpy(port->conf.macaddr.a, macaddr.a, sizeof(port->conf.macaddr.a)); 
> */
> +}

What's missing here?


I didn't review the OF DPA code but the rest looks good.

pgpdcA33IoK4F.pgp
Description: PGP signature