[libvirt] [PATCH libvirt-glib 1/5] Add suport for setting <filesystem> elements

[Christophe Fergeau]

On Wed, Nov 30, 2011 at 06:26:18PM +0100, Christophe Fergeau wrote:
> > +
> > +    if (!(old_node = gvir_config_xml_get_element(parent_node, child_name, NULL)))
> > +        return;
> > +
> > +    /* FIXME: should we make sure there are no multiple occurrences
> > +     * of this node?
> > +     */
> > +    xmlUnlinkNode(old_node);
> > +    xmlFreeNode(old_node);
> > +}
> 
> 
> I think we will get memory corruption if this API is combined with
> _attach_child:

And I'm wrong since the whole point of the introduction of GVirConfigXmlDoc
is to avoid this kind of memory corruption. However, in the example below,
fs_node will have a reference to a GVirConfigXmlDoc which has no relation
with its GVirConfigObject::node pointer, and this pointer will be pointing
to already freed memory which is suboptimal.

> 
> device_node = gvir_config_object_new("device");
> fs_node = gvir_config_object_new("fs");
> gvir_config_object_attach(device_node, fs_node);
> gvir_config_object_delete_child(device_node, "fs");
> g_object_unref(G_OBJECT(fs_node));
> 
> The xmlNodePtr held by fs_node will be freed twice, once by _delete_child
> and when _finalize runs after the call to g_object_unref
> 
> Having each GVirConfigObject keep a list of its GVirConfigObject children
> would make it possible to handle this case I think.
> 
> I'm fine with getting this function in even if there are known issues with
> it.

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/20111201/faddd955/attachment-0001.sig>