Re: [xml] function consolidation

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:

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 Veillard      | Red Hat Desktop team
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]