GNOME.org
 

[libxml++] Moved create_wrapper() and free_wrappers() to Node.

commit 95243eee9837ab6843a20fa43a6a1ef41d1fcd62
Author: Murray Cumming <murrayc murrayc com>
Date:   Sun Nov 14 16:44:36 2010 +0100

    Moved create_wrapper() and free_wrappers() to Node.
    
    * libxml++/document.[h|cc]:
    * libxml++/nodes/node.[h|cc]: Moved create_wrapper() and free_wrappers()
    to here from Document.
    free_wrappers(): Never return inside the switch/case, so we check
    xmlNode::properties for all struct types, and to avoid making the behaviour
    non-obvious.
    * libxml++/parsers/textreader.cc:
    * libxml++/validators/dtdvalidator.cc:
    * libxml++/nodes/element.cc: Adapted.

 ChangeLog                           |   31 ++++++++
 libxml++/document.cc                |  118 +-----------------------------
 libxml++/document.h                 |    4 -
 libxml++/nodes/element.cc           |   20 +++---
 libxml++/nodes/node.cc              |  136 ++++++++++++++++++++++++++++++++---
 libxml++/nodes/node.h               |   16 ++++
 libxml++/parsers/textreader.cc      |    6 +-
 libxml++/validators/dtdvalidator.cc |    6 +-
 8 files changed, 191 insertions(+), 146 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index fb88441..bd1e424 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,34 @@
+2010-11-14  Murray Cumming  <murrayc murrayc com>
+
+	Moved create_wrapper() and free_wrappers() to Node.
+
+	* libxml++/document.[h|cc]:
+	* libxml++/nodes/node.[h|cc]: Moved create_wrapper() and free_wrappers() 
+	to here from Document.
+	free_wrappers(): Never return inside the switch/case, so we check 
+	xmlNode::properties for all struct types, and to avoid making the behaviour 
+	non-obvious.
+	* libxml++/parsers/textreader.cc:
+	* libxml++/validators/dtdvalidator.cc:
+	* libxml++/nodes/element.cc: Adapted.
+	
+2010-11-08  Alessandro Pignotti <a pignotti sssup it>
+
+	Make libxml++ compatible with separate and multi-threaded libxml2 usage.
+
+	* libxml++/document.[h|cc]: Added create_wrapper() and free_wrappers(),
+	replacing on_libxml_construct() and on_libxml_destruct() .
+	Init(): Do not register these global callbacks with libxml.
+	* libxml++/nodes/element.cc:
+	* libxml++/nodes/node.[h|cc]:
+	* libxml++/parsers/textreader.cc:
+	* libxml++/validators/dtdvalidator.cc: Call these create_wrapper() before 
+	ever trying to get a C++ instance from a C instance. Call free_wrappers() 
+	in destructors and other places where we want the instance to be destroyed.
+	
+	This avoids use of libxml's global function pointers, which are not 
+	thread-safe.
+
 2010-11-08  Murray Cumming  <murrayc murrayc com>
 
 	Do not call xmlCleanupParser() because it is brutal.
diff --git a/libxml++/document.cc b/libxml++/document.cc
index 2056d3e..df159fb 100644
--- a/libxml++/document.cc
+++ b/libxml++/document.cc
@@ -11,95 +11,17 @@
 #include <libxml++/dtd.h>
 #include <libxml++/attribute.h>
 #include <libxml++/nodes/element.h>
-#include <libxml++/nodes/entityreference.h>
-#include <libxml++/nodes/textnode.h>
-#include <libxml++/nodes/commentnode.h>
-#include <libxml++/nodes/cdatanode.h>
-#include <libxml++/nodes/processinginstructionnode.h>
 #include <libxml++/exceptions/internal_error.h>
 #include <libxml++/keepblanks.h>
 #include <libxml++/io/ostreamoutputbuffer.h>
 
 #include <libxml/tree.h>
 
-#include <assert.h>
-
 #include <iostream>
 
 namespace xmlpp
 {
 
-void Document::create_wrapper(xmlNode* node)
-{
-  if(node->_private)
-  {
-	  //Node already wrapped, skip
-	  return;
-  }
-
-  switch (node->type)
-  {
-    case XML_ELEMENT_NODE:
-    {
-      node->_private = new xmlpp::Element(node);
-      break;
-    }
-    case XML_ATTRIBUTE_NODE:
-    {
-      node->_private = new xmlpp::Attribute(node);
-      break;
-    }
-    case XML_TEXT_NODE:
-    {
-      node->_private = new xmlpp::TextNode(node);
-      break;
-    }
-    case XML_COMMENT_NODE:
-    {
-      node->_private = new xmlpp::CommentNode(node);
-      break;
-    }
-    case XML_CDATA_SECTION_NODE:
-    {
-      node->_private = new xmlpp::CdataNode(node);
-      break;
-    }
-    case XML_PI_NODE:
-    {
-      node->_private = new xmlpp::ProcessingInstructionNode(node);
-      break;
-    }
-    case XML_DTD_NODE:
-    {
-      node->_private = new xmlpp::Dtd(reinterpret_cast<xmlDtd*>(node));
-      break;
-    }
-    //case XML_ENTITY_NODE:
-    //{
-    //  assert(0 && "Warning: XML_ENTITY_NODE not implemented");
-    //  //node->_private = new xmlpp::ProcessingInstructionNode(node);
-    //  break;
-    //}
-    case XML_ENTITY_REF_NODE:
-    {
-      node->_private = new xmlpp::EntityReference(node);
-      break;
-    }
-    case XML_DOCUMENT_NODE:
-    {
-      // do nothing ! in case of documents it's the wrapper that is the owner
-      break;
-    }
-    default:
-    {
-      // good default for release versions
-      node->_private = new xmlpp::Node(node);
-      assert(0 && "Warning: new node of unknown type created");
-      break;
-    }
-  }
-}
-
 Document::Init::Init()
 {
   xmlInitParser(); //Not always necessary, but necessary for thread safety.
@@ -135,44 +57,10 @@ Document::Document(xmlDoc* doc)
 
 Document::~Document()
 {
-  free_wrappers(reinterpret_cast<xmlNode*>(impl_));
+  Node::free_wrappers(reinterpret_cast<xmlNode*>(impl_));
   xmlFreeDoc(impl_);
 }
 
-void Document::free_wrappers(xmlNode* node)
-{
-  //Walk the children list
-  for(xmlNode* child=node->children; child; child=child->next)
-     free_wrappers(child);
-
-  //Delete the local one
-  switch(node->type)
-  {
-    //Node types that have no properties
-    case XML_DTD_NODE:
-      delete static_cast<Dtd*>(node->_private);
-      node->_private=0;
-      return;
-    case XML_ATTRIBUTE_NODE:
-    case XML_ELEMENT_DECL:
-    case XML_ATTRIBUTE_DECL:
-    case XML_ENTITY_DECL:
-      delete static_cast<Node*>(node->_private);
-      node->_private=0;
-      return;
-    case XML_DOCUMENT_NODE:
-      //Do not free now, the ownernship is reversed
-      return;
-    default:
-      delete static_cast<Node*>(node->_private);
-      node->_private=0;
-  }
-
-  //Walk the attributes list
-  for(xmlAttr* attr=node->properties; attr; attr=attr->next)
-     free_wrappers(reinterpret_cast<xmlNode*>(attr));
-}
-
 Glib::ustring Document::get_encoding() const
 {
   Glib::ustring encoding;
@@ -214,7 +102,7 @@ Element* Document::get_root_node() const
     return 0;
   else
   {
-    create_wrapper(root);
+    Node::create_wrapper(root);
     return reinterpret_cast<Element*>(root->_private);
   }
 }
@@ -270,7 +158,7 @@ CommentNode* Document::add_comment(const Glib::ustring& content)
 
   // Use the result, because node can be freed when merging text nodes:
   node = xmlAddChild( (xmlNode*)impl_, node);
-  create_wrapper(node);
+  Node::create_wrapper(node);
   return static_cast<CommentNode*>(node->_private);
 }
 
diff --git a/libxml++/document.h b/libxml++/document.h
index 0ed9e7b..c30068c 100644
--- a/libxml++/document.h
+++ b/libxml++/document.h
@@ -170,10 +170,6 @@ public:
   ///Access the underlying libxml implementation.
   const _xmlDoc* cobj() const;
 
-  ///Construct the right C++ instances for a given element
-  static void create_wrapper(_xmlNode* node);
-  ///Recursively destroy the created C++ instances
-  static void free_wrappers(_xmlNode* attr);
 protected:
   /** Retrieve an Entity.
    * The entity can be from an external subset or internally declared.
diff --git a/libxml++/nodes/element.cc b/libxml++/nodes/element.cc
index e275e71..1b9cf96 100644
--- a/libxml++/nodes/element.cc
+++ b/libxml++/nodes/element.cc
@@ -26,7 +26,7 @@ Element::AttributeList Element::get_attributes()
   AttributeList attributes;
   for(xmlAttr* attr = cobj()->properties; attr; attr = attr->next)
   {
-    Document::create_wrapper(reinterpret_cast<xmlNode*>(attr));
+    Node::create_wrapper(reinterpret_cast<xmlNode*>(attr));
     attributes.push_back(reinterpret_cast<Attribute*>(attr->_private));
   }
 
@@ -46,7 +46,7 @@ Attribute* Element::get_attribute(const Glib::ustring& name,
     xmlAttr* attr = xmlHasProp(const_cast<xmlNode*>(cobj()), (const xmlChar*)name.c_str());
     if( attr )
     {
-      Document::create_wrapper(reinterpret_cast<xmlNode*>(attr));
+      Node::create_wrapper(reinterpret_cast<xmlNode*>(attr));
       return reinterpret_cast<Attribute*>(attr->_private);
     }
   }
@@ -57,7 +57,7 @@ Attribute* Element::get_attribute(const Glib::ustring& name,
                                  (const xmlChar*)ns_uri.c_str());
     if( attr )
     {
-      Document::create_wrapper(reinterpret_cast<xmlNode*>(attr));
+      Node::create_wrapper(reinterpret_cast<xmlNode*>(attr));
       return reinterpret_cast<Attribute*>(attr->_private);
     }
   }
@@ -100,7 +100,7 @@ Attribute* Element::set_attribute(const Glib::ustring& name, const Glib::ustring
 
   if(attr)
   {
-    Document::create_wrapper(reinterpret_cast<xmlNode*>(attr));
+    Node::create_wrapper(reinterpret_cast<xmlNode*>(attr));
     return reinterpret_cast<Attribute*>(attr->_private);
   }
   else
@@ -125,7 +125,7 @@ const TextNode* Element::get_child_text() const
   for(xmlNode* child = cobj()->children; child; child = child->next)
      if(child->type == XML_TEXT_NODE)
      {
-       Document::create_wrapper(child);
+       Node::create_wrapper(child);
        return static_cast<TextNode*>(child->_private);
      }
 
@@ -139,7 +139,7 @@ TextNode* Element::get_child_text()
   for(xmlNode* child = cobj()->children; child; child = child->next)
      if(child->type == XML_TEXT_NODE)
      {
-       Document::create_wrapper(child);
+       Node::create_wrapper(child);
        return static_cast<TextNode*>(child->_private);
      }
 
@@ -164,7 +164,7 @@ TextNode* Element::add_child_text(const Glib::ustring& content)
      // Use the result, because node can be freed when merging text nodes:
      node = xmlAddChild(cobj(), node); 
 
-     Document::create_wrapper(node);
+     Node::create_wrapper(node);
      return static_cast<TextNode*>(node->_private);
   }
   return 0;
@@ -182,7 +182,7 @@ TextNode* Element::add_child_text(xmlpp::Node* previous_sibling, const Glib::ust
      // Use the result, because node can be freed when merging text nodes:
      node = xmlAddNextSibling(previous_sibling->cobj(), node); 
 
-     Document::create_wrapper(node);
+     Node::create_wrapper(node);
      return static_cast<TextNode*>(node->_private);
   }
   return 0;
@@ -200,7 +200,7 @@ TextNode* Element::add_child_text_before(xmlpp::Node* next_sibling, const Glib::
      // Use the result, because node can be freed when merging text nodes:
      node = xmlAddPrevSibling(next_sibling->cobj(), node); 
 
-     Document::create_wrapper(node);
+     Node::create_wrapper(node);
      return static_cast<TextNode*>(node->_private);
   }
   return 0;
@@ -242,7 +242,7 @@ CommentNode* Element::add_child_comment(const Glib::ustring& content)
  
   // Use the result, because node can be freed when merging text nodes:
   node = xmlAddChild(cobj(), node);
-  Document::create_wrapper(node);
+  Node::create_wrapper(node);
   return static_cast<CommentNode*>(node->_private);
 }
 
diff --git a/libxml++/nodes/node.cc b/libxml++/nodes/node.cc
index e9ba6dc..ab22bbf 100644
--- a/libxml++/nodes/node.cc
+++ b/libxml++/nodes/node.cc
@@ -6,6 +6,11 @@
 
 #include <libxml++/nodes/element.h>
 #include <libxml++/nodes/node.h>
+#include <libxml++/nodes/entityreference.h>
+#include <libxml++/nodes/textnode.h>
+#include <libxml++/nodes/commentnode.h>
+#include <libxml++/nodes/cdatanode.h>
+#include <libxml++/nodes/processinginstructionnode.h>
 #include <libxml++/exceptions/internal_error.h>
 #include <libxml++/document.h>
 #include <libxml/xpath.h>
@@ -36,7 +41,7 @@ Element* Node::get_parent()
   if(!(cobj()->parent && cobj()->parent->type == XML_ELEMENT_NODE))
     return 0;
 
-  Document::create_wrapper(cobj()->parent);
+  Node::create_wrapper(cobj()->parent);
   return static_cast<Element*>(cobj()->parent->_private);
 }
 
@@ -50,7 +55,7 @@ Node* Node::get_next_sibling()
   if(!cobj()->next)
     return 0;
 
-  Document::create_wrapper(cobj()->next);
+  Node::create_wrapper(cobj()->next);
   return static_cast<Node*>(cobj()->next->_private);
 }
 
@@ -64,7 +69,7 @@ Node* Node::get_previous_sibling()
   if(!cobj()->prev)
     return 0;
 
-  Document::create_wrapper(cobj()->prev);
+  Node::create_wrapper(cobj()->prev);
   return static_cast<Node*>(cobj()->prev->_private);
 }
 
@@ -79,7 +84,7 @@ Node::NodeList Node::get_children(const Glib::ustring& name)
    {
       if(name.empty() || name == (const char*)child->name)
       {
-        Document::create_wrapper(child);
+        Node::create_wrapper(child);
         children.push_back(reinterpret_cast<Node*>(child->_private));
       }
    }
@@ -104,7 +109,7 @@ Element* Node::add_child(const Glib::ustring& name,
   if(!node)
     return 0;
  
-  Document::create_wrapper(node);
+  Node::create_wrapper(node);
   return static_cast<Element*>(node->_private);
 }
 
@@ -123,7 +128,7 @@ Element* Node::add_child(xmlpp::Node* previous_sibling,
   if(!node)
     return 0;
 
-  Document::create_wrapper(node);
+  Node::create_wrapper(node);
   return static_cast<Element*>(node->_private);
 }
 
@@ -142,7 +147,7 @@ Element* Node::add_child_before(xmlpp::Node* next_sibling,
   if(!node)
     return 0;
 
-  Document::create_wrapper(node);
+  Node::create_wrapper(node);
   return static_cast<Element*>(node->_private);
 }
 
@@ -182,7 +187,7 @@ void Node::remove_child(Node* node)
 {
   //TODO: Allow a node to be removed without deleting it, to allow it to be moved?
   //This would require a more complex memory management API.
-  Document::free_wrappers(node->cobj());
+  Node::free_wrappers(node->cobj());
   xmlUnlinkNode(node->cobj());
   xmlFreeNode(node->cobj()); //The C++ instance will be deleted in a callback.
 }
@@ -204,7 +209,7 @@ Node* Node::import_node(const Node* node, bool recursive)
   xmlNode* added_node = xmlAddChild(this->cobj(),imported_node);
   if (!added_node)
   {
-    Document::free_wrappers(imported_node);
+    Node::free_wrappers(imported_node);
     xmlFreeNode(imported_node);
 
     #ifdef LIBXMLCPP_EXCEPTIONS_ENABLED
@@ -214,7 +219,7 @@ Node* Node::import_node(const Node* node, bool recursive)
     #endif //LIBXMLCPP_EXCEPTIONS_ENABLED
   }
 
-  Document::create_wrapper(imported_node);
+  Node::create_wrapper(imported_node);
   return static_cast<Node*>(imported_node->_private);
 }
 
@@ -298,7 +303,7 @@ static NodeSet find_impl(xmlXPathContext* ctxt, const Glib::ustring& xpath)
       
       //TODO: Check for other cnode->type values?
   
-      Document::create_wrapper(cnode);
+      Node::create_wrapper(cnode);
       Node* cppNode = static_cast<Node*>(cnode->_private);
       nodes.push_back(cppNode);
     }
@@ -392,5 +397,114 @@ void Node::set_namespace(const Glib::ustring& ns_prefix)
   }
 }
 
+void Node::create_wrapper(xmlNode* node)
+{
+  if(node->_private)
+  {
+	  //Node already wrapped, skip
+	  return;
+  }
+
+  switch (node->type)
+  {
+    case XML_ELEMENT_NODE:
+    {
+      node->_private = new xmlpp::Element(node);
+      break;
+    }
+    case XML_ATTRIBUTE_NODE:
+    {
+      node->_private = new xmlpp::Attribute(node);
+      break;
+    }
+    case XML_TEXT_NODE:
+    {
+      node->_private = new xmlpp::TextNode(node);
+      break;
+    }
+    case XML_COMMENT_NODE:
+    {
+      node->_private = new xmlpp::CommentNode(node);
+      break;
+    }
+    case XML_CDATA_SECTION_NODE:
+    {
+      node->_private = new xmlpp::CdataNode(node);
+      break;
+    }
+    case XML_PI_NODE:
+    {
+      node->_private = new xmlpp::ProcessingInstructionNode(node);
+      break;
+    }
+    case XML_DTD_NODE:
+    {
+      node->_private = new xmlpp::Dtd(reinterpret_cast<xmlDtd*>(node));
+      break;
+    }
+    //case XML_ENTITY_NODE:
+    //{
+    //  assert(0 && "Warning: XML_ENTITY_NODE not implemented");
+    //  //node->_private = new xmlpp::ProcessingInstructionNode(node);
+    //  break;
+    //}
+    case XML_ENTITY_REF_NODE:
+    {
+      node->_private = new xmlpp::EntityReference(node);
+      break;
+    }
+    case XML_DOCUMENT_NODE:
+    {
+      // do nothing. For Documents it's the wrapper that is the owner.
+      break;
+    }
+    default:
+    {
+      // good default for release versions
+      node->_private = new xmlpp::Node(node);
+      std::cerr << G_STRFUNC << "Warning: new node of unknown type created: " << node->type << std::endl;
+      break;
+    }
+  }
+}
+
+void Node::free_wrappers(xmlNode* node)
+{
+  if(!node)
+    return;
+    
+  //Walk the children list
+  for(xmlNode* child=node->children; child; child=child->next)
+    free_wrappers(child);
+
+  //Delete the local one
+  switch(node->type)
+  {
+    //Node types that have no properties
+    case XML_DTD_NODE:
+      delete static_cast<Dtd*>(node->_private);
+      node->_private = 0;
+      break;
+    case XML_ATTRIBUTE_NODE:
+    case XML_ELEMENT_DECL:
+    case XML_ATTRIBUTE_DECL:
+    case XML_ENTITY_DECL:
+      delete static_cast<Node*>(node->_private);
+      node->_private = 0;
+      break;
+    case XML_DOCUMENT_NODE:
+      //Do not free now. The Document is usually the one who owns the caller.
+      break;
+    default:
+      delete static_cast<Node*>(node->_private);
+      node->_private = 0;
+      break;
+  }
+
+  //Walk the attributes list
+  for(xmlAttr* attr = node->properties; attr; attr = attr->next)
+    free_wrappers(reinterpret_cast<xmlNode*>(attr));
+}
+
 
 } //namespace xmlpp
diff --git a/libxml++/nodes/node.h b/libxml++/nodes/node.h
index 088da4e..35dd97d 100644
--- a/libxml++/nodes/node.h
+++ b/libxml++/nodes/node.h
@@ -179,6 +179,22 @@ public:
   ///Access the underlying libxml implementation.
   const _xmlNode* cobj() const;
 
+  /** Construct the correct C++ instance for a given libxml C struct instance.
+   *
+   * This is only for use by the libxml++ implementation.
+   *
+   * @para node A pointer to an xmlNode or a "derived" struct, such as xmlDoc, xmlAttr, etc.
+   */
+  static void create_wrapper(_xmlNode* node);
+  
+  /** Delete the C++ instance for a given libxml C struct instance, and also 
+   * recursively destroy the C++ instances for any children.
+   *
+   * This is only for use by the libxml++ implementation.
+   * @para node A pointer to an xmlNode or a "derived" struct, such as xmlDoc, xmlAttr, etc.
+   */
+  static void free_wrappers(_xmlNode* attr);
+  
 protected:
 
   ///Create the C instance ready to be added to the parent node.
diff --git a/libxml++/parsers/textreader.cc b/libxml++/parsers/textreader.cc
index 8770771..c196d57 100644
--- a/libxml++/parsers/textreader.cc
+++ b/libxml++/parsers/textreader.cc
@@ -300,7 +300,7 @@ Node* TextReader::get_current_node()
   xmlNodePtr node = xmlTextReaderCurrentNode(impl_);
   if(node)
   {
-    Document::create_wrapper(node);
+    Node::create_wrapper(node);
     return static_cast<Node*>(node->_private);
   }
     
@@ -313,7 +313,7 @@ const Node* TextReader::get_current_node() const
   xmlNodePtr node = xmlTextReaderCurrentNode(impl_);
   if(node)
   {
-    Document::create_wrapper(node);
+    Node::create_wrapper(node);
     return static_cast<Node*>(node->_private);
   }
 
@@ -337,7 +337,7 @@ Node* TextReader::expand()
   if(node)
   if(node)
   {
-    Document::create_wrapper(node);
+    Node::create_wrapper(node);
     return static_cast<Node*>(node->_private);
   }
     
diff --git a/libxml++/validators/dtdvalidator.cc b/libxml++/validators/dtdvalidator.cc
index 9d9420a..5517720 100644
--- a/libxml++/validators/dtdvalidator.cc
+++ b/libxml++/validators/dtdvalidator.cc
@@ -68,7 +68,7 @@ void DtdValidator::parse_subset(const Glib::ustring& external,const Glib::ustrin
     #endif //LIBXMLCPP_EXCEPTIONS_ENABLED
   }
 
-  Document::create_wrapper(reinterpret_cast<xmlNode*>(dtd));
+  Node::create_wrapper(reinterpret_cast<xmlNode*>(dtd));
   dtd_ = static_cast<Dtd*>(dtd->_private);
 }
 
@@ -97,7 +97,7 @@ void DtdValidator::parse_stream(std::istream& in)
     #endif //LIBXMLCPP_EXCEPTIONS_ENABLED
   }
 
-  Document::create_wrapper(reinterpret_cast<xmlNode*>(dtd));
+  Node::create_wrapper(reinterpret_cast<xmlNode*>(dtd));
   dtd_ = static_cast<Dtd*>(dtd->_private);
 }
 
@@ -108,7 +108,7 @@ void DtdValidator::release_underlying()
     //Make a local copy as the wrapper is destroyed first
     //After free_wrappers is called dtd_ will be invalid (e.g. delete dtd_)
     xmlDtd* dtd=dtd_->cobj();
-    Document::free_wrappers(reinterpret_cast<xmlNode*>(dtd));
+    Node::free_wrappers(reinterpret_cast<xmlNode*>(dtd));
     xmlFreeDtd(dtd);
     dtd_ = 0;
   }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]