BIND 10 master, updated. fe827c7e47a642f3aead3b5414c6e8cf3a9970b4 Merge #2292

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Oct 10 10:12:45 UTC 2012


The branch, master has been updated
       via  fe827c7e47a642f3aead3b5414c6e8cf3a9970b4 (commit)
       via  c6391ad769265a1f5c2d5594dcdb113420c7eb4d (commit)
       via  d78dd497ecb846ab2545ce1cb7b6e41341ff5995 (commit)
       via  3a6ee0d12d06c1f9e5b9f261b3fcaab444a9ae80 (commit)
       via  7888a2127db48e004cb1cce24552db230cb3d685 (commit)
       via  869d14d7056ee7b0c3cf1dd0db1b375be755bba7 (commit)
       via  00d3de1af86efe6b4df1310a039adad8f6a719ae (commit)
       via  73900f62a74bf769b5f6ea7b3cf43428baa19842 (commit)
       via  7a628baa1a158b5837d6f383e10b30542d2ac59b (commit)
       via  915576b6aa27d020faadf7fbfc4e4d9bb1df129e (commit)
       via  e1b4bf1cb50160c436f2e5440fd774680b91b7a2 (commit)
       via  76243c3ccd7a30484da8478f7457e18fce485492 (commit)
       via  5c74085e2f4c16655e77e1bcbc308d4fb0a694f8 (commit)
       via  4babe763de5b20437e32829024f25a1b902df82f (commit)
      from  bcf2117983fa80d3f4bb9a13f896963497553b85 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit fe827c7e47a642f3aead3b5414c6e8cf3a9970b4
Merge: bcf2117 c6391ad
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Oct 10 11:58:50 2012 +0200

    Merge #2292

commit c6391ad769265a1f5c2d5594dcdb113420c7eb4d
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Wed Oct 10 11:58:17 2012 +0200

    [2292] Add a TODO about re-adding tests
    
    Planned for merge #2268

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/memory/domaintree.h                |   70 ++-----------
 src/lib/datasrc/memory/memory_client.cc            |   40 ++------
 src/lib/datasrc/memory/memory_client.h             |   29 ------
 src/lib/datasrc/memory/zone_finder.cc              |    2 +-
 src/lib/datasrc/memory/zone_table.cc               |   44 +++------
 src/lib/datasrc/memory/zone_table.h                |   63 +++++-------
 .../datasrc/tests/memory/domaintree_unittest.cc    |   31 +++---
 .../datasrc/tests/memory/memory_client_unittest.cc |  104 +++-----------------
 src/lib/datasrc/tests/memory/zone_data_unittest.cc |    2 +-
 .../datasrc/tests/memory/zone_table_unittest.cc    |   90 +++++++++++++----
 10 files changed, 156 insertions(+), 319 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/domaintree.h b/src/lib/datasrc/memory/domaintree.h
index 20f4693..7e68f53 100644
--- a/src/lib/datasrc/memory/domaintree.h
+++ b/src/lib/datasrc/memory/domaintree.h
@@ -1080,55 +1080,25 @@ public:
     ///    of it. In that case, node parameter is left intact.
     //@{
 
-    /// \brief Simple find.
+    /// \brief Simple find
     ///
     /// Acts as described in the \ref find section.
     Result find(const isc::dns::Name& name,
-                DomainTreeNode<T>** node) const {
-        DomainTreeNodeChain<T> node_path;
-        const isc::dns::LabelSequence ls(name);
-        return (find<void*>(ls, node, node_path, NULL, NULL));
-    }
-
-    /// \brief Simple find returning immutable node.
-    ///
-    /// Acts as described in the \ref find section, but returns immutable node
-    /// pointer.
-    Result find(const isc::dns::Name& name,
                 const DomainTreeNode<T>** node) const {
         DomainTreeNodeChain<T> node_path;
-        DomainTreeNode<T> *target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
+        Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
         return (ret);
     }
 
     /// \brief Simple find, with node_path tracking
     ///
     /// Acts as described in the \ref find section.
-    Result find(const isc::dns::Name& name, DomainTreeNode<T>** node,
-                DomainTreeNodeChain<T>& node_path) const
-    {
-        const isc::dns::LabelSequence ls(name);
-        return (find<void*>(ls, node, node_path, NULL, NULL));
-    }
-
-    /// \brief Simple find returning immutable node, with node_path tracking
-    ///
-    /// Acts as described in the \ref find section, but returns immutable node
-    /// pointer.
     Result find(const isc::dns::Name& name, const DomainTreeNode<T>** node,
                 DomainTreeNodeChain<T>& node_path) const
     {
-        DomainTreeNode<T> *target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = (find<void*>(ls, &target_node, node_path, NULL, NULL));
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
+        Result ret = (find<void*>(ls, node, node_path, NULL, NULL));
         return (ret);
     }
 
@@ -1143,13 +1113,9 @@ public:
                 bool (*callback)(const DomainTreeNode<T>&, CBARG),
                 CBARG callback_arg) const
     {
-        DomainTreeNode<T>* target_node = NULL;
         const isc::dns::LabelSequence ls(name);
-        Result ret = find(ls, &target_node, node_path, callback,
+        Result ret = find(ls, node, node_path, callback,
                           callback_arg);
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
         return (ret);
     }
 
@@ -1229,30 +1195,10 @@ public:
     ///     \c true, it returns immediately with the current node.
     template <typename CBARG>
     Result find(const isc::dns::LabelSequence& target_labels_orig,
-                DomainTreeNode<T>** node,
-                DomainTreeNodeChain<T>& node_path,
-                bool (*callback)(const DomainTreeNode<T>&, CBARG),
-                CBARG callback_arg) const;
-
-    /// \brief Simple find returning immutable node.
-    ///
-    /// Acts as described in the \ref find section, but returns immutable
-    /// node pointer.
-    template <typename CBARG>
-    Result find(const isc::dns::LabelSequence& target_labels,
                 const DomainTreeNode<T>** node,
                 DomainTreeNodeChain<T>& node_path,
                 bool (*callback)(const DomainTreeNode<T>&, CBARG),
-                CBARG callback_arg) const
-    {
-        DomainTreeNode<T>* target_node = NULL;
-        Result ret = find(target_labels, &target_node, node_path,
-                          callback, callback_arg);
-        if (ret != NOTFOUND) {
-            *node = target_node;
-        }
-        return (ret);
-    }
+                CBARG callback_arg) const;
     //@}
 
     /// \brief return the next bigger node in DNSSEC order from a given node
@@ -1515,7 +1461,7 @@ template <typename T>
 template <typename CBARG>
 typename DomainTree<T>::Result
 DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
-                    DomainTreeNode<T>** target,
+                    const DomainTreeNode<T>** target,
                     DomainTreeNodeChain<T>& node_path,
                     bool (*callback)(const DomainTreeNode<T>&, CBARG),
                     CBARG callback_arg) const
@@ -1526,11 +1472,11 @@ DomainTree<T>::find(const isc::dns::LabelSequence& target_labels_orig,
                   " and label sequence");
     }
 
-    DomainTreeNode<T>* node;
+    const DomainTreeNode<T>* node;
 
     if (!node_path.isEmpty()) {
         // Get the top node in the node chain
-        node = const_cast<DomainTreeNode<T>*>(node_path.top());
+        node = node_path.top();
         // Start searching from its down pointer
         node = node->getDown();
     } else {
diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc
index 5f6f510..1609611 100644
--- a/src/lib/datasrc/memory/memory_client.cc
+++ b/src/lib/datasrc/memory/memory_client.cc
@@ -627,22 +627,20 @@ InMemoryClient::InMemoryClientImpl::load(
     // node must point to a valid node now
     assert(node != NULL);
 
-    std::string* tstr = node->setData(new std::string(filename));
+    const std::string* tstr = node->setData(new std::string(filename));
     delete tstr;
 
-    ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
-                                                     zone_name));
+    const ZoneTable::AddResult result(zone_table_->addZone(mem_sgmt_, rrclass_,
+                                                           zone_name,
+                                                           holder.release()));
     if (result.code == result::SUCCESS) {
         // Only increment the zone count if the zone doesn't already
         // exist.
         ++zone_count_;
     }
-
-    ZoneTable::FindResult fr(zone_table_->setZoneData(zone_name,
-                                                      holder.release()));
-    assert(fr.code == result::SUCCESS);
-    if (fr.zone_data != NULL) {
-        ZoneData::destroy(mem_sgmt_, fr.zone_data, rrclass_);
+    // Destroy the old instance of the zone if there was any
+    if (result.zone_data != NULL) {
+        ZoneData::destroy(mem_sgmt_, result.zone_data, rrclass_);
     }
 
     return (result.code);
@@ -732,9 +730,9 @@ InMemoryClient::load(const isc::dns::Name& zone_name,
 
 const std::string
 InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
-    FileNameNode* node(NULL);
-    FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
-                                                               &node);
+    const FileNameNode* node(NULL);
+    const FileNameTree::Result result = impl_->file_name_tree_->find(zone_name,
+                                                                     &node);
     if (result == FileNameTree::EXACTMATCH) {
         return (*node->getData());
     } else {
@@ -742,24 +740,6 @@ InMemoryClient::getFileName(const isc::dns::Name& zone_name) const {
     }
 }
 
-result::Result
-InMemoryClient::add(const isc::dns::Name& zone_name,
-                    const ConstRRsetPtr& rrset)
-{
-    const ZoneTable::FindResult result =
-        impl_->zone_table_->findZone(zone_name);
-    if (result.code != result::SUCCESS) {
-        isc_throw(DataSourceError, "No such zone: " + zone_name.toText());
-    }
-
-    const ConstRRsetPtr sig_rrset =
-        rrset ? rrset->getRRsig() : ConstRRsetPtr();
-    impl_->add(rrset, sig_rrset, zone_name, *result.zone_data);
-
-    // add() doesn't allow duplicate add, so we always return SUCCESS.
-    return (result::SUCCESS);
-}
-
 namespace {
 
 class MemoryIterator : public ZoneIterator {
diff --git a/src/lib/datasrc/memory/memory_client.h b/src/lib/datasrc/memory/memory_client.h
index 330d62e..c37ad53 100644
--- a/src/lib/datasrc/memory/memory_client.h
+++ b/src/lib/datasrc/memory/memory_client.h
@@ -139,35 +139,6 @@ public:
     /// zone from a file before.
     const std::string getFileName(const isc::dns::Name& zone_name) const;
 
-    /// \brief Inserts an rrset into the zone.
-    ///
-    /// It puts another RRset into the zone.
-    ///
-    /// In the current implementation, this method doesn't allow an existing
-    /// RRset to be updated or overridden.  So the caller must make sure that
-    /// all RRs of the same type and name must be given in the form of a
-    /// single RRset.  The current implementation will also require that
-    /// when an RRSIG is added, the RRset to be covered has already been
-    /// added.  These restrictions are probably too strict when this data
-    /// source accepts various forms of input, so they should be revisited
-    /// later.
-    ///
-    /// Except for NullRRset and OutOfZone, this method does not guarantee
-    /// strong exception safety (it is currently not needed, if it is needed
-    /// in future, it should be implemented).
-    ///
-    /// \throw NullRRset \c rrset is a NULL pointer.
-    /// \throw OutOfZone The owner name of \c rrset is outside of the
-    /// origin of the zone.
-    /// \throw AddError Other general errors.
-    /// \throw Others This method might throw standard allocation exceptions.
-    ///
-    /// \param rrset The set to add.
-    /// \return SUCCESS or EXIST (if an rrset for given name and type already
-    ///    exists).
-    result::Result add(const isc::dns::Name& zone_name,
-                       const isc::dns::ConstRRsetPtr& rrset);
-
     /// \brief RRset is NULL exception.
     ///
     /// This is thrown if the provided RRset parameter is NULL.
diff --git a/src/lib/datasrc/memory/zone_finder.cc b/src/lib/datasrc/memory/zone_finder.cc
index 86dd874..11188a0 100644
--- a/src/lib/datasrc/memory/zone_finder.cc
+++ b/src/lib/datasrc/memory/zone_finder.cc
@@ -428,7 +428,7 @@ FindNodeResult findNode(const ZoneData& zone_data,
                         ZoneFinder::FindOptions options,
                         bool out_of_zone_ok = false)
 {
-    ZoneNode* node = NULL;
+    const ZoneNode* node = NULL;
     FindState state((options & ZoneFinder::FIND_GLUE_OK) != 0);
 
     const ZoneTree& tree(zone_data.getZoneTree());
diff --git a/src/lib/datasrc/memory/zone_table.cc b/src/lib/datasrc/memory/zone_table.cc
index a74a61d..836b020 100644
--- a/src/lib/datasrc/memory/zone_table.cc
+++ b/src/lib/datasrc/memory/zone_table.cc
@@ -69,17 +69,13 @@ ZoneTable::destroy(util::MemorySegment& mem_sgmt, ZoneTable* ztable,
 
 ZoneTable::AddResult
 ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
-                   const Name& zone_name)
+                   const Name& zone_name, ZoneData* content)
 {
-    // Create a new ZoneData instance first.  If the specified name already
-    // exists in the table, the new data will soon be destroyed, but we want
-    // to make sure if this allocation fails the tree won't be changed to
-    // provide as strong guarantee as possible.  In practice, we generally
-    // expect the caller tries to add a zone only when it's a new one, so
-    // this should be a minor concern.
-    SegmentObjectHolder<ZoneData, RRClass> holder(
-        mem_sgmt, ZoneData::create(mem_sgmt, zone_name), zone_class);
-
+    if (content == NULL) {
+        isc_throw(isc::BadValue, "Zone content must not be NULL");
+    }
+    SegmentObjectHolder<ZoneData, RRClass> holder(mem_sgmt, content,
+                                                  zone_class);
     // Get the node where we put the zone
     ZoneTableNode* node(NULL);
     switch (zones_->insert(mem_sgmt, zone_name, &node)) {
@@ -94,18 +90,18 @@ ZoneTable::addZone(util::MemorySegment& mem_sgmt, RRClass zone_class,
     // Can Not Happen
     assert(node != NULL);
 
-    // Is it empty? We either just created it or it might be nonterminal
-    if (node->isEmpty()) {
-        node->setData(holder.get());
-        return (AddResult(result::SUCCESS, holder.release()));
-    } else { // There's something there already
-        return (AddResult(result::EXIST, node->getData()));
+    // We can release now, setData never throws
+    ZoneData* old = node->setData(holder.release());
+    if (old != NULL) {
+        return (AddResult(result::EXIST, old));
+    } else {
+        return (AddResult(result::SUCCESS, NULL));
     }
 }
 
 ZoneTable::FindResult
 ZoneTable::findZone(const Name& name) const {
-    ZoneTableNode* node(NULL);
+    const ZoneTableNode* node(NULL);
     result::Result my_result;
 
     // Translate the return codes
@@ -132,20 +128,6 @@ ZoneTable::findZone(const Name& name) const {
     return (FindResult(my_result, node->getData()));
 }
 
-ZoneTable::FindResult
-ZoneTable::setZoneData(const Name& name, ZoneData* data)
-{
-    ZoneTableNode* node(NULL);
-
-    ZoneTableTree::Result result(zones_->find(name, &node));
-
-    if (result != ZoneTableTree::EXACTMATCH) {
-        return (FindResult(result::NOTFOUND, NULL));
-    } else {
-        return (FindResult(result::SUCCESS, node->setData(data)));
-    }
-}
-
 } // end of namespace memory
 } // end of namespace datasrc
 } // end of namespace isc
diff --git a/src/lib/datasrc/memory/zone_table.h b/src/lib/datasrc/memory/zone_table.h
index 8ad6213..024558e 100644
--- a/src/lib/datasrc/memory/zone_table.h
+++ b/src/lib/datasrc/memory/zone_table.h
@@ -74,23 +74,23 @@ private:
     typedef DomainTreeNode<ZoneData> ZoneTableNode;
 
 public:
-    /// \brief Result data of addZone() method.
-    struct AddResult {
-        AddResult(result::Result param_code, ZoneData* param_zone_data) :
-            code(param_code), zone_data(param_zone_data)
-        {}
-        const result::Result code;
-        ZoneData* const zone_data;
-    };
+     /// \brief Result data of addZone() method.
+     struct AddResult {
+         AddResult(result::Result param_code, ZoneData* param_zone_data) :
+             code(param_code), zone_data(param_zone_data)
+         {}
+         const result::Result code;
+         ZoneData* const zone_data;
+     };
 
     /// \brief Result data of findZone() method.
     struct FindResult {
         FindResult(result::Result param_code,
-                   ZoneData* param_zone_data) :
+                   const ZoneData* param_zone_data) :
             code(param_code), zone_data(param_zone_data)
         {}
         const result::Result code;
-        ZoneData* const zone_data;
+        const ZoneData* const zone_data;
     };
 
 private:
@@ -140,30 +140,29 @@ public:
 
     /// Add a new zone to the \c ZoneTable.
     ///
-    /// This method creates a new \c ZoneData for the given zone name and
-    /// holds it in the internal table.  The newly created zone data will be
-    /// returned via the \c zone_data member of the return value.  If the given
-    /// zone name already exists in the table, a new data object won't be
-    /// created; instead, the existing corresponding data will be returned.
-    ///
-    /// The zone table keeps the ownership of the created zone data; the
-    /// caller must not try to destroy it directly.  (We'll eventually
-    /// add an interface to delete specific zone data from the table).
+    /// This method adds a given zone data to the internal table.
     ///
     /// \throw std::bad_alloc Internal resource allocation fails.
     ///
     /// \param mem_sgmt The \c MemorySegment to allocate zone data to be
-    /// created.  It must be the same segment that was used to create
-    /// the zone table at the time of create().
+    ///     created.  It must be the same segment that was used to create
+    ///     the zone table at the time of create().
     /// \param zone_name The name of the zone to be added.
     /// \param zone_class The RR class of the zone.  It must be the RR class
-    /// that is supposed to be associated to the zone table.
+    ///     that is supposed to be associated to the zone table.
+    /// \param content This one should hold the zone content (the ZoneData).
+    ///     The ownership is passed onto the zone table. Must not be null.
+    ///     Must correspond to the name and class and must be allocated from
+    ///     mem_sgmt.
     /// \return \c result::SUCCESS If the zone is successfully
-    /// added to the zone table.
-    /// \return \c result::EXIST The zone table already contains
-    /// zone of the same origin.
-    AddResult addZone(util::MemorySegment& mem_sgmt, dns::RRClass zone_class,
-                      const dns::Name& zone_name);
+    ///     added to the zone table.
+    /// \return \c result::EXIST The zone table already contained
+    ///     zone of the same origin. The old data is replaced and returned
+    ///     inside the result.
+    AddResult addZone(util::MemorySegment& mem_sgmt,
+                      dns::RRClass zone_class,
+                      const dns::Name& zone_name,
+                      ZoneData* content);
 
     /// Find a zone that best matches the given name in the \c ZoneTable.
     ///
@@ -185,16 +184,6 @@ public:
     /// \return A \c FindResult object enclosing the search result (see above).
     FindResult findZone(const isc::dns::Name& name) const;
 
-    /// Override the ZoneData for a node (zone) in the zone tree.
-    ///
-    /// \throw none
-    ///
-    /// \param name A domain name for which the zone data is set.
-    /// \param data The new zone data to set.
-    /// \return A \c FindResult object containing the old data if the
-    /// zone was found.
-    FindResult setZoneData(const isc::dns::Name& name, ZoneData* data);
-
 private:
     boost::interprocess::offset_ptr<ZoneTableTree> zones_;
 };
diff --git a/src/lib/datasrc/tests/memory/domaintree_unittest.cc b/src/lib/datasrc/tests/memory/domaintree_unittest.cc
index cb16e02..45e256a 100644
--- a/src/lib/datasrc/tests/memory/domaintree_unittest.cc
+++ b/src/lib/datasrc/tests/memory/domaintree_unittest.cc
@@ -256,8 +256,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // "g.h" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
+    EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 
     // fission the node "g.h"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -270,8 +270,8 @@ TEST_F(DomainTreeTest, subTreeRoot) {
 
     // "g.h" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("g.h"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("g.h"), &cdtnode));
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 }
 
 TEST_F(DomainTreeTest, additionalNodeFission) {
@@ -286,8 +286,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // "t.0" is not a subtree root
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_FALSE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
+    EXPECT_FALSE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 
     // fission the node "t.0"
     EXPECT_EQ(TestDomainTree::ALREADYEXISTS,
@@ -300,8 +300,8 @@ TEST_F(DomainTreeTest, additionalNodeFission) {
 
     // "t.0" should be a subtree root now.
     EXPECT_EQ(TestDomainTree::EXACTMATCH,
-              dtree_expose_empty_node.find(Name("t.0"), &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
+              dtree_expose_empty_node.find(Name("t.0"), &cdtnode));
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_SUBTREE_ROOT));
 }
 
 TEST_F(DomainTreeTest, findName) {
@@ -328,10 +328,10 @@ TEST_F(DomainTreeTest, findName) {
     EXPECT_EQ(TestDomainTree::PARTIALMATCH,
               dtree_expose_empty_node.find(Name("m.d.e.f"), &cdtnode));
 
-    // find dtnode
+    // find cdtnode
     EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("q.w.y.d.e.f"),
-                                                   &dtnode));
-    EXPECT_EQ(Name("q"), dtnode->getName());
+                                                   &cdtnode));
+    EXPECT_EQ(Name("q"), cdtnode->getName());
 }
 
 TEST_F(DomainTreeTest, findError) {
@@ -411,11 +411,12 @@ performCallbackTest(TestDomainTree& dtree,
                                                         Name("example"),
                                                         &parentdtnode));
     // the child/parent nodes shouldn't "inherit" the callback flag.
-    // "dtnode" may be invalid due to the insertion, so we need to re-find
-    // it.
+    // "dtnode" should still validly point to "callback.example", but we
+    // explicitly confirm it.
     EXPECT_EQ(TestDomainTree::EXACTMATCH, dtree.find(Name("callback.example"),
-                                                   &dtnode));
-    EXPECT_TRUE(dtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
+                                                     &cdtnode));
+    ASSERT_EQ(dtnode, cdtnode);
+    EXPECT_TRUE(cdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
     EXPECT_FALSE(subdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
     EXPECT_FALSE(parentdtnode->getFlag(TestDomainTreeNode::FLAG_CALLBACK));
 
diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
index 58979a4..004f663 100644
--- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc
+++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
@@ -543,29 +543,6 @@ TEST_F(MemoryClientTest, loadRRSIGs) {
     EXPECT_EQ(1, client_->getZoneCount());
 }
 
-TEST_F(MemoryClientTest, loadRRSIGsRdataMixedCoveredTypes) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    RRsetPtr rrset(new RRset(Name("example.org"),
-                             RRClass::IN(), RRType::A(), RRTTL(3600)));
-    rrset->addRdata(in::A("192.0.2.1"));
-    rrset->addRdata(in::A("192.0.2.2"));
-
-    RRsetPtr rrsig(new RRset(Name("example.org"), zclass_,
-                             RRType::RRSIG(), RRTTL(300)));
-    rrsig->addRdata(generic::RRSIG("A 5 3 3600 20000101000000 20000201000000 "
-                                   "12345 example.org. FAKEFAKEFAKE"));
-    rrsig->addRdata(generic::RRSIG("NS 5 3 3600 20000101000000 20000201000000 "
-                                   "54321 example.org. FAKEFAKEFAKEFAKE"));
-    rrset->addRRsig(rrsig);
-
-    EXPECT_THROW(client_->add(Name("example.org"), rrset),
-                 InMemoryClient::AddError);
-
-    // Teardown checks for memory segment leaks
-}
-
 TEST_F(MemoryClientTest, getZoneCount) {
     EXPECT_EQ(0, client_->getZoneCount());
     client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
@@ -655,75 +632,6 @@ TEST_F(MemoryClientTest, getIteratorGetSOAThrowsNotImplemented) {
     EXPECT_THROW(iterator->getSOA(), isc::NotImplemented);
 }
 
-TEST_F(MemoryClientTest, addRRsetToNonExistentZoneThrows) {
-    // The zone "example.org" doesn't exist, so we can't add an RRset to
-    // it.
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a), DataSourceError);
-}
-
-TEST_F(MemoryClientTest, addOutOfZoneThrows) {
-    // Out of zone names should throw.
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-empty.zone");
-
-    RRsetPtr rrset_a(new RRset(Name("a.example.com"),
-                               RRClass::IN(), RRType::A(), RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
-                 OutOfZone);
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, addNullRRsetThrows) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    EXPECT_THROW(client_->add(Name("example.org"), ConstRRsetPtr()),
-                 InMemoryClient::NullRRset);
-
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, addEmptyRRsetThrows) {
-    client_->load(Name("example.org"),
-                  TEST_DATA_DIR "/example.org-rrsigs.zone");
-
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    EXPECT_THROW(client_->add(Name("example.org"), rrset_a),
-                 InMemoryClient::AddError);
-
-    // Teardown checks for memory segment leaks
-}
-
-TEST_F(MemoryClientTest, add) {
-    client_->load(Name("example.org"), TEST_DATA_DIR "/example.org-empty.zone");
-
-    // Add another RRset
-    RRsetPtr rrset_a(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
-                               RRTTL(300)));
-    rrset_a->addRdata(rdata::in::A("192.0.2.1"));
-    client_->add(Name("example.org"), rrset_a);
-
-    ZoneIteratorPtr iterator(client_->getIterator(Name("example.org")));
-
-    // First we have the SOA
-    ConstRRsetPtr rrset(iterator->getNextRRset());
-    EXPECT_TRUE(rrset);
-    EXPECT_EQ(RRType::A(), rrset->getType());
-
-    rrset = iterator->getNextRRset();
-    EXPECT_TRUE(rrset);
-    EXPECT_EQ(RRType::SOA(), rrset->getType());
-
-    // There's nothing else in this zone
-    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
-}
-
 TEST_F(MemoryClientTest, findZoneData) {
     client_->load(Name("example.org"),
                   TEST_DATA_DIR "/example.org-rrsigs.zone");
@@ -774,4 +682,16 @@ TEST_F(MemoryClientTest, getJournalReaderNotImplemented) {
     EXPECT_THROW(client_->getJournalReader(Name("."), 0, 0),
                  isc::NotImplemented);
 }
+
+// TODO (upon merge of #2268): Re-add (and modify not to need
+// InMemoryClient::add) the tests removed in
+// 7a628baa1a158b5837d6f383e10b30542d2ac59b. Maybe some of them
+// are really not needed.
+//
+// * MemoryClientTest::loadRRSIGsRdataMixedCoveredTypes
+// * MemoryClientTest::addRRsetToNonExistentZoneThrows
+// * MemoryClientTest::addOutOfZoneThrows
+// * MemoryClientTest::addNullRRsetThrows
+// * MemoryClientTest::addEmptyRRsetThrows
+// * MemoryClientTest::add
 }
diff --git a/src/lib/datasrc/tests/memory/zone_data_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_unittest.cc
index d15fe8b..3c28cec 100644
--- a/src/lib/datasrc/tests/memory/zone_data_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_data_unittest.cc
@@ -108,7 +108,7 @@ void
 checkFindRdataSet(const ZoneTree& tree, const Name& name, RRType type,
                   const RdataSet* expected_set)
 {
-    ZoneNode* node = NULL;
+    const ZoneNode* node = NULL;
     tree.find(name, &node);
     ASSERT_NE(static_cast<ZoneNode*>(NULL), node);
     EXPECT_EQ(expected_set, RdataSet::find(node->getData(), type));
diff --git a/src/lib/datasrc/tests/memory/zone_table_unittest.cc b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
index 359df49..9cf1b34 100644
--- a/src/lib/datasrc/tests/memory/zone_table_unittest.cc
+++ b/src/lib/datasrc/tests/memory/zone_table_unittest.cc
@@ -22,6 +22,7 @@
 #include <datasrc/result.h>
 #include <datasrc/memory/zone_data.h>
 #include <datasrc/memory/zone_table.h>
+#include <datasrc/memory/segment_object_holder.h>
 
 #include <gtest/gtest.h>
 
@@ -30,6 +31,7 @@
 using namespace isc::dns;
 using namespace isc::datasrc;
 using namespace isc::datasrc::memory;
+using namespace isc::datasrc::memory::detail;
 
 namespace {
 // Memory segment specified for tests.  It normally behaves like a "local"
@@ -87,46 +89,89 @@ TEST_F(ZoneTableTest, create) {
 }
 
 TEST_F(ZoneTableTest, addZone) {
+    // It doesn't accept empty (NULL) zones
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, zname1, NULL),
+                 isc::BadValue);
+
+    SegmentObjectHolder<ZoneData, RRClass> holder1(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    const ZoneData* data1(holder1.get());
     // Normal successful case.
-    const ZoneTable::AddResult result1 =
-        zone_table->addZone(mem_sgmt_, zclass_, zname1);
+    const ZoneTable::AddResult result1(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder1.release()));
     EXPECT_EQ(result::SUCCESS, result1.code);
+    EXPECT_EQ(NULL, result1.zone_data);
+    // It got released by it
+    EXPECT_EQ(NULL, holder1.get());
 
     // Duplicate add doesn't replace the existing data.
-    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 zname1).code);
-    EXPECT_EQ(result1.zone_data,
-              zone_table->addZone(mem_sgmt_, zclass_, zname1).zone_data);
+    SegmentObjectHolder<ZoneData, RRClass> holder2(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    const ZoneTable::AddResult result2(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           zname1,
+                                                           holder2.release()));
+    EXPECT_EQ(result::EXIST, result2.code);
+    // The old one gets out
+    EXPECT_EQ(data1, result2.zone_data);
+    // It releases this one even when we replace the old zone
+    EXPECT_EQ(NULL, holder2.get());
+    // We need to release the old one manually
+    ZoneData::destroy(mem_sgmt_, result2.zone_data, zclass_);
+
+    SegmentObjectHolder<ZoneData, RRClass> holder3(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("EXAMPLE.COM")),
+                                    zclass_);
     // names are compared in a case insensitive manner.
-    EXPECT_EQ(result::EXIST, zone_table->addZone(mem_sgmt_, zclass_,
-                                                 Name("EXAMPLE.COM")).code);
+    const ZoneTable::AddResult result3(zone_table->addZone(mem_sgmt_, zclass_,
+                                                           Name("EXAMPLE.COM"),
+                                                           holder3.release()));
+    EXPECT_EQ(result::EXIST, result3.code);
+    ZoneData::destroy(mem_sgmt_, result3.zone_data, zclass_);
     // Add some more different ones.  Should just succeed.
+    SegmentObjectHolder<ZoneData, RRClass> holder4(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder4.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder5(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder5.release()).code);
 
     // Have the memory segment throw an exception in extending the internal
     // tree.  It still shouldn't cause memory leak (which would be detected
     // in TearDown()).
-    mem_sgmt_.setThrowCount(2);
-    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org")),
+    SegmentObjectHolder<ZoneData, RRClass> holder6(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("example.org")), zclass_);
+    mem_sgmt_.setThrowCount(1);
+    EXPECT_THROW(zone_table->addZone(mem_sgmt_, zclass_, Name("example.org"),
+                                     holder6.release()),
                  std::bad_alloc);
 }
 
 TEST_F(ZoneTableTest, findZone) {
-    const ZoneTable::AddResult add_result1 =
-        zone_table->addZone(mem_sgmt_, zclass_, zname1);
-    EXPECT_EQ(result::SUCCESS, add_result1.code);
+    SegmentObjectHolder<ZoneData, RRClass> holder1(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname1), zclass_);
+    ZoneData* zone_data = holder1.get();
+    EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_, zname1,
+                                                   holder1.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder2(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname2), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname2).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname2,
+                                  holder2.release()).code);
+    SegmentObjectHolder<ZoneData, RRClass> holder3(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, zname3), zclass_);
     EXPECT_EQ(result::SUCCESS,
-              zone_table->addZone(mem_sgmt_, zclass_, zname3).code);
+              zone_table->addZone(mem_sgmt_, zclass_, zname3,
+                                  holder3.release()).code);
 
     const ZoneTable::FindResult find_result1 =
         zone_table->findZone(Name("example.com"));
     EXPECT_EQ(result::SUCCESS, find_result1.code);
-    EXPECT_EQ(add_result1.zone_data, find_result1.zone_data);
+    EXPECT_EQ(zone_data, find_result1.zone_data);
 
     EXPECT_EQ(result::NOTFOUND,
               zone_table->findZone(Name("example.org")).code);
@@ -137,14 +182,17 @@ TEST_F(ZoneTableTest, findZone) {
     // and the code should be PARTIALMATCH.
     EXPECT_EQ(result::PARTIALMATCH,
               zone_table->findZone(Name("www.example.com")).code);
-    EXPECT_EQ(add_result1.zone_data,
+    EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 
     // make sure the partial match is indeed the longest match by adding
     // a zone with a shorter origin and query again.
+    SegmentObjectHolder<ZoneData, RRClass> holder4(
+        mem_sgmt_, ZoneData::create(mem_sgmt_, Name("com")), zclass_);
     EXPECT_EQ(result::SUCCESS, zone_table->addZone(mem_sgmt_, zclass_,
-                                                   Name("com")).code);
-    EXPECT_EQ(add_result1.zone_data,
+                                                   Name("com"),
+                                                   holder4.release()).code);
+    EXPECT_EQ(zone_data,
               zone_table->findZone(Name("www.example.com")).zone_data);
 }
 }



More information about the bind10-changes mailing list