Bug #15966 Updated: replace_node incorrect

ID: 15966 Updated by: steinm@php.net Reported By: bigredlinux@yahoo.com -Status: Open +Status: Analyzed Bug Type: DOM XML related Operating System: Linux PHP Version: 4.1.2 New Comment: xmlReplaceNode replaces the node in the first argument, which is the current node, by the new copied node. Sounds reasonable to me. It returns the old node which we don't care about. You should have a reference to the old node anyway. Ok, the old node isn't deleted only unlinked but that is the only way to go since the corresponding PHP variable can't be deleted at this time. Making a copy is actually a point to discuss. But currently it needs to be done because if the same libxml node is returned you will probably end up with two PHP variables pointing to the same libxml node. If one is deleted the other one will lose its libxml node as well. I guess this could be solved with references but I don't know how yet. Previous Comments: ------------------------------------------------------------------------ [2002-03-08 20:39:22] bigredlinux@yahoo.com When replace_node is used, if the new node being inserted already exists in the xml document, it is to be first removed. Thus, the following lines in the C code are incorrect... if (NULL == (new_repnode = xmlCopyNode(repnode, 1))) { php_error(E_WARNING, "%s(): unable to clone node", get_active_function_name(TSRMLS_C)); RETURN_FALSE; } repnode = xmlReplaceNode(nodep, new_repnode); This code is making a copy of the new node, and hence when the libxml2 function xmlReplaceNode is run, it does not remove the node already existing in the document. The call to xmlReplaceNode should be the repnode, not the new_repnode. As a matter of fact, I see no need to even attempt to clone the node, unless it is to check that it is a true node. Thoughts? ------------------------------------------------------------------------ -- Edit this bug report at http://bugs.php.net/?id=15966&edit=1