Re: [xml] Potential wrong usage of xmlIsID() in tree.c

[Daniel Veillard <veillard redhat com>]

On Fri, Feb 03, 2006 at 03:52:46PM +0100, Kasimier Buchcik wrote:
> Hi,
>
> We have the following code in tree.c, xmlCopyPropInternal():
>
> if ((target!= NULL) && (cur!= NULL) &&
>       (target->doc != NULL) && (cur->doc != NULL) &&
>       (cur->doc->ids != NULL) && (cur->parent != NULL)) {
>       if (xmlIsID(cur->doc, cur->parent, cur)) {
>   [... add the id to the target doc ...]
> }
>
> This queries if the current attr is an ID in the _source_ doc via
> xmlIsID().
>
> I think this should check if the newly created attribute is an ID in
> the _destination_ doc.

  Hum, true, sounds there is somthing fishy there, though the itent is
to keep the IDness property, i.e. if you used #foo in th old doc to point
to that node, well #foo will still point to it in the new one.

> Just an observation: Currently, for branch-copy operations a DTD-lookup
> is performed (together with a QName building string operation) for every
> attribute (except for xml:id attributes).
>
> There is more code in tree.c using xmlIsID() and I think some of
> the calls should be substituted for the XML_ATTRIBUTE_ID-test.
> E.g. in xmlSetProp(), xmlIsID() is called for an existent
> attribute if it is about to be replaced by a new one.
>
> Doesn't xmlIsID() have to be called _only_ if a newly created
> attribute is added to the doc's tree? If it's already insideo

   Hum, it just test, and should not have side effect so I don't see why
we should be more restrictive in its use, plus it's part of the public API...

> the doc's tree, then XML_ATTRIBUTE_ID should be
> aready set - or am I missing something here?

  unclear,

Daniel

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