Hi,
I'd like to discuss a possible improvement of the API.
To illustrate my idea I'll take the example of the Element class, but
all nodes are concerned.
Currently, the lifetime of nodes is controlled by libxml2, via callbacks
on node ceation/deletion. The restrictions are that we cannot create a
standalone node, nor detach and then delete a node like a normal class
instance.
I think there is a solution which, without breaking the current
behavior, would permit such things. I describe the deletion first
because it's more simple.
0. Example
With this modifications we could something like :
Document doc;
// ...
Element * e = <any node of the document>
e->detach(); // detach the node from the tree
e->attach( new Element("achild") );
delete e;
1. Deletion
Currently the destructor of Node does not release the underlying C
structure, because it is supposed to be deleted from the callback
"on_libxml_destruct".
To allow a safe deletion, we have to release the structure, but ensure
that the callback will not delete a second time the object. This can be
done by setting impl_->_private to 0 in Element destructor (Node
destructor would probably be ok too).
The problem is that a destruction initiated by libxml2 would release
twice the C structure. To avoid that, we can reset the Node::impl_ field
before deleting the object. We would have then :
Node::~Node()
{
if( impl_ )
{
imp_->_private = 0;
xmlFreeNode(impl_);
}
}
on_libxml_destruct(xmlNode* node)
{
if( node->_private = 0 )
{
return;
}
// ...
118 else
119 {
120 xmlpp::Node* cppNode =
static_cast<xmlpp::Node*>(node->_private);
121 if(cppNode)
122 {
> cppNode->impl_ = 0;
123 delete cppNode;
124 bPrivateDeleted = true;
125 }
126 }
127
}
2. Instanciation
The instanciation is a bit more complex because we can't flag anything
to tell the create callback that no node needs to be instanciated. My
idea is to deactivate the callback during the node instanciation, if the
default constructor is called. The callback registration in libxml2
being thread specific it should be safe to do so.
The implementation would be something like this :
/// This class deactivate the callback in the constructor,
/// and reactive it in its destructor
class ScopedCallbackDeactivator {
// ...
};
/// Static function
xmlNode * Element::initXmlNode(Glib::ustring const & name)
{
ScopedCallbackDeactivator cd;
return xmlNewNode((const xmlChar *)name.c_str());
}
Element::Element(Glib::ustring const & name)
: impl_( initXmlNode(name) )
{
impl_->_private = static_cast<Node*>(this);
}
To be consistent, this technic could be used for destruction.
Comments / critics are welcome, before I eventually produce a patch.
Regards,
Christophe
|
