[libvirt] [PATCHv2 09/12] pci: virPCIDeviceListAddCopy API

[Laine Stump]

On 06/25/2013 06:45 AM, Daniel P. Berrange wrote:
> On Mon, Jun 24, 2013 at 11:05:35PM -0400, Laine Stump wrote:
>> Make a copy of the device and add the copy to the
>> list. (virPCIDeviceListAdd() adds the original object to the list
>> instead).
>> ---
>>  src/libvirt_private.syms |  1 +
>>  src/util/virpci.c        | 17 +++++++++++++++++
>>  src/util/virpci.h        |  1 +
>>  3 files changed, 19 insertions(+)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index e906742..bf9bd12 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1703,6 +1703,7 @@ virPCIDeviceGetUnbindFromStub;
>>  virPCIDeviceGetUsedBy;
>>  virPCIDeviceIsAssignable;
>>  virPCIDeviceListAdd;
>> +virPCIDeviceListAddCopy;
>>  virPCIDeviceListCount;
>>  virPCIDeviceListDel;
>>  virPCIDeviceListFind;
>> diff --git a/src/util/virpci.c b/src/util/virpci.c
>> index cbade1b..378b4f3 100644
>> --- a/src/util/virpci.c
>> +++ b/src/util/virpci.c
>> @@ -1705,6 +1705,23 @@ virPCIDeviceListAdd(virPCIDeviceListPtr list,
>>      return 0;
>>  }
>>  
>> +
>> +/* virPCIDeviceListAddCopy - add a *copy* of the device to this list */
>> +int
>> +virPCIDeviceListAddCopy(virPCIDeviceListPtr list, virPCIDevicePtr dev)
>> +{
>> +    virPCIDevicePtr copy = virPCIDeviceCopy(dev);
>> +
>> +    if (!copy)
>> +        return -1;
>> +    if (virPCIDeviceListAdd(list, copy) < 0) {
>> +        virPCIDeviceFree(copy);
>> +        return -1;
>> +    }
>> +    return 0;
>> +}
>> +
>> +
>>  virPCIDevicePtr
>>  virPCIDeviceListGet(virPCIDeviceListPtr list,
>>                      int idx)
>> diff --git a/src/util/virpci.h b/src/util/virpci.h
>> index 972f86b..5f80de3 100644
>> --- a/src/util/virpci.h
>> +++ b/src/util/virpci.h
>> @@ -83,6 +83,7 @@ void virPCIDeviceReattachInit(virPCIDevice *dev);
>>  virPCIDeviceListPtr virPCIDeviceListNew(void);
>>  int  virPCIDeviceListAdd(virPCIDeviceListPtr list,
>>                           virPCIDevicePtr dev);
>> +int virPCIDeviceListAddCopy(virPCIDeviceListPtr list, virPCIDevicePtr dev);
>>  virPCIDevicePtr virPCIDeviceListGet(virPCIDeviceListPtr list,
>>                                      int idx);
>>  int virPCIDeviceListCount(virPCIDeviceListPtr list);
> ACK, though as a followup, it'd be nice to actually eliminate the
> virPCIDeviceListAdd variant, since APIs stealing ownership of passed
> in parameters have been a good source of bugs historically.

Yes, and a source of confusion (which I guess is a good part of the
cause of the bugs :-)

The *really* ugly part of all the virPCIDeviceList code is all the
places that an object isn't even stolen, but is placed on multiple lists
simultaneously.