[2107] have DomainTreeNode::setData() return pointer to old data.

added some test cases to confirm the behavior; update the doc.

src/lib/datasrc/memory/domaintree.h

@@ -248,13 +248,22 @@ public:
 
     /// \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.
-    void setData(T* data) {
+    /// If there is old data, a pointer to the data will be returned;
+    /// otherwise NULL will be returned.  The caller is responsible for
+    /// releasing any resource for the old data if it's not needed any more.
+    /// See also the note about data ownership in the \c DomainTree
+    /// description.
+    ///
+    /// \c data can be NULL, in which case it effectively clears any existing
+    /// old data.
+    ///
+    /// \param data The new data to set.  It can be NULL.
+    /// \return A pointer to the old data or NULL if the node doesn't have
+    /// data.
+    T* setData(T* data) {
+        T* olddata = data_.get();
         data_ = data;
+        return (olddata);
     }
     //@}
 

src/lib/datasrc/memory/tests/domaintree_unittest.cc

@@ -97,12 +97,13 @@ 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(new int(i + 1));
+            // Check the node doesn't have any data initially.
+            EXPECT_EQ(static_cast<int*>(NULL),
+                      dtnode->setData(new int(i + 1)));
 
             dtree_expose_empty_node.insert(mem_sgmt_, Name(domain_names[i]),
                                             &dtnode);
-            dtnode->setData(new int(i + 1));
-
+            EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(i + 1)));
         }
     }
 
@@ -125,8 +126,17 @@ TEST_F(DomainTreeTest, nodeCount) {
 }
 
 TEST_F(DomainTreeTest, setGetData) {
-    dtnode->setData(new int(11));
+    // set new data to an existing node.  It should have some data.
+    int* newdata = new int(11);
+    int* olddata = dtnode->setData(newdata);
+    EXPECT_NE(static_cast<int*>(NULL), olddata);
+    deleteData(olddata);
     EXPECT_EQ(11, *(dtnode->getData()));
+
+    // clear the node.  we should get the new data back we just passed.
+    olddata = dtnode->setData(NULL);
+    EXPECT_EQ(newdata, olddata);
+    deleteData(olddata);
 }
 
 TEST_F(DomainTreeTest, insertNames) {
@@ -146,7 +156,9 @@ TEST_F(DomainTreeTest, insertNames) {
                                                   Name("example.com"),
                                                   &dtnode));
     EXPECT_EQ(17, dtree.getNodeCount());
-    dtnode->setData(new int(12));
+    // ad data to it; also make sure it doesn't have data right now
+    // (otherwise it would leak)
+    EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(12)));
 
     // return ALREADYEXISTS, since node "example.com" already has
     // been explicitly inserted
@@ -376,7 +388,7 @@ performCallbackTest(TestDomainTree& dtree,
     EXPECT_EQ(TestDomainTree::SUCCESS, dtree.insert(mem_sgmt,
                                                   Name("callback.example"),
                                                   &dtnode));
-    dtnode->setData(new int(1));
+    EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(1)));
     EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
 
     // enable/re-disable callback
@@ -392,7 +404,7 @@ performCallbackTest(TestDomainTree& dtree,
     EXPECT_EQ(TestDomainTree::SUCCESS, dtree.insert(mem_sgmt,
                                                   Name("sub.callback.example"),
                                                   &subdtnode));
-    subdtnode->setData(new int(2));
+    EXPECT_EQ(static_cast<int*>(NULL), subdtnode->setData(new int(2)));
     TestDomainTreeNode* parentdtnode;
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS, dtree.insert(mem_sgmt,
                                                         Name("example"),
@@ -992,7 +1004,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(new int(1));
+    EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(1)));
 
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
               root.find(Name::ROOT_NAME(), &cdtnode));
@@ -1004,7 +1016,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(new int(2));
+    EXPECT_EQ(static_cast<int*>(NULL), dtnode->setData(new int(2)));
     EXPECT_EQ(TestDomainTree::PARTIALMATCH,
               root.find(Name("example.com"), &cdtnode));
     EXPECT_EQ(dtnode, cdtnode);