Re: [libvirt] [PATCH v4 10/25] [ACKED] conf: make virDomainPCIAddressGetNextSlot() a local static function

2016-10-21 Thread Laine Stump

On 10/18/2016 10:46 AM, Andrea Bolognani wrote:

On Fri, 2016-10-14 at 15:54 -0400, Laine Stump wrote:

This function is no longer needed outside of domain_addr.c.
---
   src/conf/domain_addr.c   | 2 +-
   src/conf/domain_addr.h   | 5 -
   src/libvirt_private.syms | 1 -
   3 files changed, 1 insertion(+), 7 deletions(-)
  
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c

index 1710220..3a9e474 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -591,7 +591,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
   }
   
   
-int

+static int
   virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,
  virPCIDeviceAddressPtr next_addr,
  virDomainPCIConnectFlags flags)
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 904d060..4d6d12a 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -155,11 +155,6 @@ int 
virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs,
  virPCIDeviceAddressPtr addr)
   ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
   
-int virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,

-   virPCIDeviceAddressPtr next_addr,
-   virDomainPCIConnectFlags flags)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

As noted while reviewing v3, you're losing both
ATTRIBUTE_NONNULL() with this commit. I was probably not
clear enough last time around, sorry about that :(


Nah, I just assumed you were merely pointing it out, not that you 
actually wanted me to maintain it in the static function.


I actually dislike ATTRIBUTE_NONNULL() because it doesn't guarantee any 
behavior of the functions callers, and even worse - it ends up 
optimizing out any code in the function that actually *checks* for those 
arguments being NULL. So it ends up being 0 help in catching any 
accidental NULL references (*maybe* it can be used by a static analyzer, 
but I remember at least one occasion where it actually covered up / 
created a bug by removing the check for NULL when one of the callers of 
a function actually was sending a NULL).


Still, I'll move the ATTRIBUTE_NONNULL over to the static function, and 
leave the debate of whether or not it really should be there to another day.


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v4 10/25] [ACKED] conf: make virDomainPCIAddressGetNextSlot() a local static function

2016-10-18 Thread Andrea Bolognani
On Fri, 2016-10-14 at 15:54 -0400, Laine Stump wrote:
> This function is no longer needed outside of domain_addr.c.
> ---
>  src/conf/domain_addr.c   | 2 +-
>  src/conf/domain_addr.h   | 5 -
>  src/libvirt_private.syms | 1 -
>  3 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 1710220..3a9e474 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -591,7 +591,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr 
> addrs)
>  }
>  
>  
> -int
> +static int
>  virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr next_addr,
> virDomainPCIConnectFlags flags)
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index 904d060..4d6d12a 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -155,11 +155,6 @@ int 
> virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs,
> virPCIDeviceAddressPtr addr)
>  ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>  
> -int virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,
> -   virPCIDeviceAddressPtr next_addr,
> -   virDomainPCIConnectFlags flags)
> -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

As noted while reviewing v3, you're losing both
ATTRIBUTE_NONNULL() with this commit. I was probably not
clear enough last time around, sorry about that :(

ACK with the attributes properly carried over.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v4 10/25] [ACKED] conf: make virDomainPCIAddressGetNextSlot() a local static function

2016-10-14 Thread Laine Stump
This function is no longer needed outside of domain_addr.c.
---
 src/conf/domain_addr.c   | 2 +-
 src/conf/domain_addr.h   | 5 -
 src/libvirt_private.syms | 1 -
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 1710220..3a9e474 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -591,7 +591,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs)
 }
 
 
-int
+static int
 virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,
virPCIDeviceAddressPtr next_addr,
virDomainPCIConnectFlags flags)
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 904d060..4d6d12a 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -155,11 +155,6 @@ int 
virDomainPCIAddressReleaseSlot(virDomainPCIAddressSetPtr addrs,
virPCIDeviceAddressPtr addr)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
-int virDomainPCIAddressGetNextSlot(virDomainPCIAddressSetPtr addrs,
-   virPCIDeviceAddressPtr next_addr,
-   virDomainPCIConnectFlags flags)
-ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-
 int virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev,
virDomainPCIConnectFlags flags,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9f9512a..0c76f17 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -99,7 +99,6 @@ virDomainPCIAddressAsString;
 virDomainPCIAddressBusSetModel;
 virDomainPCIAddressEnsureAddr;
 virDomainPCIAddressFlagsCompatible;
-virDomainPCIAddressGetNextSlot;
 virDomainPCIAddressReleaseSlot;
 virDomainPCIAddressReserveAddr;
 virDomainPCIAddressReserveNextAddr;
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list