Re: [xml] xmlRemoveID patch

[Daniel Veillard <veillard redhat com>]

On Thu, Oct 02, 2003 at 02:59:06PM -0400, Rob Richards wrote:
> From: Daniel Veillard
> >   and I don't want that behaviour when streaming.
> > When streaming I don't want the ID to be removed from the table by that
> > function.
> > I want the attribute it reference to be NULLed and the id kept in the
> > table so that validation checking works in streaming mode too.
>
> The only way I can see this working is that xmlTextReaderFreeProp calls a
> different function than xmlRemoveID, or adding an additonal parameter, which
> is probably not a good thing to do if people are using it - though I still
> say it never worked ;). If you dont need the reference to the attribute,
> though I believe you would since someone could call xmlGetID while the
> element was still in scope which would return NULL, I am not sure wether
> xmlAddID could be fixed to test for this streaming condition as currently
> its test: ctxt->vstateNr != 0 fails.

  Yeah, I'm slowly migrating myself to this decision. It's quite easier now
at the time the reader was using the same functions.

> I tweaked that patch a bit and changed the line where it removes the ID from
> the hash to just cur->attr = NULL;
>
> That test works fine that way. No leaks, no segfaults. This really isnt the
> desired behavior however when not streaming, as it should remove the IDPtr
> and not just remove the reference between the IDPtr and AttrPtr, which is
> why I suggest either an additional function or just remove that block in
> xmlTextReaderFreeProp which was added last week (if the patch had been
> submitted before 9/27, the tests would have passed).

  Well if you have a new patch which works with CVS, please send it, though
creating that new xmlReaderFreeID() is the right solution.

Daniel

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