[Libvirt-cim] [PATCH] Make xmlgen.c use libxml2 instead of hacking it out manually

[Daniel Veillard]

On Tue, Dec 02, 2008 at 08:43:48AM -0800, Dan Smith wrote:
> # HG changeset patch
> # User Dan Smith <danms at us.ibm.com>
> # Date 1228236226 28800
> # Node ID c34ae7c0ded8870aeba40a0798ac31f1ea2c5bbd
> # Parent  6e8a488cf67b47efb618c28aa2a9104d5b0e1b91
> Make xmlgen.c use libxml2 instead of hacking it out manually
> 
> Now that our XML generation is getting complex enough, I think it's worth
> using libxml2 to build a proper DOM tree and XML string.
> 
> Note that due to the invasiveness of this patch, it's almost unreadable.  It
> may be easier to review after it's applied to the tree.  Also, it deserves
> some serious review and testing for correctness and error-handling abilities.
> 
> Comments on this approach are welcome too, of course.

  up to here no real comment about the libxml2 API usage :-)

> +char *device_to_xml(struct virt_device *dev)
> +{
[...]
> +        doc = xmlNewDoc(BAD_CAST "1.0");
> +        if (doc == NULL)
> +                goto out;
> +        root = xmlNewNode(NULL, BAD_CAST "tmp");
> +        if (root == NULL)
> +                goto out;

  okay you need to build a root, surprizing but okay

[...]

> +        msg = func(root, dominfo);
> +        if (msg != NULL) {
> +                CU_DEBUG("Failed to create device XML: %s", msg);
> +                goto out;
> +        }
> +
> +        xmlDocSetRootElement(doc, root->children);
> +
> +        xmlDocDumpFormatMemory(doc, &_xml, &xml_size, 1);
> +        xml = strdup((char *)_xml);
> +        xmlFree(_xml);

  ouch it hurts a bit. In general I would rather suggest to use the new
xmlsave APIs http://xmlsoft.org/html/libxml-xmlsave.html
since you rely on the indenting, and hence some of the saving options

   - create an xmlBufferPtr  (xmlNewBuffer IIRC)
   - http://xmlsoft.org/html/libxml-xmlsave.html#xmlSaveToBuffer
     with XML_SAVE_FORMAT option
   - http://xmlsoft.org/html/libxml-xmlsave.html#xmlSaveTree
   - http://xmlsoft.org/html/libxml-xmlsave.html#xmlSaveClose
   - then grab the string from the buffer  and free the buffer

it's a bit more complex but gives you more control over the saving in
the long run, which may be useful. Could be encapsulated as a generic
node serialization routine if you plan to do this many times.


> +        xmlDocDumpFormatMemory(doc, &_xml, &xml_len, 1);
> +        if (_xml == NULL)
> +                goto out;
> +
> +        xml = strdup((char *)_xml);
> +        if (xml == NULL)
> +                msg = "Out of memory generating XML";
> +        else
> +                msg = NULL;

  similar here but with 
  http://xmlsoft.org/html/libxml-xmlsave.html#xmlSaveDoc
  instead of xmlSaveTree()

yours :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/