[Libvir] [PATCH] XPath accessors cleanup

[Daniel Veillard]

On Fri, Mar 30, 2007 at 11:01:25AM +0100, Mark McLoughlin wrote:
> Hi Daniel,
> 	Nice stuff !
> 
> On Fri, 2007-03-30 at 05:38 -0400, Daniel Veillard wrote:
> 
> > +char *
> > +virXPathString(const char *xpath, xmlXPathContextPtr ctxt) {
> ...
> > +    ret = (char *) obj->stringval;
> > +    obj->stringval = NULL;
> 
> > +int
> > +virXPathNodeSet(const char *xpath, xmlXPathContextPtr ctxt, xmlNodePtr **list) {
> ...
> > +    if (list != NULL) {
> > +       *list = obj->nodesetval->nodeTab;
> > +       obj->nodesetval->nodeTab = NULL;
> > +    }
> 
> 	This is all a little voodooish to me - e.g. you're assuming that
> libxml2 allocated these with malloc() and you're also kind of breaking
> the abstraction of xmlXPathObject by manipulating the structure like
> this.

  Right, sorry I just wanted to make the API and code as less dependant
from libxml2 idioms as possible, but you raise good point ;-)

> 	i.e. you know this works, because you wrote libxml2, but you're not
> using the abstraction very cleanly.
> 
> 	I'd suggest either:
> 
>   - copy the memory from the xmlXPathObject and return that
> 
>   - make the functions return "libxml memory" and use xmlFree() in the 
>     callers

  I prefer to copy and limit the libxml2 expertise needed to understand the
normal parsing code, so I will implement #1, there is no harm is doing a few
extra malloc()/free() at that point there.

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/