BIND 10 trac2268, updated. 2150123bd60f00e070198b9c9717eb685dd0eebe [2268] Move ZoneDataLoader to a separate file

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Oct 12 03:40:48 UTC 2012


The branch, trac2268 has been updated
       via  2150123bd60f00e070198b9c9717eb685dd0eebe (commit)
       via  6bceaa250bef898410db1555341887b6a6f7e900 (commit)
       via  65c54c90ed874825b0418acade6179bd905bd470 (commit)
      from  c1ce5449aa72d538cbfca94f88cda0b8617081ba (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 2150123bd60f00e070198b9c9717eb685dd0eebe
Author: Mukund Sivaraman <muks at isc.org>
Date:   Fri Oct 12 09:10:38 2012 +0530

    [2268] Move ZoneDataLoader to a separate file

commit 6bceaa250bef898410db1555341887b6a6f7e900
Author: Mukund Sivaraman <muks at isc.org>
Date:   Fri Oct 12 09:10:15 2012 +0530

    [2268] Rename load method to loadInternal()

commit 65c54c90ed874825b0418acade6179bd905bd470
Author: Mukund Sivaraman <muks at isc.org>
Date:   Fri Oct 12 08:48:14 2012 +0530

    [2268] Add a comment about InMemoryClient construction after setThrowCount()

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

Summary of changes:
 src/lib/datasrc/memory/Makefile.am                 |    1 +
 src/lib/datasrc/memory/memory_client.cc            |   21 ++--
 src/lib/datasrc/memory/memory_client.h             |    8 +-
 src/lib/datasrc/memory/zone_data_loader.cc         |  108 ++++++++++++++++++++
 src/lib/datasrc/memory/zone_data_loader.h          |   85 +++++++++++++++
 src/lib/datasrc/memory/zone_data_updater.cc        |   81 ---------------
 src/lib/datasrc/memory/zone_data_updater.h         |   45 --------
 .../datasrc/tests/memory/memory_client_unittest.cc |    4 +
 8 files changed, 213 insertions(+), 140 deletions(-)
 create mode 100644 src/lib/datasrc/memory/zone_data_loader.cc
 create mode 100644 src/lib/datasrc/memory/zone_data_loader.h

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory/Makefile.am b/src/lib/datasrc/memory/Makefile.am
index 205f137..6fce958 100644
--- a/src/lib/datasrc/memory/Makefile.am
+++ b/src/lib/datasrc/memory/Makefile.am
@@ -22,6 +22,7 @@ libdatasrc_memory_la_SOURCES += zone_finder.h zone_finder.cc
 libdatasrc_memory_la_SOURCES += zone_table_segment.h zone_table_segment.cc
 libdatasrc_memory_la_SOURCES += zone_table_segment_local.h zone_table_segment_local.cc
 libdatasrc_memory_la_SOURCES += zone_data_updater.h zone_data_updater.cc
+libdatasrc_memory_la_SOURCES += zone_data_loader.h zone_data_loader.cc
 libdatasrc_memory_la_SOURCES += memory_client.h memory_client.cc
 
 nodist_libdatasrc_memory_la_SOURCES = memory_messages.h memory_messages.cc
diff --git a/src/lib/datasrc/memory/memory_client.cc b/src/lib/datasrc/memory/memory_client.cc
index d6b2803..df15f6a 100644
--- a/src/lib/datasrc/memory/memory_client.cc
+++ b/src/lib/datasrc/memory/memory_client.cc
@@ -22,6 +22,7 @@
 #include <datasrc/memory/treenode_rrset.h>
 #include <datasrc/memory/zone_finder.h>
 #include <datasrc/memory/zone_data_updater.h>
+#include <datasrc/memory/zone_data_loader.h>
 
 #include <util/memory_segment_local.h>
 
@@ -83,10 +84,10 @@ InMemoryClient::~InMemoryClient() {
 }
 
 result::Result
-InMemoryClient::load(const Name& zone_name,
-                     const string& filename,
-                     boost::function<void(internal::LoadCallback)>
-                         rrset_installer)
+InMemoryClient::loadInternal(const Name& zone_name,
+                             const string& filename,
+                             boost::function<void(internal::LoadCallback)>
+                             rrset_installer)
 {
     SegmentObjectHolder<ZoneData, RRClass> holder(
         mem_sgmt_, ZoneData::create(mem_sgmt_, zone_name), rrclass_);
@@ -214,17 +215,17 @@ InMemoryClient::load(const isc::dns::Name& zone_name,
     LOG_DEBUG(logger, DBG_TRACE_BASIC, DATASRC_MEMORY_MEM_LOAD).arg(zone_name).
         arg(filename);
 
-    return (load(zone_name, filename,
-                 boost::bind(masterLoadWrapper, filename.c_str(),
-                             zone_name, getClass(), _1)));
+    return (loadInternal(zone_name, filename,
+                         boost::bind(masterLoadWrapper, filename.c_str(),
+                                     zone_name, getClass(), _1)));
 }
 
 result::Result
 InMemoryClient::load(const isc::dns::Name& zone_name,
                      ZoneIterator& iterator) {
-    return (load(zone_name, string(),
-                 boost::bind(generateRRsetFromIterator,
-                             &iterator, _1)));
+    return (loadInternal(zone_name, string(),
+                         boost::bind(generateRRsetFromIterator,
+                                     &iterator, _1)));
 }
 
 const std::string
diff --git a/src/lib/datasrc/memory/memory_client.h b/src/lib/datasrc/memory/memory_client.h
index 9b3113e..1a17613 100644
--- a/src/lib/datasrc/memory/memory_client.h
+++ b/src/lib/datasrc/memory/memory_client.h
@@ -208,10 +208,10 @@ private:
     // another data source.
     // filename is the file name of the master file or empty if the zone is
     // loaded from another data source.
-    result::Result load(const isc::dns::Name& zone_name,
-                        const std::string& filename,
-                        boost::function<void(internal::LoadCallback)>
-                            rrset_installer);
+    result::Result loadInternal(const isc::dns::Name& zone_name,
+				const std::string& filename,
+				boost::function<void(internal::LoadCallback)>
+				rrset_installer);
 
     util::MemorySegment& mem_sgmt_;
     const isc::dns::RRClass rrclass_;
diff --git a/src/lib/datasrc/memory/zone_data_loader.cc b/src/lib/datasrc/memory/zone_data_loader.cc
new file mode 100644
index 0000000..86dc6da
--- /dev/null
+++ b/src/lib/datasrc/memory/zone_data_loader.cc
@@ -0,0 +1,108 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/memory/zone_data_loader.h>
+
+#include <dns/rdataclass.h>
+
+#include <boost/foreach.hpp>
+
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+void
+ZoneDataLoader::addFromLoad(const ConstRRsetPtr& rrset) {
+    // If we see a new name, flush the temporary holders, adding the
+    // pairs of RRsets and RRSIGs of the previous name to the zone.
+    if ((!node_rrsets_.empty() || !node_rrsigsets_.empty()) &&
+        (getCurrentName() != rrset->getName())) {
+        flushNodeRRsets();
+    }
+
+    // Store this RRset until it can be added to the zone.  The current
+    // implementation requires RRs of the same RRset should be added at
+    // once, so we check the "duplicate" here.
+    const bool is_rrsig = rrset->getType() == RRType::RRSIG();
+    NodeRRsets& node_rrsets = is_rrsig ? node_rrsigsets_ : node_rrsets_;
+    const RRType& rrtype = is_rrsig ? getCoveredType(rrset) : rrset->getType();
+    if (!node_rrsets.insert(NodeRRsetsVal(rrtype, rrset)).second) {
+        isc_throw(ZoneDataUpdater::AddError,
+                  "Duplicate add of the same type of"
+                  << (is_rrsig ? " RRSIG" : "") << " RRset: "
+                  << rrset->getName() << "/" << rrtype);
+    }
+
+    if (rrset->getRRsig()) {
+        addFromLoad(rrset->getRRsig());
+    }
+}
+
+void
+ZoneDataLoader::flushNodeRRsets() {
+    BOOST_FOREACH(NodeRRsetsVal val, node_rrsets_) {
+        // Identify the corresponding RRSIG for the RRset, if any.  If
+        // found add both the RRset and its RRSIG at once.
+        ConstRRsetPtr sig_rrset;
+        NodeRRsets::iterator sig_it = node_rrsigsets_.find(val.first);
+        if (sig_it != node_rrsigsets_.end()) {
+            sig_rrset = sig_it->second;
+            node_rrsigsets_.erase(sig_it);
+        }
+        updater_.add(val.second, sig_rrset);
+    }
+
+    // Right now, we don't accept RRSIG without covered RRsets (this
+    // should eventually allowed, but to do so we'll need to update the
+    // finder).
+    if (!node_rrsigsets_.empty()) {
+        isc_throw(ZoneDataUpdater::AddError,
+                  "RRSIG is added without covered RRset for "
+                  << getCurrentName());
+    }
+
+    node_rrsets_.clear();
+    node_rrsigsets_.clear();
+}
+
+RRType
+ZoneDataLoader::getCoveredType(const ConstRRsetPtr& sig_rrset) {
+    RdataIteratorPtr it = sig_rrset->getRdataIterator();
+    // Empty RRSIG shouldn't be passed either via a master file or
+    // another data source iterator, but it could still happen if the
+    // iterator has a bug.  We catch and reject such cases.
+    if (it->isLast()) {
+        isc_throw(isc::Unexpected,
+                  "Empty RRset is passed in-memory loader, name: "
+                  << sig_rrset->getName());
+    }
+    return (dynamic_cast<const generic::RRSIG&>(it->getCurrent()).
+            typeCovered());
+}
+
+const Name&
+ZoneDataLoader::getCurrentName() const {
+    if (!node_rrsets_.empty()) {
+        return (node_rrsets_.begin()->second->getName());
+    }
+    assert(!node_rrsigsets_.empty());
+    return (node_rrsigsets_.begin()->second->getName());
+}
+
+} // namespace memory
+} // namespace datasrc
+} // namespace isc
diff --git a/src/lib/datasrc/memory/zone_data_loader.h b/src/lib/datasrc/memory/zone_data_loader.h
new file mode 100644
index 0000000..2d1bdbb
--- /dev/null
+++ b/src/lib/datasrc/memory/zone_data_loader.h
@@ -0,0 +1,85 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DATASRC_ZONE_DATA_LOADER_H
+#define DATASRC_ZONE_DATA_LOADER_H 1
+
+#include <datasrc/memory/zone_data_updater.h>
+
+#include <datasrc/memory/zone_data.h>
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+#include <util/memory_segment.h>
+
+#include <boost/noncopyable.hpp>
+#include <map>
+
+namespace isc {
+namespace datasrc {
+namespace memory {
+
+// A helper internal class for load().  make it non-copyable to avoid
+// accidental copy.
+//
+// The current internal implementation expects that both a normal
+// (non RRSIG) RRset and (when signed) its RRSIG are added at once.
+// Also in the current implementation, the input sequence of RRsets
+// are grouped with their owner name (so once a new owner name is encountered,
+// no subsequent RRset has the previous owner name), but the ordering
+// in the same group is not fixed.  So we hold all RRsets of the same
+// owner name in node_rrsets_ and node_rrsigsets_, and add the matching
+// pairs of RRsets to the zone when we see a new owner name.
+//
+// The caller is responsible for adding the RRsets of the last group
+// in the input sequence by explicitly calling flushNodeRRsets() at the
+// end.  It's cleaner and more robust if we let the destructor of this class
+// do it, but since we cannot guarantee the adding operation is exception free,
+// we don't choose that option to maintain the common expectation for
+// destructors.
+class ZoneDataLoader : boost::noncopyable {
+public:
+    ZoneDataLoader(util::MemorySegment& mem_sgmt,
+                   const isc::dns::RRClass rrclass,
+                   const isc::dns::Name& zone_name, ZoneData& zone_data) :
+        updater_(mem_sgmt, rrclass, zone_name, zone_data)
+    {}
+
+    void addFromLoad(const isc::dns::ConstRRsetPtr& rrset);
+    void flushNodeRRsets();
+
+private:
+    typedef std::map<isc::dns::RRType, isc::dns::ConstRRsetPtr> NodeRRsets;
+    typedef NodeRRsets::value_type NodeRRsetsVal;
+
+    // A helper to identify the covered type of an RRSIG.
+    static isc::dns::RRType getCoveredType
+        (const isc::dns::ConstRRsetPtr& sig_rrset);
+    const isc::dns::Name& getCurrentName() const;
+
+private:
+    NodeRRsets node_rrsets_;
+    NodeRRsets node_rrsigsets_;
+    ZoneDataUpdater updater_;
+};
+
+} // namespace memory
+} // namespace datasrc
+} // namespace isc
+
+#endif // DATASRC_ZONE_DATA_LOADER_H
+
+// Local Variables:
+// mode: c++
+// End:
diff --git a/src/lib/datasrc/memory/zone_data_updater.cc b/src/lib/datasrc/memory/zone_data_updater.cc
index fd30fba..037eeb4 100644
--- a/src/lib/datasrc/memory/zone_data_updater.cc
+++ b/src/lib/datasrc/memory/zone_data_updater.cc
@@ -18,8 +18,6 @@
 
 #include <dns/rdataclass.h>
 
-#include <boost/foreach.hpp>
-
 using namespace isc::dns;
 using namespace isc::dns::rdata;
 
@@ -344,85 +342,6 @@ ZoneDataUpdater::add(const ConstRRsetPtr& rrset,
     addRdataSet(rrset, sig_rrset);
 }
 
-
-void
-ZoneDataLoader::addFromLoad(const ConstRRsetPtr& rrset) {
-    // If we see a new name, flush the temporary holders, adding the
-    // pairs of RRsets and RRSIGs of the previous name to the zone.
-    if ((!node_rrsets_.empty() || !node_rrsigsets_.empty()) &&
-        (getCurrentName() != rrset->getName())) {
-        flushNodeRRsets();
-    }
-
-    // Store this RRset until it can be added to the zone.  The current
-    // implementation requires RRs of the same RRset should be added at
-    // once, so we check the "duplicate" here.
-    const bool is_rrsig = rrset->getType() == RRType::RRSIG();
-    NodeRRsets& node_rrsets = is_rrsig ? node_rrsigsets_ : node_rrsets_;
-    const RRType& rrtype = is_rrsig ? getCoveredType(rrset) : rrset->getType();
-    if (!node_rrsets.insert(NodeRRsetsVal(rrtype, rrset)).second) {
-        isc_throw(ZoneDataUpdater::AddError,
-                  "Duplicate add of the same type of"
-                  << (is_rrsig ? " RRSIG" : "") << " RRset: "
-                  << rrset->getName() << "/" << rrtype);
-    }
-
-    if (rrset->getRRsig()) {
-        addFromLoad(rrset->getRRsig());
-    }
-}
-
-void
-ZoneDataLoader::flushNodeRRsets() {
-    BOOST_FOREACH(NodeRRsetsVal val, node_rrsets_) {
-        // Identify the corresponding RRSIG for the RRset, if any.  If
-        // found add both the RRset and its RRSIG at once.
-        ConstRRsetPtr sig_rrset;
-        NodeRRsets::iterator sig_it = node_rrsigsets_.find(val.first);
-        if (sig_it != node_rrsigsets_.end()) {
-            sig_rrset = sig_it->second;
-            node_rrsigsets_.erase(sig_it);
-        }
-        updater_.add(val.second, sig_rrset);
-    }
-
-    // Right now, we don't accept RRSIG without covered RRsets (this
-    // should eventually allowed, but to do so we'll need to update the
-    // finder).
-    if (!node_rrsigsets_.empty()) {
-        isc_throw(ZoneDataUpdater::AddError,
-                  "RRSIG is added without covered RRset for "
-                  << getCurrentName());
-    }
-
-    node_rrsets_.clear();
-    node_rrsigsets_.clear();
-}
-
-RRType
-ZoneDataLoader::getCoveredType(const ConstRRsetPtr& sig_rrset) {
-    RdataIteratorPtr it = sig_rrset->getRdataIterator();
-    // Empty RRSIG shouldn't be passed either via a master file or
-    // another data source iterator, but it could still happen if the
-    // iterator has a bug.  We catch and reject such cases.
-    if (it->isLast()) {
-        isc_throw(isc::Unexpected,
-                  "Empty RRset is passed in-memory loader, name: "
-                  << sig_rrset->getName());
-    }
-    return (dynamic_cast<const generic::RRSIG&>(it->getCurrent()).
-            typeCovered());
-}
-
-const Name&
-ZoneDataLoader::getCurrentName() const {
-    if (!node_rrsets_.empty()) {
-        return (node_rrsets_.begin()->second->getName());
-    }
-    assert(!node_rrsigsets_.empty());
-    return (node_rrsigsets_.begin()->second->getName());
-}
-
 } // namespace memory
 } // namespace datasrc
 } // namespace isc
diff --git a/src/lib/datasrc/memory/zone_data_updater.h b/src/lib/datasrc/memory/zone_data_updater.h
index 092668d..341d8ae 100644
--- a/src/lib/datasrc/memory/zone_data_updater.h
+++ b/src/lib/datasrc/memory/zone_data_updater.h
@@ -24,7 +24,6 @@
 #include <util/memory_segment.h>
 
 #include <boost/noncopyable.hpp>
-#include <map>
 
 namespace isc {
 namespace datasrc {
@@ -170,50 +169,6 @@ private:
     const isc::dns::NSEC3Hash* hash_;
 };
 
-// A helper internal class for load().  make it non-copyable to avoid
-// accidental copy.
-//
-// The current internal implementation expects that both a normal
-// (non RRSIG) RRset and (when signed) its RRSIG are added at once.
-// Also in the current implementation, the input sequence of RRsets
-// are grouped with their owner name (so once a new owner name is encountered,
-// no subsequent RRset has the previous owner name), but the ordering
-// in the same group is not fixed.  So we hold all RRsets of the same
-// owner name in node_rrsets_ and node_rrsigsets_, and add the matching
-// pairs of RRsets to the zone when we see a new owner name.
-//
-// The caller is responsible for adding the RRsets of the last group
-// in the input sequence by explicitly calling flushNodeRRsets() at the
-// end.  It's cleaner and more robust if we let the destructor of this class
-// do it, but since we cannot guarantee the adding operation is exception free,
-// we don't choose that option to maintain the common expectation for
-// destructors.
-class ZoneDataLoader : boost::noncopyable {
-public:
-    ZoneDataLoader(util::MemorySegment& mem_sgmt,
-                   const isc::dns::RRClass rrclass,
-                   const isc::dns::Name& zone_name, ZoneData& zone_data) :
-        updater_(mem_sgmt, rrclass, zone_name, zone_data)
-    {}
-
-    void addFromLoad(const isc::dns::ConstRRsetPtr& rrset);
-    void flushNodeRRsets();
-
-private:
-    typedef std::map<isc::dns::RRType, isc::dns::ConstRRsetPtr> NodeRRsets;
-    typedef NodeRRsets::value_type NodeRRsetsVal;
-
-    // A helper to identify the covered type of an RRSIG.
-    static isc::dns::RRType getCoveredType
-        (const isc::dns::ConstRRsetPtr& sig_rrset);
-    const isc::dns::Name& getCurrentName() const;
-
-private:
-    NodeRRsets node_rrsets_;
-    NodeRRsets node_rrsigsets_;
-    ZoneDataUpdater updater_;
-};
-
 } // namespace memory
 } // namespace datasrc
 } // namespace isc
diff --git a/src/lib/datasrc/tests/memory/memory_client_unittest.cc b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
index 766de2f..74d7aa8 100644
--- a/src/lib/datasrc/tests/memory/memory_client_unittest.cc
+++ b/src/lib/datasrc/tests/memory/memory_client_unittest.cc
@@ -373,6 +373,10 @@ TEST_F(MemoryClientTest, loadMemoryAllocationFailures) {
         SCOPED_TRACE("For throw count = " + i);
         mem_sgmt_.setThrowCount(i);
         EXPECT_THROW({
+            // Include the InMemoryClient construction too here. Now,
+            // even allocations done from InMemoryClient constructor
+            // fail (due to MemorySegmentTest throwing) and we check for
+            // leaks when this happens.
             InMemoryClient client2(mem_sgmt_, zclass_);
             client2.load(Name("example.org"),
                          TEST_DATA_DIR "/example.org.zone");



More information about the bind10-changes mailing list