On Thu, 05/24 19:16, Paolo Bonzini wrote: > On 21/05/2018 08:35, Fam Zheng wrote: > > Coverity doesn't like the tests under fail label (report CID 1385847). > > Reset the fields so the clean up order is more apparent. > > > > Signed-off-by: Fam Zheng <f...@redhat.com> > > --- > > block/nvme.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/block/nvme.c b/block/nvme.c > > index 6f71122bf5..8239b920c8 100644 > > --- a/block/nvme.c > > +++ b/block/nvme.c > > @@ -560,6 +560,13 @@ static int nvme_init(BlockDriverState *bs, const char > > *device, int namespace, > > qemu_co_queue_init(&s->dma_flush_queue); > > s->nsid = namespace; > > s->aio_context = bdrv_get_aio_context(bs); > > + > > + /* Fields we've not touched should be zero-initialized by block layer > > + * already, but reset them anyway to make the error handling code > > easier to > > + * reason. */ > > + s->regs = NULL; > > + s->vfio = NULL; > > + > > ret = event_notifier_init(&s->irq_notifier, 0); > > if (ret) { > > error_setg(errp, "Failed to init event notifier"); > > > > I think we should just mark it as a false positive or do something like > > fail_regs: > qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE); > fail_vfio: > qemu_vfio_close(s->vfio); > fail: > g_free(s->queues); > event_notifier_cleanup(&s->irq_notifier); > return ret; > > even though it's a larger patch.
And that makes five labels in total, I'm not sure I like it: fail_handler: aio_set_event_notifier(bdrv_get_aio_context(bs), &s->irq_notifier, false, NULL, NULL); fail_queue: nvme_free_queue_pair(bs, s->queues[0]); fail_regs: qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)s->regs, 0, NVME_BAR_SIZE); fail_vfio: qemu_vfio_close(s->vfio); fail: g_free(s->queues); event_notifier_cleanup(&s->irq_notifier); return ret; Maybe we just mark it as false positive then? Fam