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