Re: [REVIEW] Re: Fix xpath() to return namespace definitions

> I don't know enough about XML/XPATH to know if this is a good idea or not,
>

Actually currently because of the namespace problem, xpath() returns wrong
result (even worse: invalid xml!). So this patch corrects the behavior of
xpath() to the correct one.

> but I did go read the documentation of xmlCopyNode(), and I notice it says
> the second argument is
>
> extended: if 1 do a recursive copy (properties, namespaces and children
> when applicable) if 2 copy properties and namespaces (when applicable)
>
> Do we really want it to "copy children"? Perhaps the value should be 2?
> (I have no idea, I'm just raising the question.)
>

If we put 1 as the parameter, the resulting Node will link to the existing
children. In this case, the namespace problem for the children might be
still exists. If any of the children uses different namespace(s) than the
parent, the namespace definition will not be copied correctly.

Just to be safe, in the patch 1 passed as the second argument.

Ideally, we can traverse the Node object and generate XML accordingly, but
it is a lot of work, and a lot of duplicating libxml's code. Maybe we
should push this upstream to libxml?

I also notice that it says
>
> Returns: a new #xmlNodePtr, or NULL in case of error.
>
> and the patch is failing to cover the error case. Most likely, that
> would represent out-of-memory, so it definitely seems to be something
> we should account for.
>

Ah, overlooked that.

Skimming through libxml2's source, it looks like there are some other cases
beside out-of-memory. Will update the patch according to these cases.

Thanks for the review.

--
Ali Akbar