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

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

 



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

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]