On 12.10.2012 11:58, Laine Stump wrote:
> 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;
> }
>
ACK
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list