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

[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.