On Fri, Mar 25, 2005 at 04:24:49PM -0500, Rob Richards wrote: > Daniel Veillard wrote: > > > it must free the string before returning since the caller won't free it > >clearly this was the previous behaviour. > > > > > Hmm, I'm not too sure about that. > > The previous behavior (using testapi still) created and appended the > attribute on the PI node, so the call: > ret_val = xmlNewNsPropEatName(node, ns, name, (const xmlChar *)value); > would return an attribute node. > > The name was freed by the next line: > desret_xmlAttrPtr(ret_val); > > Now, however, ret_val is NULL since its an invalid node to set the > attribute on and no attribute created.. > It now works the same way as xmlNewNodeEatName as in that function if > NULL is returned, the caller has to free the name. (NULL only returned > if xmlMalloc failed there). > > Your call on behavior here but wanted to make sure I was clear on what > is happening before changing it.
I still think we should xmlFree() the name parameter. The underlying reason is that it's an API refinement and I prefer an immediate crash immediately identified in case someone expected a different behaviour than a slow memory leak which may only be tracked painfully months later. The function eats the name that's expected I think. The case of xmlNewNodeEatName the only error is when the program runs out of memory, then it will generate a leak, I will fix that, it's broken. We should really have this discussion on-list. Even if posting again the patch was looking like a duplicate we should stick to the list, that's why I'm adding it again. Someone may disagree with me for a good reason, and should have a chance to object, so Cc'ed to the list again. Daniel -- Daniel Veillard | Red Hat Desktop team http://redhat.com/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ xml mailing list, project page http://xmlsoft.org/ [email protected] http://mail.gnome.org/mailman/listinfo/xml
