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

[Kasimier Buchcik <K Buchcik 4commerce de>]

Hi,

On Fri, 2006-02-03 at 11:13 -0500, Daniel Veillard wrote:
> 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.

Keep the IDness even if the target doc has a different or even no DTD?
If the attribute is an xml:id then yes, but otherwise I would say no.

> > 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,

Sorry, I was referring to the attr->atype == XML_ATTRIBUTE_ID check.
This marker is set on the attribute in xmlAddID(). We wouldn't have to
query through xmlIsID() if the marker is used to test IDness of attrs
already added to the doc's tree.

Regards,

Kasimier