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

[Michal Privoznik]

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