On Wed, 2018-09-05 at 14:53 +0200, Martin Kletzander wrote:
> On Wed, Sep 05, 2018 at 10:09:26AM +0200, Andrea Bolognani wrote:
> > for (i = 0; i < *n_vfname; i++) {
> > - if (virPCIGetAddrString((*virt_fns)[i]->domain,
> > - (*virt_fns)[i]->bus,
> > - (*virt_fns)[i]->slot,
> > - (*virt_fns)[i]->function,
> > - &pciConfigAddr) < 0) {
> > + if (!(pciConfigAddr = virPCIDeviceAddressAsString((*virt_fns)[i]))) {
>
> There is one more thing that this change does, previously it would fail for
> non-existing device, which it will not now. I don't think that is a problem,
> but wanted to mention that.
Well spotted; in this case, however, the addresses were obtained
directly from sysfs through virPCIGetVirtualFunctions() so, as you
mention, validating them again is unnecessary.
> > virReportSystemError(ENOSYS, "%s",
> > _("Failed to get PCI Config Address String"));
>
> You should remove this error message as virAsprintf() in
> virPCIDeviceAddressAsString() already sets an OOM Error.
Done.
> I you like working on this function there are lot of things that leak from the
> loop, so looking at that would be great, thanks.
I'll see what I can do :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
