Browse Source

[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 <T, DT> to <T>.
JINMEI Tatuya 12 years ago
parent
commit
6679f424fb
2 changed files with 270 additions and 271 deletions
  1. 254 250
      src/lib/datasrc/memory/domaintree.h
  2. 16 21
      src/lib/datasrc/memory/tests/domaintree_unittest.cc

File diff suppressed because it is too large
+ 254 - 250
src/lib/datasrc/memory/domaintree.h


+ 16 - 21
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<int, DeleterType> TestDomainTree;
-typedef DomainTreeNode<int, DeleterType> TestDomainTreeNode;
-typedef DomainTreeNodeChain<int, DeleterType> TestDomainTreeNodeChain;
+typedef DomainTree<int> TestDomainTree;
+typedef DomainTreeNode<int> TestDomainTreeNode;
+typedef DomainTreeNodeChain<int> 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);