BIND 10 trac1605, updated. f88766fd3085974acaa0c3e7930f8295374a8467 [1605] Other changes as result of review

BIND 10 source code commits bind10-changes at lists.isc.org
Wed Feb 29 16:50:15 UTC 2012


The branch, trac1605 has been updated
       via  f88766fd3085974acaa0c3e7930f8295374a8467 (commit)
       via  776afd136b2675b928605368199af7dc130607d1 (commit)
      from  0169ac7327f43e82979102b0fbd87bf004dece16 (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 f88766fd3085974acaa0c3e7930f8295374a8467
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Feb 29 16:48:54 2012 +0000

    [1605] Other changes as result of review

commit 776afd136b2675b928605368199af7dc130607d1
Author: Stephen Morris <stephen at isc.org>
Date:   Wed Feb 29 12:37:09 2012 +0000

    [1605] Move RBNodeRRset to the isc::datasrc::internal namespace

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

Summary of changes:
 src/lib/datasrc/memory_datasrc.cc                |    2 +-
 src/lib/datasrc/rbnode_rrset.h                   |   21 ++++---
 src/lib/datasrc/tests/memory_datasrc_unittest.cc |   28 +++++---
 src/lib/datasrc/tests/rbnode_rrset_unittest.cc   |   73 ++++++++++------------
 4 files changed, 64 insertions(+), 60 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/memory_datasrc.cc b/src/lib/datasrc/memory_datasrc.cc
index bdaf2e6..3cd52ab 100644
--- a/src/lib/datasrc/memory_datasrc.cc
+++ b/src/lib/datasrc/memory_datasrc.cc
@@ -432,7 +432,7 @@ struct InMemoryZoneFinder::InMemoryZoneFinderImpl {
         // ... although instead of loading the RRset directly, we encapsulate
         // it within an RBNodeRRset.  This contains additional information that
         // speeds up queries.
-        ConstRRsetPtr rrset(new RBNodeRRset(rawrrset));
+        ConstRRsetPtr rrset(new internal::RBNodeRRset(rawrrset));
 
         if (rrset->getType() == RRType::NSEC3()) {
             return (addNSEC3(rrset, zone_data));
diff --git a/src/lib/datasrc/rbnode_rrset.h b/src/lib/datasrc/rbnode_rrset.h
index 667ce93..87e09ee 100644
--- a/src/lib/datasrc/rbnode_rrset.h
+++ b/src/lib/datasrc/rbnode_rrset.h
@@ -27,6 +27,7 @@
 
 namespace isc {
 namespace datasrc {
+namespace internal {
 
 /// \brief Special RRset for optimizing memory datasource requirement
 ///
@@ -41,11 +42,11 @@ namespace datasrc {
 /// - Calls to methods that change attributes of the underlying RRset (such as
 ///   TTL or Name) cause an exception to be thrown.  The in-memory data source
 ///   does not allow modification of these attributes.
-/// - Calls that modify the associated RRSIGs of the RRset are allowed (even
-///   though the pointer is to a "const" object).  The reason here is because
-///   RRSIGs are added to the in-memory data source after the RBNodeRRset
-///   objects have been created.  Thus there has to be the capability of
-///   modifying this information.
+/// - Calls that add the pointer to the associated RRSIG to the RRset are
+///   allowed (even though the pointer is to a "const" RRset).  The reason here
+///   is that RRSIGs are added to the in-memory data source after the
+///   RBNodeRRset objects have been created.  Thus there has to be the
+///   capability of modifying this information.
 ///
 /// The class is not derived from RRset itself to simplify coding: part of the
 /// loading of the memory data source is handled in the BIND 10 "libdns++"
@@ -60,8 +61,8 @@ class RBNodeRRset : public isc::dns::AbstractRRset {
 private:
     // Note: The copy constructor and the assignment operator are intentionally
     // defined as private as we would normally not duplicate a RBNodeRRset.
-    // (We use the "private" method instead of inheriting from boost::noncopyable
-    // so as to avoid multiple inheritance.)
+    // (We use the "private" method instead of inheriting from
+    // boost::noncopyable so as to avoid multiple inheritance.)
     RBNodeRRset(const RBNodeRRset& source);
     RBNodeRRset& operator=(const RBNodeRRset& source);
 
@@ -71,7 +72,8 @@ public:
     /// Creates an RBNodeRRset from the pointer to the RRset passed to it.
     ///
     /// \param rrset Pointer to underlying RRset encapsulated by this object.
-    RBNodeRRset(const isc::dns::ConstRRsetPtr& rrset) : rrset_(rrset) {}
+    explicit RBNodeRRset(const isc::dns::ConstRRsetPtr& rrset) : rrset_(rrset)
+    {}
 
     /// \brief Destructor
     virtual ~RBNodeRRset() {}
@@ -180,7 +182,7 @@ public:
     /// \brief Return underlying RRset pointer
     ///
     /// ... mainly for testing.
-    virtual isc::dns::ConstRRsetPtr getUnderlyingRRset() const {
+    isc::dns::ConstRRsetPtr getUnderlyingRRset() const {
         return (rrset_);
     }
 
@@ -188,6 +190,7 @@ private:
     isc::dns::ConstRRsetPtr rrset_;     ///< Underlying RRset
 };
 
+}   // namespace internal
 }   // namespace datasrc
 }   // namespace isc
 
diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
index e8a62fc..954d7cc 100644
--- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc
+++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
@@ -172,19 +172,25 @@ TEST_F(InMemoryClientTest, iterator) {
     EXPECT_EQ(result::SUCCESS, zone->add(aRRsetA));
     EXPECT_EQ(result::SUCCESS, zone->add(aRRsetAAAA));
     EXPECT_EQ(result::SUCCESS, zone->add(subRRsetA));
-    // Check it with full zone, one by one.
-    // It should be in ascending order in case of InMemory data source
-    // (isn't guaranteed in general)
-    // Since the in-memory data source uses objects that encapsulate the
-    // RRsets stored, the iterator does not iterate over the RRsets stored -
-    // it iterates over the encapsulating objects.  This means that we cannot
-    // directly compare pointer values: instead, we compare the actual data
-    // stored.
+
+    // Check it with full zone.
+    vector<ConstRRsetPtr> expected_rrsets;
+    expected_rrsets.push_back(aRRsetA);
+    expected_rrsets.push_back(aRRsetAAAA);
+    expected_rrsets.push_back(subRRsetA);
+
     iterator = memory_client.getIterator(Name("a"));
-    EXPECT_EQ(aRRsetA->toText(), iterator->getNextRRset()->toText());
-    EXPECT_EQ(aRRsetAAAA->toText(), iterator->getNextRRset()->toText());
-    EXPECT_EQ(subRRsetA->toText(), iterator->getNextRRset()->toText());
+    vector<ConstRRsetPtr> actual_rrsets;
+    for (int i = 0; i < 3; ++i) {
+        ConstRRsetPtr actual = iterator->getNextRRset();
+        ASSERT_NE(ConstRRsetPtr(), actual);
+        actual_rrsets.push_back(actual);
+    }
     EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
+
+    rrsetsCheck(expected_rrsets.begin(), expected_rrsets.end(),
+                actual_rrsets.begin(), actual_rrsets.end());
+
 }
 
 TEST_F(InMemoryClientTest, iterator_separate_rrs) {
diff --git a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
index 5ef4dc4..9fc1403 100644
--- a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
+++ b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
@@ -17,6 +17,7 @@
 #include <exceptions/exceptions.h>
 #include <dns/rdataclass.h>
 #include <datasrc/rbnode_rrset.h>
+#include <testutils/dnsmessage_test.h>
 
 #include <gtest/gtest.h>
 
@@ -24,12 +25,14 @@
 
 using isc::UnitTestUtil;
 
-using namespace std;
 using namespace isc;
 using namespace isc::datasrc;
+using namespace isc::datasrc::internal;
 using namespace isc::dns;
 using namespace isc::dns::rdata;
+using namespace isc::testutils;
 using namespace isc::util;
+using namespace std;
 
 // These tests are very similar to those for RRset - indeed, this file was
 // created from those tests.  However, the significant difference in behaviour
@@ -135,33 +138,36 @@ TEST_F(RBNodeRRsetTest, toText) {
     EXPECT_THROW(rrset_a_empty.toText(), EmptyRRset);
 }
 
-TEST_F(RBNodeRRsetTest, toWireRenderer) {
-    OutputBuffer buffer(0);
-    MessageRenderer renderer(buffer);
-    rrset_a.toWire(renderer);
+// Note: although the next two tests are essentially the same and used common
+// test code, they use different test data: the MessageRenderer produces
+// compressed wire data whereas the OutputBuffer does not.
+
+template <typename T>
+void
+performToWireTest(T& dataHolder, const RBNodeRRset& rrset,
+                  const RBNodeRRset& rrset_empty, const char* testdata)
+{
+    rrset.toWire(dataHolder);
 
     std::vector<unsigned char> wiredata;
-    UnitTestUtil::readWireData("rrset_toWire2", wiredata);
-    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
-                        buffer.getLength(), &wiredata[0], wiredata.size());
+    UnitTestUtil::readWireData(testdata, wiredata);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, dataHolder.getData(),
+                        dataHolder.getLength(), &wiredata[0], wiredata.size());
 
     // toWire() cannot be performed for an empty RRset.
-    renderer.clear();
-    EXPECT_THROW(rrset_a_empty.toWire(renderer), EmptyRRset);
+    dataHolder.clear();
+    EXPECT_THROW(rrset_empty.toWire(dataHolder), EmptyRRset);
 }
 
-TEST_F(RBNodeRRsetTest, toWireBuffer) {
+TEST_F(RBNodeRRsetTest, toWireRenderer) {
     OutputBuffer buffer(0);
-    rrset_a.toWire(buffer);
-
-    std::vector<unsigned char> wiredata;
-    UnitTestUtil::readWireData("rrset_toWire1", wiredata);
-    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, buffer.getData(),
-                        buffer.getLength(), &wiredata[0], wiredata.size());
+    MessageRenderer renderer(buffer);
+    performToWireTest(renderer, rrset_a, rrset_a_empty, "rrset_toWire2");
+}
 
-    // toWire() cannot be performed for an empty RRset.
-    buffer.clear();
-    EXPECT_THROW(rrset_a_empty.toWire(buffer), EmptyRRset);
+TEST_F(RBNodeRRsetTest, toWireBuffer) {
+    OutputBuffer buffer(0);
+    performToWireTest(buffer, rrset_a, rrset_a_empty, "rrset_toWire1");
 }
 
 TEST_F(RBNodeRRsetTest, addRdata) {
@@ -204,24 +210,13 @@ TEST_F(RBNodeRRsetTest, LeftShiftOperator) {
               "test.example.com. 3600 IN A 192.0.2.2\n", oss.str());
 }
 
-// General RRSIG check function.  Get the RRSIG from the RRset and check that
-// the RDATA is what we expect.
-void
-checkSignature(const RBNodeRRset& rrset) {
-    RRsetPtr rrsig = rrset.getRRsig();
-    ASSERT_TRUE(rrsig);
-    RdataIteratorPtr it = rrsig->getRdataIterator();
-    EXPECT_EQ(RRSIG_TXT, it->getCurrent().toText());
-}
-
 // addRRSIG tests.
 TEST_F(RBNodeRRsetTest, addRRsigConstRdataPointer) {
     EXPECT_FALSE(rrset_a.getRRsig());
-    RdataPtr data = createRdata(rrset_siga->getType(), rrset_siga->getClass(),
-                                RRSIG_TXT);
-    ConstRdataPtr cdata(data);
-    rrset_a.addRRsig(cdata);
-    checkSignature(rrset_a);
+    ConstRdataPtr data = createRdata(rrset_siga->getType(),
+                                     rrset_siga->getClass(), RRSIG_TXT);
+    rrset_a.addRRsig(data);
+    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigRdataPointer) {
@@ -229,19 +224,19 @@ TEST_F(RBNodeRRsetTest, addRRsigRdataPointer) {
     RdataPtr data = createRdata(rrset_siga->getType(), rrset_siga->getClass(),
                                 RRSIG_TXT);
     rrset_a.addRRsig(data);
-    checkSignature(rrset_a);
+    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigAbstractRRset) {
     EXPECT_FALSE(rrset_a.getRRsig());
     rrset_a.addRRsig(*(rrset_siga.get()));
-    checkSignature(rrset_a);
+    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigConstantRRsetPointer) {
     EXPECT_FALSE(rrset_a.getRRsig());
     rrset_a.addRRsig(rrset_siga);
-    checkSignature(rrset_a);
+    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigRRsetPointer) {
@@ -250,7 +245,7 @@ TEST_F(RBNodeRRsetTest, addRRsigRRsetPointer) {
                    RRTTL(3600)));
     rrsig->addRdata(generic::RRSIG(RRSIG_TXT));
     rrset_a.addRRsig(rrsig);
-    checkSignature(rrset_a);
+    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, removeRRsig) {



More information about the bind10-changes mailing list