From: JINMEI Tatuya Date: Tue, 21 Aug 2012 07:05:41 +0000 (-0700) Subject: [2107] revised tree-data deleter mechanism: we now pass deleter to destroy. X-Git-Tag: trac2351_base~109^2~1^2~35 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6679f424fbcf59fee039531ef213eb1f8488fcff;p=thirdparty%2Fkea.git [2107] revised tree-data deleter mechanism: we now pass deleter to destroy. ...instead of give its type as a class template parameter and instantiate it within the tree. it won't work (well) for ZoneData, because we need to know the RR class to delete RdataSet (which is the data type for ZoneData tree), but there's no way to tell the RR class about it. A good side effect of this change is that we now don't have to link the memory segment for the tree and its nodes with the zone data (although in practice it's quite likely to use the same segment for all of them). I also updated setData() (further from #2100); it now doesn't even delete any existing data. This is partly relaetd to the above change on deleter - we cannot instantiate the deleter within this method. We could pass the deleter as a parameter, but, actually, in the case of ZoneData we wouldn't have to delete the data; when setData() encounters existing data, it's more likely that ZoneData tries to extend the RdataSet list. It makes more sense to let ZoneData manage all data, including when to delete them. (although not absolutely necessary) I also changed the type of deleter in the domain tree test to highlight we could use a plain old function instead of a functor object. change size is big, but most of the changes are trivial conversion from something like to . --- diff --git a/src/lib/datasrc/memory/domaintree.h b/src/lib/datasrc/memory/domaintree.h index 0c8d14693e..bb7bbae4e4 100644 --- a/src/lib/datasrc/memory/domaintree.h +++ b/src/lib/datasrc/memory/domaintree.h @@ -44,7 +44,7 @@ namespace memory { /// Forward declare DomainTree class here is convinent for following /// friend class declare inside DomainTreeNode and DomainTreeNodeChain -template +template class DomainTree; /// \brief \c DomainTreeNode is used by DomainTree to store any data @@ -79,18 +79,18 @@ class DomainTree; /// immediately following the main node object. The size of the /// allocated space for the labels data is encoded by borrowing some /// bits of the "flags" field. -template +template class DomainTreeNode : public boost::noncopyable { private: /// The DomainTreeNode is meant for use from within DomainTree, so /// it has access to it. - friend class DomainTree; + friend class DomainTree; /// \brief Just a type alias /// /// We are going to use a lot of these offset pointers here and they /// have a long name. - typedef boost::interprocess::offset_ptr > + typedef boost::interprocess::offset_ptr > DomainTreeNodePtr; /// \name Constructors @@ -131,12 +131,12 @@ private: /// /// \param mem_sgmt A \c MemorySegment from which memory for the new /// \c DomainTreeNode is allocated. - static DomainTreeNode* create(util::MemorySegment& mem_sgmt, - const dns::LabelSequence& labels) + static DomainTreeNode* create(util::MemorySegment& mem_sgmt, + const dns::LabelSequence& labels) { const size_t labels_len = labels.getSerializedLength(); - void* p = mem_sgmt.allocate(sizeof(DomainTreeNode) + labels_len); - DomainTreeNode* node = new(p) DomainTreeNode(labels_len); + void* p = mem_sgmt.allocate(sizeof(DomainTreeNode) + labels_len); + DomainTreeNode* node = new(p) DomainTreeNode(labels_len); labels.serialize(node->getLabelsData(), labels_len); return (node); } @@ -151,11 +151,12 @@ private: /// that was originally created by the \c create() method (the behavior /// is undefined if this condition isn't met). static void destroy(util::MemorySegment& mem_sgmt, - DomainTreeNode* node) { + DomainTreeNode* node) + { const size_t labels_capacity = node->labels_capacity_; - node->~DomainTreeNode(); + node->~DomainTreeNode(); mem_sgmt.deallocate(node, - sizeof(DomainTreeNode) + labels_capacity); + sizeof(DomainTreeNode) + labels_capacity); } /// \brief Reset node's label sequence to a new one. @@ -245,22 +246,14 @@ public: /// \name Setter functions. //@{ - /// \brief Set the data stored in the node. If there is old data, it - /// is either returned or destroyed based on what is passed in \c - /// old_data. - /// \param mem_sgmt The \c MemorySegment that allocated memory for - /// the node data. + /// \brief Set the data stored in the node. + /// + /// If there is old data, it will be simply dropped; unless the data + /// is managed outside the node and its resource is released (if needed), + /// it will leak. + /// /// \param data The new data to set. - /// \param old_data If \c NULL is passed here, any old data is - /// destroyed. Otherwise, the old data is returned - /// in this location. - void setData(util::MemorySegment& mem_sgmt, T* data, T** old_data = NULL) { - if (old_data != NULL) { - *old_data = data; - } else { - const DT deleter; - deleter(mem_sgmt, data_.get()); - } + void setData(T* data) { data_ = data; } //@} @@ -313,7 +306,7 @@ public: private: /// \name Callback related methods /// - /// See the description of \c DomainTree::find() at \ref callback + /// See the description of \c DomainTree::find() at \ref callback /// about callbacks. /// /// These methods never throw an exception. @@ -364,7 +357,7 @@ public: /// (which should be absolute), it will return \c NULL. /// /// This method never throws an exception. - const DomainTreeNode* getUpperNode() const; + const DomainTreeNode* getUpperNode() const; private: /// \brief return the next node which is bigger than current node @@ -382,7 +375,7 @@ private: /// returns \c NULL. /// /// This method never throws an exception. - const DomainTreeNode* successor() const; + const DomainTreeNode* successor() const; /// \brief return the next node which is smaller than current node /// in the same subtree @@ -399,7 +392,7 @@ private: /// returns \c NULL. /// /// This method never throws an exception. - const DomainTreeNode* predecessor() const; + const DomainTreeNode* predecessor() const; /// \brief private shared implementation of successor and predecessor /// @@ -412,11 +405,11 @@ private: /// The overhead of the member pointers should be optimised out, as this /// will probably get completely inlined into predecessor and successor /// methods. - const DomainTreeNode* - abstractSuccessor(typename DomainTreeNode::DomainTreeNodePtr - DomainTreeNode::*left, - typename DomainTreeNode::DomainTreeNodePtr - DomainTreeNode::*right) + const DomainTreeNode* + abstractSuccessor(typename DomainTreeNode::DomainTreeNodePtr + DomainTreeNode::*left, + typename DomainTreeNode::DomainTreeNodePtr + DomainTreeNode::*right) const; /// \name Data to maintain the rbtree structure. @@ -429,29 +422,29 @@ private: //@{ DomainTreeNodePtr parent_; /// \brief Access the parent_ as bare pointer. - DomainTreeNode* getParent() { + DomainTreeNode* getParent() { return (parent_.get()); } /// \brief Access the parent_ as bare pointer, const. - const DomainTreeNode* getParent() const { + const DomainTreeNode* getParent() const { return (parent_.get()); } DomainTreeNodePtr left_; /// \brief Access the left_ as bare pointer. - DomainTreeNode* getLeft() { + DomainTreeNode* getLeft() { return (left_.get()); } /// \brief Access the left_ as bare pointer, const. - const DomainTreeNode* getLeft() const { + const DomainTreeNode* getLeft() const { return (left_.get()); } DomainTreeNodePtr right_; /// \brief Access the right_ as bare pointer. - DomainTreeNode* getRight() { + DomainTreeNode* getRight() { return (right_.get()); } /// \brief Access the right_ as bare pointer, const. - const DomainTreeNode* getRight() const { + const DomainTreeNode* getRight() const { return (right_.get()); } //@} @@ -467,11 +460,11 @@ private: /// avoiding storage of the same domain labels multiple times. DomainTreeNodePtr down_; /// \brief Access the down_ as bare pointer. - DomainTreeNode* getDown() { + DomainTreeNode* getDown() { return (down_.get()); } /// \brief Access the down_ as bare pointer, const. - const DomainTreeNode* getDown() const { + const DomainTreeNode* getDown() const { return (down_.get()); } @@ -498,8 +491,8 @@ private: BOOST_STATIC_ASSERT((1 << 9) > dns::LabelSequence::MAX_SERIALIZED_LENGTH); }; -template -DomainTreeNode::DomainTreeNode(size_t labels_capacity) : +template +DomainTreeNode::DomainTreeNode(size_t labels_capacity) : parent_(NULL), left_(NULL), right_(NULL), @@ -510,14 +503,14 @@ DomainTreeNode::DomainTreeNode(size_t labels_capacity) : { } -template -DomainTreeNode::~DomainTreeNode() { +template +DomainTreeNode::~DomainTreeNode() { } -template -const DomainTreeNode* -DomainTreeNode::getUpperNode() const { - const DomainTreeNode* current = this; +template +const DomainTreeNode* +DomainTreeNode::getUpperNode() const { + const DomainTreeNode* current = this; // current would never be equal to NULL here (in a correct tree // implementation) @@ -528,12 +521,11 @@ DomainTreeNode::getUpperNode() const { return (current->getParent()); } -template -const DomainTreeNode* -DomainTreeNode::abstractSuccessor(typename DomainTreeNode::DomainTreeNodePtr - DomainTreeNode::*left, - typename DomainTreeNode::DomainTreeNodePtr - DomainTreeNode::*right) +template +const DomainTreeNode* +DomainTreeNode::abstractSuccessor( + typename DomainTreeNode::DomainTreeNodePtr DomainTreeNode::*left, + typename DomainTreeNode::DomainTreeNodePtr DomainTreeNode::*right) const { // This function is written as a successor. It becomes predecessor if @@ -541,12 +533,12 @@ DomainTreeNode::abstractSuccessor(typename DomainTreeNode::DomainT // the left pointer points to right and vice versa. Don't get confused // by the idea, just imagine the pointers look into a mirror. - const DomainTreeNode* current = this; + const DomainTreeNode* current = this; // If it has right node, the successor is the left-most node of the right // subtree. if ((current->*right).get() != NULL) { current = (current->*right).get(); - const DomainTreeNode* left_n; + const DomainTreeNode* left_n; while ((left_n = (current->*left).get()) != NULL) { current = left_n; } @@ -556,7 +548,7 @@ DomainTreeNode::abstractSuccessor(typename DomainTreeNode::DomainT // Otherwise go up until we find the first left branch on our path to // root. If found, the parent of the branch is the successor. // Otherwise, we return the null node - const DomainTreeNode* parent = current->getParent(); + const DomainTreeNode* parent = current->getParent(); while ((!current->isSubTreeRoot()) && (current == (parent->*right).get())) { current = parent; @@ -570,19 +562,19 @@ DomainTreeNode::abstractSuccessor(typename DomainTreeNode::DomainT } } -template -const DomainTreeNode* -DomainTreeNode::successor() const { - return (abstractSuccessor(&DomainTreeNode::left_, - &DomainTreeNode::right_)); +template +const DomainTreeNode* +DomainTreeNode::successor() const { + return (abstractSuccessor(&DomainTreeNode::left_, + &DomainTreeNode::right_)); } -template -const DomainTreeNode* -DomainTreeNode::predecessor() const { +template +const DomainTreeNode* +DomainTreeNode::predecessor() const { // Swap the left and right pointers for the abstractSuccessor - return (abstractSuccessor(&DomainTreeNode::right_, - &DomainTreeNode::left_)); + return (abstractSuccessor(&DomainTreeNode::right_, + &DomainTreeNode::left_)); } /// \brief DomainTreeNodeChain stores detailed information of \c @@ -619,11 +611,11 @@ DomainTreeNode::predecessor() const { /// DomainTree. /// This is the reason why manipulation methods such as \c push() and \c pop() /// are private (and not shown in the doxygen document). -template +template class DomainTreeNodeChain { /// DomainTreeNodeChain is initialized by DomainTree, only DomainTree has /// knowledge to manipulate it. - friend class DomainTree; + friend class DomainTree; public: /// \name Constructors and Assignment Operator. /// @@ -643,8 +635,8 @@ public: {} private: - DomainTreeNodeChain(const DomainTreeNodeChain&); - DomainTreeNodeChain& operator=(const DomainTreeNodeChain&); + DomainTreeNodeChain(const DomainTreeNodeChain&); + DomainTreeNodeChain& operator=(const DomainTreeNodeChain&); //@} public: @@ -672,7 +664,7 @@ public: /// tree is empty), this method returns \c NULL. /// /// \exception None - const DomainTreeNode* getLastComparedNode() const { + const DomainTreeNode* getLastComparedNode() const { return (last_compared_); } @@ -712,7 +704,7 @@ public: "called on an empty chain"); } - const DomainTreeNode* top_node = top(); + const DomainTreeNode* top_node = top(); isc::dns::Name absolute_name = top_node->getName(); int node_count = node_count_ - 1; while (node_count > 0) { @@ -740,7 +732,7 @@ private: /// root node of DomainTree /// /// \exception None - const DomainTreeNode* top() const { + const DomainTreeNode* top() const { assert(!isEmpty()); return (nodes_[node_count_ - 1]); } @@ -763,7 +755,7 @@ private: /// otherwise the node should be the root node of DomainTree. /// /// \exception None - void push(const DomainTreeNode* node) { + void push(const DomainTreeNode* node) { assert(node_count_ < RBT_MAX_LEVEL); nodes_[node_count_++] = node; } @@ -775,8 +767,8 @@ private: const static int RBT_MAX_LEVEL = isc::dns::Name::MAX_LABELS; int node_count_; - const DomainTreeNode* nodes_[RBT_MAX_LEVEL]; - const DomainTreeNode* last_compared_; + const DomainTreeNode* nodes_[RBT_MAX_LEVEL]; + const DomainTreeNode* last_compared_; isc::dns::NameComparisonResult last_comparison_; }; @@ -811,12 +803,8 @@ private: * the \c insert() method will still return \c ALREADYEXISTS regardless of * the search policy. * - * The template parameters taken by \c DomainTree are \c T (the type of - * data which is stored by the tree) and \c DT (a type whose instance is - * used to destroy data stored in the tree). operator() is - * called on a \c DT instance and passed a pointer to the data - * (T*) to be destroyed. This method should be written to - * accept \c NULL arguments. + * The template parameters taken by \c DomainTree is \c T (the type of + * data which is stored by the tree). * * \anchor diagram * @@ -852,9 +840,9 @@ private: * \todo * - add remove interface */ -template +template class DomainTree : public boost::noncopyable { - friend class DomainTreeNode; + friend class DomainTreeNode; public: /// \brief The return value for the \c find() and insert() methods enum Result { @@ -880,8 +868,8 @@ public: static DomainTree* create(util::MemorySegment& mem_sgmt, bool return_empty_node = false) { - void* p = mem_sgmt.allocate(sizeof(DomainTree)); - return (new(p) DomainTree(return_empty_node)); + void* p = mem_sgmt.allocate(sizeof(DomainTree)); + return (new(p) DomainTree(return_empty_node)); } /// \brief Destruct and deallocate \c DomainTree @@ -889,6 +877,12 @@ public: /// This method also destroys and deallocates all nodes inserted to the /// tree. /// + /// The template parameter, \c DataDeleter, is a type whose instance is + /// used to destroy data stored in the tree nodes. It must have a + /// operator() method, which is called on a \c DataDeleter + /// instance and passed a pointer to the data (T*) to be + /// destroyed. This method should be written to accept \c NULL arguments. + /// /// \note The memory segment (\c mem_sgmt) must be the same one that /// was originally used to allocate memory for the tree (and for all /// nodes inserted to the tree, due to the requirement of \c insert()), @@ -909,11 +903,15 @@ public: /// \param tree A non NULL pointer to a valid \c DomainTree object /// that was originally created by the \c create() method (the behavior /// is undefined if this condition isn't met). + /// \param deleter A deleter fanctor or function to delete node data. + template static void destroy(util::MemorySegment& mem_sgmt, - DomainTree* tree) { - tree->deleteAllNodes(mem_sgmt); - tree->~DomainTree(); - mem_sgmt.deallocate(tree, sizeof(DomainTree)); + DomainTree* tree, + DataDeleter deleter) + { + tree->deleteAllNodes(mem_sgmt, deleter); + tree->~DomainTree(); + mem_sgmt.deallocate(tree, sizeof(DomainTree)); } private: @@ -991,8 +989,8 @@ public: /// /// Acts as described in the \ref find section. Result find(const isc::dns::Name& name, - DomainTreeNode** node) const { - DomainTreeNodeChain node_path; + DomainTreeNode** node) const { + DomainTreeNodeChain node_path; const isc::dns::LabelSequence ls(name); return (find(ls, node, node_path, NULL, NULL)); } @@ -1002,9 +1000,9 @@ public: /// Acts as described in the \ref find section, but returns immutable node /// pointer. Result find(const isc::dns::Name& name, - const DomainTreeNode** node) const { - DomainTreeNodeChain node_path; - DomainTreeNode *target_node = NULL; + const DomainTreeNode** node) const { + DomainTreeNodeChain node_path; + DomainTreeNode *target_node = NULL; const isc::dns::LabelSequence ls(name); Result ret = (find(ls, &target_node, node_path, NULL, NULL)); if (ret != NOTFOUND) { @@ -1016,8 +1014,8 @@ public: /// \brief Simple find, with node_path tracking /// /// Acts as described in the \ref find section. - Result find(const isc::dns::Name& name, DomainTreeNode** node, - DomainTreeNodeChain& node_path) const + Result find(const isc::dns::Name& name, DomainTreeNode** node, + DomainTreeNodeChain& node_path) const { const isc::dns::LabelSequence ls(name); return (find(ls, node, node_path, NULL, NULL)); @@ -1027,10 +1025,10 @@ public: /// /// Acts as described in the \ref find section, but returns immutable node /// pointer. - Result find(const isc::dns::Name& name, const DomainTreeNode** node, - DomainTreeNodeChain& node_path) const + Result find(const isc::dns::Name& name, const DomainTreeNode** node, + DomainTreeNodeChain& node_path) const { - DomainTreeNode *target_node = NULL; + DomainTreeNode *target_node = NULL; const isc::dns::LabelSequence ls(name); Result ret = (find(ls, &target_node, node_path, NULL, NULL)); if (ret != NOTFOUND) { @@ -1045,12 +1043,12 @@ public: /// node pointer. template Result find(const isc::dns::Name& name, - const DomainTreeNode** node, - DomainTreeNodeChain& node_path, - bool (*callback)(const DomainTreeNode&, CBARG), + const DomainTreeNode** node, + DomainTreeNodeChain& node_path, + bool (*callback)(const DomainTreeNode&, CBARG), CBARG callback_arg) const { - DomainTreeNode* target_node = NULL; + DomainTreeNode* target_node = NULL; const isc::dns::LabelSequence ls(name); Result ret = find(ls, &target_node, node_path, callback, callback_arg); @@ -1134,9 +1132,9 @@ public: /// \c true, it returns immediately with the current node. template Result find(const isc::dns::LabelSequence& target_labels_orig, - DomainTreeNode** node, - DomainTreeNodeChain& node_path, - bool (*callback)(const DomainTreeNode&, CBARG), + DomainTreeNode** node, + DomainTreeNodeChain& node_path, + bool (*callback)(const DomainTreeNode&, CBARG), CBARG callback_arg) const; /// \brief Simple find returning immutable node. @@ -1145,12 +1143,12 @@ public: /// node pointer. template Result find(const isc::dns::LabelSequence& target_labels, - const DomainTreeNode** node, - DomainTreeNodeChain& node_path, - bool (*callback)(const DomainTreeNode&, CBARG), + const DomainTreeNode** node, + DomainTreeNodeChain& node_path, + bool (*callback)(const DomainTreeNode&, CBARG), CBARG callback_arg) const { - DomainTreeNode* target_node = NULL; + DomainTreeNode* target_node = NULL; Result ret = find(target_labels, &target_node, node_path, callback, callback_arg); if (ret != NOTFOUND) { @@ -1183,8 +1181,8 @@ public: /// /// \return An \c DomainTreeNode that is next bigger than \c node; /// if \c node is the largest, \c NULL will be returned. - const DomainTreeNode* - nextNode(DomainTreeNodeChain& node_path) const; + const DomainTreeNode* + nextNode(DomainTreeNodeChain& node_path) const; /// \brief return the next smaller node in DNSSEC order from a node /// searched by DomainTree::find(). @@ -1208,8 +1206,8 @@ public: /// /// \return An \c DomainTreeNode that is next smaller than \c node; /// if \c node is the smallest, \c NULL will be returned. - const DomainTreeNode* - previousNode(DomainTreeNodeChain& node_path) const; + const DomainTreeNode* + previousNode(DomainTreeNodeChain& node_path) const; /// \brief Get the total number of nodes in the tree /// @@ -1274,7 +1272,7 @@ public: /// - ALREADYEXISTS There was already a node of that name, so it was not /// added. Result insert(util::MemorySegment& mem_sgmt, const isc::dns::Name& name, - DomainTreeNode** inserted_node); + DomainTreeNode** inserted_node); /// \brief Delete all tree nodes. /// @@ -1283,7 +1281,8 @@ public: /// \param mem_sgmt The \c MemorySegment object used to insert the nodes /// (which was also used for creating the tree due to the requirement of /// \c inert()). - void deleteAllNodes(util::MemorySegment& mem_sgmt); + template + void deleteAllNodes(util::MemorySegment& mem_sgmt, DataDeleter deleter); /// \brief Swaps two tree's contents. /// @@ -1293,7 +1292,7 @@ public: /// /// This acts the same as many std::*.swap functions, exchanges the /// contents. This doesn't throw anything. - void swap(DomainTree& other) { + void swap(DomainTree& other) { std::swap(root_, other.root_); std::swap(node_count_, other.node_count_); } @@ -1303,31 +1302,32 @@ private: /// \name DomainTree balance functions //@{ void - insertRebalance(typename DomainTreeNode::DomainTreeNodePtr* root, - DomainTreeNode* node); + insertRebalance(typename DomainTreeNode::DomainTreeNodePtr* root, + DomainTreeNode* node); - DomainTreeNode* - rightRotate(typename DomainTreeNode::DomainTreeNodePtr* root, - DomainTreeNode* node); + DomainTreeNode* + rightRotate(typename DomainTreeNode::DomainTreeNodePtr* root, + DomainTreeNode* node); - DomainTreeNode* - leftRotate(typename DomainTreeNode::DomainTreeNodePtr* root, - DomainTreeNode* node); + DomainTreeNode* + leftRotate(typename DomainTreeNode::DomainTreeNodePtr* root, + DomainTreeNode* node); //@} /// \name Helper functions //@{ /// \brief delete tree whose root is equal to node + template void deleteHelper(util::MemorySegment& mem_sgmt, - DomainTreeNode *node, - const DT& deleter); + DomainTreeNode *node, + const DataDeleter& deleter); /// \brief Print the information of given DomainTreeNode. - void dumpTreeHelper(std::ostream& os, const DomainTreeNode* node, + void dumpTreeHelper(std::ostream& os, const DomainTreeNode* node, unsigned int depth) const; /// \brief Print the information of given DomainTreeNode for dot. - int dumpDotHelper(std::ostream& os, const DomainTreeNode* node, + int dumpDotHelper(std::ostream& os, const DomainTreeNode* node, int* nodecount, bool show_pointers) const; /// \brief Indentation helper function for dumpTree @@ -1340,78 +1340,80 @@ private: /// the entire tree. This ensures that a pointer to a node keeps its /// semantics even if the tree structure is changed (as long as the node /// itself remains valid). - void nodeFission(util::MemorySegment& mem_sgmt, DomainTreeNode& node, + void nodeFission(util::MemorySegment& mem_sgmt, DomainTreeNode& node, const isc::dns::LabelSequence& new_prefix, const isc::dns::LabelSequence& new_suffix); //@} - typename DomainTreeNode::DomainTreeNodePtr root_; + typename DomainTreeNode::DomainTreeNodePtr root_; /// the node count of current tree unsigned int node_count_; /// search policy for domaintree const bool needsReturnEmptyNode_; }; -template -DomainTree::DomainTree(bool returnEmptyNode) : +template +DomainTree::DomainTree(bool returnEmptyNode) : root_(NULL), node_count_(0), needsReturnEmptyNode_(returnEmptyNode) { } -template -DomainTree::~DomainTree() { +template +DomainTree::~DomainTree() { assert(node_count_ == 0); } -template +template +template void -DomainTree::deleteHelper(util::MemorySegment& mem_sgmt, - DomainTreeNode* root, - const DT& deleter) { +DomainTree::deleteHelper(util::MemorySegment& mem_sgmt, + DomainTreeNode* root, + const DataDeleter& deleter) +{ while (root != NULL) { // If there is a left, right or down node, walk into it and // iterate. if (root->getLeft() != NULL) { - DomainTreeNode* node = root; + DomainTreeNode* node = root; root = root->getLeft(); node->left_ = NULL; } else if (root->getRight() != NULL) { - DomainTreeNode* node = root; + DomainTreeNode* node = root; root = root->getRight(); node->right_ = NULL; } else if (root->getDown() != NULL) { - DomainTreeNode* node = root; + DomainTreeNode* node = root; root = root->getDown(); node->down_ = NULL; } else { // There are no left, right or down nodes, so we can // free this one and go back to its parent. - DomainTreeNode* node = root; + DomainTreeNode* node = root; root = root->getParent(); - deleter(mem_sgmt, node->data_.get()); - DomainTreeNode::destroy(mem_sgmt, node); + deleter(node->data_.get()); + DomainTreeNode::destroy(mem_sgmt, node); --node_count_; } } } -template +template template -typename DomainTree::Result -DomainTree::find(const isc::dns::LabelSequence& target_labels_orig, - DomainTreeNode** target, - DomainTreeNodeChain& node_path, - bool (*callback)(const DomainTreeNode&, CBARG), - CBARG callback_arg) const +typename DomainTree::Result +DomainTree::find(const isc::dns::LabelSequence& target_labels_orig, + DomainTreeNode** target, + DomainTreeNodeChain& node_path, + bool (*callback)(const DomainTreeNode&, CBARG), + CBARG callback_arg) const { if (!node_path.isEmpty()) { isc_throw(isc::BadValue, "DomainTree::find is given a non empty chain"); } - DomainTreeNode* node = root_.get(); + DomainTreeNode* node = root_.get(); Result ret = NOTFOUND; dns::LabelSequence target_labels(target_labels_orig); @@ -1439,7 +1441,7 @@ DomainTree::find(const isc::dns::LabelSequence& target_labels_orig, ret = PARTIALMATCH; *target = node; if (callback != NULL && - node->getFlag(DomainTreeNode::FLAG_CALLBACK)) { + node->getFlag(DomainTreeNode::FLAG_CALLBACK)) { if ((callback)(*node, callback_arg)) { break; } @@ -1458,20 +1460,20 @@ DomainTree::find(const isc::dns::LabelSequence& target_labels_orig, return (ret); } -template -const DomainTreeNode* -DomainTree::nextNode(DomainTreeNodeChain& node_path) const { +template +const DomainTreeNode* +DomainTree::nextNode(DomainTreeNodeChain& node_path) const { if (node_path.isEmpty()) { isc_throw(isc::BadValue, "DomainTree::nextNode is given an empty chain"); } - const DomainTreeNode* node = node_path.top(); + const DomainTreeNode* node = node_path.top(); // if node has sub domain, the next domain is the smallest // domain in sub domain tree - const DomainTreeNode* down = node->getDown(); + const DomainTreeNode* down = node->getDown(); if (down != NULL) { - const DomainTreeNode* left_most = down; + const DomainTreeNode* left_most = down; while (left_most->getLeft() != NULL) { left_most = left_most->getLeft(); } @@ -1485,7 +1487,7 @@ DomainTree::nextNode(DomainTreeNodeChain& node_path) const { // up node doesn't have successor we gonna keep moving to up // level while (!node_path.isEmpty()) { - const DomainTreeNode* up_node_successor = + const DomainTreeNode* up_node_successor = node_path.top()->successor(); node_path.pop(); if (up_node_successor != NULL) { @@ -1497,9 +1499,9 @@ DomainTree::nextNode(DomainTreeNodeChain& node_path) const { return (NULL); } -template -const DomainTreeNode* -DomainTree::previousNode(DomainTreeNodeChain& node_path) const { +template +const DomainTreeNode* +DomainTree::previousNode(DomainTreeNodeChain& node_path) const { if (getNodeCount() == 0) { // Special case for empty trees. It would look every time like // we didn't search, because the last compared is empty. This is @@ -1539,13 +1541,13 @@ DomainTree::previousNode(DomainTreeNodeChain& node_path) const { // compared one (it is either the compared one, or some // subdomain of it). There probably is not an easy trick // for this, so we just find the correct place. - const DomainTreeNode* current(node_path.last_compared_); + const DomainTreeNode* current(node_path.last_compared_); while (current != NULL) { node_path.push(current); // Go a level down and as much right there as possible current = current->getDown(); if (current != NULL) { - const DomainTreeNode* right; + const DomainTreeNode* right; while ((right = current->getRight()) != NULL) { current = right; } @@ -1596,7 +1598,7 @@ DomainTree::previousNode(DomainTreeNodeChain& node_path) const { return (NULL); } - const DomainTreeNode* node(node_path.top()); + const DomainTreeNode* node(node_path.top()); // Try going left in this tree node = node->predecessor(); @@ -1619,13 +1621,13 @@ DomainTree::previousNode(DomainTreeNodeChain& node_path) const { node_path.push(node); // Try going as deep as possible, keeping on the right side of the trees - const DomainTreeNode* down; + const DomainTreeNode* down; while ((down = node->getDown()) != NULL) { // Move to the tree below node = down; if (node != NULL) { // And get as much to the right of the tree as possible - const DomainTreeNode* right; + const DomainTreeNode* right; while ((right = node->getRight()) != NULL) { node = right; } @@ -1640,15 +1642,15 @@ DomainTree::previousNode(DomainTreeNodeChain& node_path) const { return (node); } -template -typename DomainTree::Result -DomainTree::insert(util::MemorySegment& mem_sgmt, - const isc::dns::Name& target_name, - DomainTreeNode** new_node) +template +typename DomainTree::Result +DomainTree::insert(util::MemorySegment& mem_sgmt, + const isc::dns::Name& target_name, + DomainTreeNode** new_node) { - DomainTreeNode* parent = NULL; - DomainTreeNode* current = root_.get(); - DomainTreeNode* up_node = NULL; + DomainTreeNode* parent = NULL; + DomainTreeNode* current = root_.get(); + DomainTreeNode* up_node = NULL; isc::dns::LabelSequence target_labels(target_name); int order = -1; @@ -1688,17 +1690,17 @@ DomainTree::insert(util::MemorySegment& mem_sgmt, } } - typename DomainTreeNode::DomainTreeNodePtr* current_root = + typename DomainTreeNode::DomainTreeNodePtr* current_root = (up_node != NULL) ? &(up_node->down_) : &root_; // Once a new node is created, no exception will be thrown until the end // of the function, so we can simply create and hold a new node pointer. - DomainTreeNode* node = DomainTreeNode::create(mem_sgmt, - target_labels); + DomainTreeNode* node = DomainTreeNode::create(mem_sgmt, + target_labels); node->parent_ = parent; if (parent == NULL) { *current_root = node; // node is the new root of sub tree, so its init color is BLACK - node->setColor(DomainTreeNode::BLACK); + node->setColor(DomainTreeNode::BLACK); node->setSubTreeRoot(true); node->parent_ = up_node; } else if (order < 0) { @@ -1717,28 +1719,30 @@ DomainTree::insert(util::MemorySegment& mem_sgmt, return (SUCCESS); } -template +template +template void -DomainTree::deleteAllNodes(util::MemorySegment& mem_sgmt) { - const DT deleter; +DomainTree::deleteAllNodes(util::MemorySegment& mem_sgmt, + DataDeleter deleter) +{ deleteHelper(mem_sgmt, root_.get(), deleter); root_ = NULL; } -template +template void -DomainTree::nodeFission(util::MemorySegment& mem_sgmt, - DomainTreeNode& node, - const isc::dns::LabelSequence& new_prefix, - const isc::dns::LabelSequence& new_suffix) +DomainTree::nodeFission(util::MemorySegment& mem_sgmt, + DomainTreeNode& node, + const isc::dns::LabelSequence& new_prefix, + const isc::dns::LabelSequence& new_suffix) { // Create and reset the labels. // Once a new node is created, no exception will be thrown until // the end of the function, and it will keep consistent behavior // (i.e., a weak form of strong exception guarantee) even if code // after the call to this function throws an exception. - DomainTreeNode* up_node = DomainTreeNode::create(mem_sgmt, - new_suffix); + DomainTreeNode* up_node = DomainTreeNode::create(mem_sgmt, + new_suffix); node.resetLabels(new_prefix); up_node->parent_ = node.getParent(); @@ -1772,7 +1776,7 @@ DomainTree::nodeFission(util::MemorySegment& mem_sgmt, // set color of both nodes; the initial subtree node color is BLACK up_node->setColor(node.getColor()); - node.setColor(DomainTreeNode::BLACK); + node.setColor(DomainTreeNode::BLACK); // set the subtree root flag of both nodes up_node->setSubTreeRoot(node.isSubTreeRoot()); @@ -1782,27 +1786,27 @@ DomainTree::nodeFission(util::MemorySegment& mem_sgmt, } -template +template void -DomainTree::insertRebalance - (typename DomainTreeNode::DomainTreeNodePtr* root, - DomainTreeNode* node) +DomainTree::insertRebalance + (typename DomainTreeNode::DomainTreeNodePtr* root, + DomainTreeNode* node) { - DomainTreeNode* uncle; - DomainTreeNode* parent; + DomainTreeNode* uncle; + DomainTreeNode* parent; while (node != (*root).get() && ((parent = node->getParent())->getColor()) == - DomainTreeNode::RED) { + DomainTreeNode::RED) { // Here, node->parent_ is not NULL and it is also red, so // node->parent_->parent_ is also not NULL. if (parent == parent->getParent()->getLeft()) { uncle = parent->getParent()->getRight(); if (uncle != NULL && uncle->getColor() == - DomainTreeNode::RED) { - parent->setColor(DomainTreeNode::BLACK); - uncle->setColor(DomainTreeNode::BLACK); - parent->getParent()->setColor(DomainTreeNode::RED); + DomainTreeNode::RED) { + parent->setColor(DomainTreeNode::BLACK); + uncle->setColor(DomainTreeNode::BLACK); + parent->getParent()->setColor(DomainTreeNode::RED); node = parent->getParent(); } else { if (node == parent->getRight()) { @@ -1810,18 +1814,18 @@ DomainTree::insertRebalance leftRotate(root, node); parent = node->getParent(); } - parent->setColor(DomainTreeNode::BLACK); - parent->getParent()->setColor(DomainTreeNode::RED); + parent->setColor(DomainTreeNode::BLACK); + parent->getParent()->setColor(DomainTreeNode::RED); rightRotate(root, parent->getParent()); } } else { uncle = parent->getParent()->getLeft(); if (uncle != NULL && uncle->getColor() == - DomainTreeNode::RED) { - parent->setColor(DomainTreeNode::BLACK); - uncle->setColor(DomainTreeNode::BLACK); - parent->getParent()->setColor(DomainTreeNode::RED); + DomainTreeNode::RED) { + parent->setColor(DomainTreeNode::BLACK); + uncle->setColor(DomainTreeNode::BLACK); + parent->getParent()->setColor(DomainTreeNode::RED); node = parent->getParent(); } else { if (node == parent->getLeft()) { @@ -1829,31 +1833,31 @@ DomainTree::insertRebalance rightRotate(root, node); parent = node->getParent(); } - parent->setColor(DomainTreeNode::BLACK); - parent->getParent()->setColor(DomainTreeNode::RED); + parent->setColor(DomainTreeNode::BLACK); + parent->getParent()->setColor(DomainTreeNode::RED); leftRotate(root, parent->getParent()); } } } - (*root)->setColor(DomainTreeNode::BLACK); + (*root)->setColor(DomainTreeNode::BLACK); } -template -DomainTreeNode* -DomainTree::leftRotate - (typename DomainTreeNode::DomainTreeNodePtr* root, - DomainTreeNode* node) +template +DomainTreeNode* +DomainTree::leftRotate + (typename DomainTreeNode::DomainTreeNodePtr* root, + DomainTreeNode* node) { - DomainTreeNode* const right = node->getRight(); - DomainTreeNode* const rleft = right->getLeft(); + DomainTreeNode* const right = node->getRight(); + DomainTreeNode* const rleft = right->getLeft(); node->right_ = rleft; if (rleft != NULL) { rleft->parent_ = node; } - DomainTreeNode* const parent = node->getParent(); + DomainTreeNode* const parent = node->getParent(); right->parent_ = parent; if (!node->isSubTreeRoot()) { @@ -1874,20 +1878,20 @@ DomainTree::leftRotate return (node); } -template -DomainTreeNode* -DomainTree::rightRotate - (typename DomainTreeNode::DomainTreeNodePtr* root, - DomainTreeNode* node) +template +DomainTreeNode* +DomainTree::rightRotate + (typename DomainTreeNode::DomainTreeNodePtr* root, + DomainTreeNode* node) { - DomainTreeNode* const left = node->getLeft(); - DomainTreeNode* const lright = left->getRight(); + DomainTreeNode* const left = node->getLeft(); + DomainTreeNode* const lright = left->getRight(); node->left_ = lright; if (lright != NULL) { lright->parent_ = node; } - DomainTreeNode* const parent = node->getParent(); + DomainTreeNode* const parent = node->getParent(); left->parent_ = parent; if (!node->isSubTreeRoot()) { @@ -1909,19 +1913,19 @@ DomainTree::rightRotate } -template +template void -DomainTree::dumpTree(std::ostream& os, unsigned int depth) const { +DomainTree::dumpTree(std::ostream& os, unsigned int depth) const { indent(os, depth); os << "tree has " << node_count_ << " node(s)\n"; dumpTreeHelper(os, root_.get(), depth); } -template +template void -DomainTree::dumpTreeHelper(std::ostream& os, - const DomainTreeNode* node, - unsigned int depth) const +DomainTree::dumpTreeHelper(std::ostream& os, + const DomainTreeNode* node, + unsigned int depth) const { if (node == NULL) { indent(os, depth); @@ -1931,7 +1935,7 @@ DomainTree::dumpTreeHelper(std::ostream& os, indent(os, depth); os << node->getLabels() << " (" - << ((node->getColor() == DomainTreeNode::BLACK) ? "black" : "red") + << ((node->getColor() == DomainTreeNode::BLACK) ? "black" : "red") << ")"; if (node->isEmpty()) { os << " [invisible]"; @@ -1941,7 +1945,7 @@ DomainTree::dumpTreeHelper(std::ostream& os, } os << "\n"; - const DomainTreeNode* down = node->getDown(); + const DomainTreeNode* down = node->getDown(); if (down != NULL) { indent(os, depth + 1); os << "begin down from " << node->getLabels() << "\n"; @@ -1953,16 +1957,16 @@ DomainTree::dumpTreeHelper(std::ostream& os, dumpTreeHelper(os, node->getRight(), depth + 1); } -template +template void -DomainTree::indent(std::ostream& os, unsigned int depth) { +DomainTree::indent(std::ostream& os, unsigned int depth) { static const unsigned int INDENT_FOR_EACH_DEPTH = 5; os << std::string(depth * INDENT_FOR_EACH_DEPTH, ' '); } -template +template void -DomainTree::dumpDot(std::ostream& os, bool show_pointers) const { +DomainTree::dumpDot(std::ostream& os, bool show_pointers) const { int nodecount = 0; os << "digraph g {\n"; @@ -1971,11 +1975,11 @@ DomainTree::dumpDot(std::ostream& os, bool show_pointers) const { os << "}\n"; } -template +template int -DomainTree::dumpDotHelper(std::ostream& os, - const DomainTreeNode* node, - int* nodecount, bool show_pointers) const +DomainTree::dumpDotHelper(std::ostream& os, + const DomainTreeNode* node, + int* nodecount, bool show_pointers) const { if (node == NULL) { return 0; @@ -1995,7 +1999,7 @@ DomainTree::dumpDotHelper(std::ostream& os, } os << "\"] ["; - if (node->getColor() == DomainTreeNode::RED) { + if (node->getColor() == DomainTreeNode::RED) { os << "color=red"; } else { os << "color=black"; diff --git a/src/lib/datasrc/memory/tests/domaintree_unittest.cc b/src/lib/datasrc/memory/tests/domaintree_unittest.cc index cfb223a719..21d6dbb7b6 100644 --- a/src/lib/datasrc/memory/tests/domaintree_unittest.cc +++ b/src/lib/datasrc/memory/tests/domaintree_unittest.cc @@ -59,18 +59,13 @@ const size_t Name::MAX_LABELS; namespace { -class DeleterType { -public: - DeleterType() {} - - void operator()(util::MemorySegment&, int* i) const { - delete i; - } -}; +void deleteData(int* i) { + delete i; +} -typedef DomainTree TestDomainTree; -typedef DomainTreeNode TestDomainTreeNode; -typedef DomainTreeNodeChain TestDomainTreeNodeChain; +typedef DomainTree TestDomainTree; +typedef DomainTreeNode TestDomainTreeNode; +typedef DomainTreeNodeChain TestDomainTreeNodeChain; class TreeHolder { public: @@ -78,7 +73,7 @@ public: mem_sgmt_(mem_sgmt), tree_(tree) {} ~TreeHolder() { - TestDomainTree::destroy(mem_sgmt_, tree_); + TestDomainTree::destroy(mem_sgmt_, tree_, deleteData); } TestDomainTree* get() { return (tree_); } private: @@ -102,11 +97,11 @@ protected: int name_count = sizeof(domain_names) / sizeof(domain_names[0]); for (int i = 0; i < name_count; ++i) { dtree.insert(mem_sgmt_, Name(domain_names[i]), &dtnode); - dtnode->setData(mem_sgmt_, new int(i + 1)); + dtnode->setData(new int(i + 1)); dtree_expose_empty_node.insert(mem_sgmt_, Name(domain_names[i]), &dtnode); - dtnode->setData(mem_sgmt_, new int(i + 1)); + dtnode->setData(new int(i + 1)); } } @@ -125,12 +120,12 @@ TEST_F(DomainTreeTest, nodeCount) { // Delete all nodes, then the count should be set to 0. This also tests // the behavior of deleteAllNodes(). - dtree.deleteAllNodes(mem_sgmt_); + dtree.deleteAllNodes(mem_sgmt_, deleteData); EXPECT_EQ(0, dtree.getNodeCount()); } TEST_F(DomainTreeTest, setGetData) { - dtnode->setData(mem_sgmt_, new int(11)); + dtnode->setData(new int(11)); EXPECT_EQ(11, *(dtnode->getData())); } @@ -151,7 +146,7 @@ TEST_F(DomainTreeTest, insertNames) { Name("example.com"), &dtnode)); EXPECT_EQ(17, dtree.getNodeCount()); - dtnode->setData(mem_sgmt_, new int(12)); + dtnode->setData(new int(12)); // return ALREADYEXISTS, since node "example.com" already has // been explicitly inserted @@ -381,7 +376,7 @@ performCallbackTest(TestDomainTree& dtree, EXPECT_EQ(TestDomainTree::SUCCESS, dtree.insert(mem_sgmt, Name("callback.example"), &dtnode)); - dtnode->setData(mem_sgmt, new int(1)); + dtnode->setData(new int(1)); EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK)); // enable/re-disable callback @@ -397,7 +392,7 @@ performCallbackTest(TestDomainTree& dtree, EXPECT_EQ(TestDomainTree::SUCCESS, dtree.insert(mem_sgmt, Name("sub.callback.example"), &subdtnode)); - subdtnode->setData(mem_sgmt, new int(2)); + subdtnode->setData(new int(2)); TestDomainTreeNode* parentdtnode; EXPECT_EQ(TestDomainTree::ALREADYEXISTS, dtree.insert(mem_sgmt, Name("example"), @@ -997,7 +992,7 @@ TEST_F(DomainTreeTest, root) { TreeHolder tree_holder(mem_sgmt_, TestDomainTree::create(mem_sgmt_)); TestDomainTree& root(*tree_holder.get()); root.insert(mem_sgmt_, Name::ROOT_NAME(), &dtnode); - dtnode->setData(mem_sgmt_, new int(1)); + dtnode->setData(new int(1)); EXPECT_EQ(TestDomainTree::EXACTMATCH, root.find(Name::ROOT_NAME(), &cdtnode)); @@ -1009,7 +1004,7 @@ TEST_F(DomainTreeTest, root) { // Insert a new name that better matches the query name. find() should // find the better one. root.insert(mem_sgmt_, Name("com"), &dtnode); - dtnode->setData(mem_sgmt_, new int(2)); + dtnode->setData(new int(2)); EXPECT_EQ(TestDomainTree::PARTIALMATCH, root.find(Name("example.com"), &cdtnode)); EXPECT_EQ(dtnode, cdtnode);