[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] XPath accessors cleanup



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.

	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

Cheers,
Mark.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]