Re: [xml] xmlRemoveID bug and a patch

[Daniel Veillard <veillard redhat com>]

On Sat, Aug 30, 2003 at 08:13:48PM +0100, bill hobbiton cjb net wrote:
> Hi,
>
> I would like to draw your attention to the xmlRemoveID function in the
> valid.c file of the libxml2 tree. I have noticed a bug in that this
> function does not actually do what it is intended for and remove the ID
> from the internal ID hash table. I have attached a patch that rectifies

  No that's intended. The IDness is defined by the presence of the attribute.
I think your patch is wrong w.r.t. the definition of IDness in the XML spec.

> Also, I would like to request either some documentation or functionality 
> changes of the following functions:
>
> xmlSetProp (and xmlNewProp) - these functions will NOT call xmlRemoveID or 
> xmlAddID on any ID properties. I think that they either should, if 
> necessary, or it be explicitly documented that they do not.

  Substanciate why you think it's wrong. I do think the current behaviour
is the right one w.r.t. the XML spec. The fact that an attribute is of type
ID is dependant on the DTD, this does not change with xmlSetProp/xmlNewProp.
So the IDness property is directly related to 1/ the definition in the DTD
2/ the presence of the attribute on the element.

> xmlUnsetProp, or xmlFreeProp to be more precise - will xmlRemoveID if 
> necessary. This doesn't seem symmetrical with the xmlSetProp as the name 
> implies and led me to memory being freed twice errors.

  I don't understand where you got a double free in libxml2, provide a
description and reproducable test please.

> Finally, I think that the same functions regarding to ID references have 
> the same problems. I haven't touched these, but it should probably be 
> fixed or noted as a bug aswell.
>
> I hope this is of some help.

   Unfortunately I think this is wrong. The problem is that you do not
try to justify the change, and until you can, I don't think a change
should be done. Please provide a reproductible case if you generate a 
libxml2 crash, and a pointer to the part of the spec you think the
implementation violates if this is the case too.

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/