Re: [xml] xmlRemoveID bug and a patch

On Sat, Aug 30, 2003 at 08:13:48PM +0100, bill hobbiton cjb net wrote:


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 Veillard      | Red Hat Network
veillard redhat com  | libxml GNOME XML XSLT toolkit | Rpmfind RPM search engine

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]