Re: [xml] Useless function calls in xmlSetProp()?

[Daniel Veillard <veillard redhat com>]

On Fri, Jan 25, 2008 at 11:33:05AM +0100, Julien Charbon wrote:
>   Hi all,
>
> it's seems that function calls:
>
>   buffer = xmlEncodeEntitiesReentrant(doc, value)
>   list = xmlStringGetNodeList(doc, buffer);
>
> can be exactly replaced by a simple:
>
>   list = xmlNewDocText(doc, value);
>
>   You will find theses calls in tree.c. More precisely  in 
> xmlNewPropInternal() and in xmlSetNsProp(), both called by xmlSetProp().
>
>   In fact all that xmlEncodeEntitiesReentrant() does, is exactly 
> undone by xmlStringGetNodeList(). There is any 
> technical/practical/historical reasons to keep these calls in tree.c?
>
>   Below a patch that do this replacement on current trunk. [Just  to 
> illustrate my concern]. Our application and libxml2's "make tests" are 
> happy with this change.

  I don't believe the patch is right because an attribute 
list of children can be list of text and entities references,
and well your patch reduces it to just the case where you don't have an
entity reference in attribute values. Even if broken parser APIs
like SAX let people believe that attribute values can only be made
of one text node, this is not true from the spec POV and libxml2 which
was designed as an editing toolkit allows maintains entities references
in attribute values.

Daniel


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