[libxml++] Node: Fix memory problems in import_node().

[Kjell Ahlstedt <kjellahl src gnome org>]

commit 4022ac8831cb2d92ac3040e839ac9d729dc99c55
Author: Kjell Ahlstedt <kjell ahlstedt bredband net>
Date:   Thu Apr 19 09:21:55 2012 +0200

    Node: Fix memory problems in import_node().
    
    * libxml++/nodes/node.[h|cc]: Return added_node instead of imported_node,
    which libxml2 may delete. Delete the C++ wrapper of a deleted attribute node.
    * examples/import_node/example[1|2].xml:
    * examples/import_node/main.cc: Import attributes and a text node which is
    merged with an existing text node. Bug #672992.

 ChangeLog                         |   10 ++++++++++
 examples/import_node/example1.xml |    4 ++--
 examples/import_node/example2.xml |    2 +-
 examples/import_node/main.cc      |   34 ++++++++++++++++++++++++++++------
 libxml++/nodes/node.cc            |   23 ++++++++++++++++++++---
 libxml++/nodes/node.h             |    9 ++++++++-
 6 files changed, 69 insertions(+), 13 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 6cf98bc..1a6d339 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2012-04-19  Kjell Ahlstedt  <kjell ahlstedt bredband net>
+
+	Node: Fix memory problems in import_node().
+
+	* libxml++/nodes/node.[h|cc]: Return added_node instead of imported_node,
+	which libxml2 may delete. Delete the C++ wrapper of a deleted attribute node.
+	* examples/import_node/example[1|2].xml:
+	* examples/import_node/main.cc: Import attributes and a text node which is
+	merged with an existing text node. Bug #672992.
+
 2012-04-12  Kjell Ahlstedt  <kjell ahlstedt bredband net>
 
 	Define LIBXMLCPP_EXCEPTIONS_ENABLED unconditionally.
diff --git a/examples/import_node/example1.xml b/examples/import_node/example1.xml
index d8ead55..febf49d 100644
--- a/examples/import_node/example1.xml
+++ b/examples/import_node/example1.xml
@@ -1,6 +1,6 @@
 <?xml version="1.0"?>
-<root>
-<child>content</child>
+<root name="example1">
+<child>content </child>
 <child>more content</child>
 <child>even more content</child>
 </root>
diff --git a/examples/import_node/example2.xml b/examples/import_node/example2.xml
index c4f030b..adf538b 100644
--- a/examples/import_node/example2.xml
+++ b/examples/import_node/example2.xml
@@ -1,5 +1,5 @@
 <?xml version="1.0"?>
-<root>
+<root name="example2" type="example">
 <child>added content from other document
 <grandchild>grand child content</grandchild> 
 </child>
diff --git a/examples/import_node/main.cc b/examples/import_node/main.cc
index 22abfb4..d6d5f30 100644
--- a/examples/import_node/main.cc
+++ b/examples/import_node/main.cc
@@ -18,19 +18,41 @@ int main (int /* argc */, char** /* argv */)
     DomParser example1("example1.xml");
     DomParser example2("example2.xml");
     
-    Document *doc1 = example1.get_document();
-    Document *doc2 = example2.get_document();
+    Document* doc1 = example1.get_document();
+    Document* doc2 = example2.get_document();
     
-    Element *root1 = doc1->get_root_node();
-    Element *root2 = doc2->get_root_node();
+    Element* root1 = doc1->get_root_node();
+    Element* root2 = doc2->get_root_node();
 
     // find the first "child" element in example2
-    Node::NodeList child_list = root2->get_children("child");
-    Node *node_to_add = child_list.front();
+    Node::NodeList child_list2 = root2->get_children("child");
+    Node* node_to_add = child_list2.front();
 
     // import the node under the root element (recursive is default)
     root1->import_node(node_to_add);
+
+    // Import an attribute that will replace an existing attribute in the root element.
+    Attribute* attribute_to_add = root2->get_attribute("name");
+    root1->import_node(attribute_to_add);
+    
+    // Import an attribute that will be added to the root element.
+    attribute_to_add = root2->get_attribute("type");
+    root1->import_node(attribute_to_add);
     
+    // Find the first text child of the first "child" element in example2.
+    Element* first_child2 = dynamic_cast<Element*>(child_list2.front());
+    if (!first_child2)
+    {
+      cout << "first_child2 == 0" << endl;
+      return EXIT_FAILURE;
+    }
+    TextNode* text_to_add = first_child2->get_child_text();
+
+    // Import the text under the first "child" element in example1.
+    // Adjacent text nodes are merged.
+    Node* first_child1 = root1->get_first_child("child");
+    first_child1->import_node(text_to_add);
+
     // print out the new doc1
     string doc1_string = doc1->write_to_string_formatted();
     cout << doc1_string;
diff --git a/libxml++/nodes/node.cc b/libxml++/nodes/node.cc
index b00af2e..b08d81d 100644
--- a/libxml++/nodes/node.cc
+++ b/libxml++/nodes/node.cc
@@ -231,8 +231,22 @@ Node* Node::import_node(const Node* node, bool recursive)
     throw exception("Unable to import node");
   }
 
+  // If the copied node is an attribute node, and there is an attribute with
+  // the same name, the old attribute is destroyed (freed). If it's an attribute,
+  // Node::free_wrappers() is registered as a callback that will delete the
+  // C++ wrapper before the C object is deleted.
+  xmlDeregisterNodeFunc old_callback = 0;
+  const bool register_callback = imported_node->type == XML_ATTRIBUTE_NODE;
+  if (register_callback)
+    old_callback = xmlDeregisterNodeDefault(Node::free_wrappers);
+
   //Add the node:
-  xmlNode* added_node = xmlAddChild(this->cobj(),imported_node);
+  xmlNode* added_node = xmlAddChild(this->cobj(), imported_node);
+
+  // Remove the free_wrappers() callback and reinsert old callback function, if any.
+  if (register_callback)
+    xmlDeregisterNodeDefault(old_callback);
+
   if (!added_node)
   {
     Node::free_wrappers(imported_node);
@@ -241,8 +255,11 @@ Node* Node::import_node(const Node* node, bool recursive)
     throw exception("Unable to add imported node to current node");
   }
 
-  Node::create_wrapper(imported_node);
-  return static_cast<Node*>(imported_node->_private);
+  // Usually added_node == imported_node, but a text node is merged with an
+  // adjacent text node. In that case, xmlAddChild() frees imported_node, and
+  // added_node is a pointer to the old text node.
+  Node::create_wrapper(added_node);
+  return static_cast<Node*>(added_node->_private);
 }
 
 Glib::ustring Node::get_name() const
diff --git a/libxml++/nodes/node.h b/libxml++/nodes/node.h
index 06d2753..746bab2 100644
--- a/libxml++/nodes/node.h
+++ b/libxml++/nodes/node.h
@@ -163,9 +163,16 @@ public:
   void remove_child(Node* node);
 
   /** Import node(s) from another document under this node, without affecting the source node.
+   *
+   * If the imported node is an attribute node, and this node has an attribute with
+   * the same name as the imported attribute, the existing attribute is destroyed
+   * before the imported attribute is added. Any pointer to a destroyed attribute
+   * node becomes invalid.
+   *
    * @param node The node to copy and insert under the current node.
    * @param recursive Whether to import the child nodes also. Defaults to true.
-   * @returns The newly-created node.
+   * @returns Usually the newly created node, but adjacent text nodes are merged,
+   *          and the old text node with merged contents is returned.
    * @throws exception
    */
   Node* import_node(const Node* node, bool recursive = true);