Re: [PATCH] XPath accessors cleanup

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

 



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.


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]