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

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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