Re: [libvirt] [PATCH 1/4] conf: fix virDevicePCIAddressEqual args

This function really should have been taking virDevicePCIAddress* instead of the inefficient virDevicePCIAddress (results in copying two entire structs onto the stack rather than just two pointers), and returning a bool true/false (not matching is not necessarily a "failure", as a -1 return would imply, and also using "if (!virDevicePCIAddressEqual(x, y))" to mean "if x == y" is just a bit counterintuitive). --- src/conf/device_conf.c | 21 +++++++++------------ src/conf/device_conf.h | 4 ++-- src/network/bridge_driver.c | 8 ++++---- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index a852960..7b97f45 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -130,18 +130,15 @@ virDevicePCIAddressFormat(virBufferPtr buf, return 0; } -int -virDevicePCIAddressEqual(virDevicePCIAddress addr1, - virDevicePCIAddress addr2) +bool +virDevicePCIAddressEqual(virDevicePCIAddress *addr1, + virDevicePCIAddress *addr2) { - int ret = -1; - - if (addr1.domain == addr2.domain && - addr1.bus == addr2.bus && - addr1.slot == addr2.slot && - addr1.function == addr2.function) { - ret = 0; + if (addr1->domain == addr2->domain && + addr1->bus == addr2->bus && + addr1->slot == addr2->slot && + addr1->function == addr2->function) { + return true; } - - return ret; + return false; } diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index 26d5637..5318738 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -59,8 +59,8 @@ int virDevicePCIAddressFormat(virBufferPtr buf, virDevicePCIAddress addr, bool includeTypeInAddr); -int virDevicePCIAddressEqual(virDevicePCIAddress addr1, - virDevicePCIAddress addr2); +bool virDevicePCIAddressEqual(virDevicePCIAddress *addr1, + virDevicePCIAddress *addr2); VIR_ENUM_DECL(virDeviceAddressPciMulti) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a41c49c..917dd34 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3820,8 +3820,8 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) for (ii = 0; ii < netdef->nForwardIfs; ii++) { if (netdef->forwardIfs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI && - (virDevicePCIAddressEqual(hostdev->source.subsys.u.pci, - netdef->forwardIfs[ii].device.pci) == 0)) { + virDevicePCIAddressEqual(&hostdev->source.subsys.u.pci, + &netdef->forwardIfs[ii].device.pci)) { dev = &netdef->forwardIfs[ii]; break; } @@ -3972,8 +3972,8 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) for (ii = 0; ii < netdef->nForwardIfs; ii++) { if (netdef->forwardIfs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI && - (virDevicePCIAddressEqual(hostdev->source.subsys.u.pci, - netdef->forwardIfs[ii].device.pci) == 0)) { + virDevicePCIAddressEqual(&hostdev->source.subsys.u.pci, + &netdef->forwardIfs[ii].device.pci)) { dev = &netdef->forwardIfs[ii]; break; }