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@xxxxxxxxxx> 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
Attachment:
pgp2VM848Kvy6.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list