Re: [libvirt] [PATCH 02/22] pci: change stubDriver from const char* to char*
On 06/24/2013 10:55 AM, Daniel P. Berrange wrote:
On Mon, Jun 24, 2013 at 05:54:51AM -0400, Laine Stump wrote:
> Previously stubDriver was always set from a string literal, so it was
> okay to use a const char * that wasn't freed when the virPCIDevice was
> freed. This will not be the case in the near future, so it is now a
> char* that is allocated in virPCIDeviceSetStubDriver() and freed
> during virPCIDeviceFree().
> ---
> src/qemu/qemu_driver.c | 6 ++++--
> src/qemu/qemu_hostdev.c | 25 +++++++++++++++++--------
> src/util/virpci.c | 8 +++++---
> src/util/virpci.h | 4 ++--
> src/xen/xen_driver.c | 3 ++-
> 5 files changed, 30 insertions(+), 16 deletions(-)
ACK
It is bad practice to assume you can keep around a copy of any
'const char*' passed in as a parameter, since the const-ness of
the parameter says nothing about whether the caller will free
the parameter value.
And this is just the tip of the iceberg with these device lists. While
re-verifying the veracity of this fix, I ran across yet another case
where virPCIDevices are simultaneously placed on 2 lists (through more
use of this "list steal" function), and under certain circumstances
would end up being leaked or (much worse) double freed (I think
sometimes it's overlooked that virObjectUnref of a virPCIDeviceListPtr
will free all the devices on the list, regardless of how many other
places still point to them...)
I'm going to try and unthread that and post a fix tonight.
At any rate, this one is now pushed. Thanks!