[libvirt] [PATCH] Merge all returns paths from dispatcher into single path

Eric Blake eblake at redhat.com
Thu Apr 14 20:04:01 UTC 2011


On 04/14/2011 02:01 PM, Eric Blake wrote:
> On 04/14/2011 07:29 AM, Daniel P. Berrange wrote:
>>>>  
>>>>      parent = virNodeDeviceGetParent(dev);
>>>
>>> ...and malloc'd on this path.
>>
>> It isn't malloc'd here actually. This is returning
>> a 'const char *'...
> 
> Umm - libvirt.c declares it as 'const char *', but defers to the
> deviceMonitor->deviceGetParent callback.  And that callback returns
> 'char *', not 'const char *'.  In turn, in
> node_device_driver.c:nodeDeviceGetParent, the callback that actually
> implements things is actually setting ret to at strdup() value.
> 
> Ouch - our API inherently leaks.
> 
>>>
>>> ...and add unconditional VIR_FREE(parent) here.  I'm surprised we
>>> haven't noticed that leak before.
>>
>> ..meaning it isn't actually a leak here :-)
> 
> Yes it is, by design :(

Now I'm waffling.  It looks like libvirt.c is caching the result from
the callback.  So even though the callback exposes strdup()'d memory,
libvirt.c is caching that in the virNodeDevicePtr, and only ever calling
the callback once.  So it's not a leak after all.

Sorry for my confusion.  /me goes and crawls back in my hole ...

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110414/ef38d2ed/attachment-0001.sig>


More information about the libvir-list mailing list