[libvirt] [libvirt-glib 23/37] Add test for adding a disk device

[Christophe Fergeau]

On Fri, Nov 11, 2011 at 07:05:18PM +0100, Marc-André Lureau wrote:
> On Thu, Nov 10, 2011 at 9:33 PM, Christophe Fergeau <cfergeau at redhat.com> wrote:
> >  devices = g_list_append(devices, disk);
> > +    gvir_config_domain_set_devices(domain, devices);
> > +    g_list_free(devices);
> > +    devices = NULL;
> 
> 
> Hmm, I realize this is a bit more tricky. You give up devices element
> owner ship but no the container.. I think this is bad, as the list
> contains invalid objects after leaving the functions. Insead, the
> set_devices () function should ref the element, and the caller should
> unref too with g_list_free_full (devices, g_object_unref)

I know this part is a bit messy, the _set_devices function doesn't keep the
device list around so it does not need to get a reference on these. The
reason why there is not a g_list_free_full call in the test program is that
each GVirConfigObject::finalize function will call
xmlFreeDoc(obj->priv->node->doc), but since after calling
gvir_config_domain_set_devices, both GVirConfigDomain and GVirConfigDevice*
will reference the same document, we will get a double free if we unref all
of the objects deriving from GVirConfigObject.

I chose to avoid this issue for now by not unreffing anything in the
example program. Maybe it would be "better" to have the unref here, but to
comment out the xmlFreeDoc() from GVirConfigObject::finalize.
This issue is solved the right way in some future patches by using a
refcounted GVirConfigXmlDoc which wraps xmlDocPtr.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111115/6354705f/attachment-0001.sig>