Re: [PATCH 5/6] conf: give each hostdevdef a parent pointer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/20/2012 05:02 PM, Eric Blake wrote:
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 004cba7..12d48fb 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -1318,6 +1318,12 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def)
>>      if (!def)
>>          return;
>>  
>> +    /* if there is a parent device object, it will handle freeing the
>> +     * memory (including the info).
>> +     */
>> +    if (def->parent.type != VIR_DOMAIN_DEVICE_NONE)
>> +       return;
>> +
>>      virDomainDeviceInfoFree(def->info);
>>      VIR_FREE(def);
>>  }
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index 25019e7..924c7ec 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -344,6 +344,7 @@ enum virDomainHostdevSubsysType {
>>  typedef struct _virDomainHostdevDef virDomainHostdevDef;
>>  typedef virDomainHostdevDef *virDomainHostdevDefPtr;
>>  struct _virDomainHostdevDef {
>> +    virDomainDeviceDef parent; /* higher level Def containing this */
> However, that makes virDomainHostdevDef a rather large struct, since it
> is including an entire virDomainDeviceDef even if it is not otherwise
> used.  Is it any better to keep the back pointer as a pointer, along the
> lines of:
>
> if (!def->parent) return;
>
> struct _virDomainHostdefDef {
>     virDomainDeviceDefPtr parent;

virDomainDeviceDef *looks* big, but it's really just one int and one
pointer (everything after "int type" is a union of several different
pointer types). Making parent a virDomainDeviceDefPtr only saves
sizeof(int), but introduces an extra level of indirection and the
possibility of a memory leak, so I think it's best left as it is.

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]