From: Stephen Morris <stephen@isc.org>
Date: Thu, 1 Mar 2012 18:18:35 +0000 (+0000)
Subject: [1605] Minor changes as a result of (re-)review
X-Git-Tag: trac2351_base~226^2~116^2~136^2~1^2~2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=9e89c7638a259fd103316f2b7f9be539b2cf3dce;p=thirdparty%2Fkea.git

[1605] Minor changes as a result of (re-)review
---

diff --git a/src/lib/datasrc/rbnode_rrset.h b/src/lib/datasrc/rbnode_rrset.h
index 87e09ee6bc..968e7a83ff 100644
--- a/src/lib/datasrc/rbnode_rrset.h
+++ b/src/lib/datasrc/rbnode_rrset.h
@@ -41,7 +41,14 @@ namespace internal {
 ///
 /// - 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.
+///   does not allow modification of these attributes.  In theory, it is a bad
+///   practice in that it doesn't preserve the assumed behavior of the base
+///   class.  In practice, however, it should be acceptable because this
+///   class is effectively hidden from applications and will only be given
+///   to them as a const pointer to the base class via find() variants.
+///   So the application cannot call non const methods anyway unless it
+///   intentionally breaks the constness.
+///
 /// - 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
@@ -51,9 +58,14 @@ namespace internal {
 /// 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++"
 /// code, which creates RRsets and passes them to the data source code.  This
-/// does not have to be altered if encapsulation, rather than inheritcance, is
+/// does not have to be altered if encapsulation, rather than inheritance, is
 /// used.
-
+///
+/// \note This class is exposed in this separate header file so that test code
+/// can refer to its definition, and only for that purpose.  Otherwise this is
+/// essentially a private class of the in-memory data source implementation,
+/// and an application shouldn't directly refer to this class.
+/// 
 // Note: non-Doxygen-documented methods are documented in the base class.
 
 class RBNodeRRset : public isc::dns::AbstractRRset {
diff --git a/src/lib/datasrc/tests/memory_datasrc_unittest.cc b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
index 954d7cc0c9..c1563192e6 100644
--- a/src/lib/datasrc/tests/memory_datasrc_unittest.cc
+++ b/src/lib/datasrc/tests/memory_datasrc_unittest.cc
@@ -181,12 +181,10 @@ TEST_F(InMemoryClientTest, iterator) {
 
     iterator = memory_client.getIterator(Name("a"));
     vector<ConstRRsetPtr> actual_rrsets;
-    for (int i = 0; i < 3; ++i) {
-        ConstRRsetPtr actual = iterator->getNextRRset();
-        ASSERT_NE(ConstRRsetPtr(), actual);
+    ConstRRsetPtr actual;
+    while ((actual = iterator->getNextRRset()) != NULL) {
         actual_rrsets.push_back(actual);
     }
-    EXPECT_EQ(ConstRRsetPtr(), iterator->getNextRRset());
 
     rrsetsCheck(expected_rrsets.begin(), expected_rrsets.end(),
                 actual_rrsets.begin(), actual_rrsets.end());
diff --git a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
index 9fc1403779..421da9f38c 100644
--- a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
+++ b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
@@ -216,7 +216,7 @@ TEST_F(RBNodeRRsetTest, addRRsigConstRdataPointer) {
     ConstRdataPtr data = createRdata(rrset_siga->getType(),
                                      rrset_siga->getClass(), RRSIG_TXT);
     rrset_a.addRRsig(data);
-    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
+    rrsetCheck(rrset_siga, rrset_a.getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigRdataPointer) {
@@ -224,19 +224,19 @@ TEST_F(RBNodeRRsetTest, addRRsigRdataPointer) {
     RdataPtr data = createRdata(rrset_siga->getType(), rrset_siga->getClass(),
                                 RRSIG_TXT);
     rrset_a.addRRsig(data);
-    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
+    rrsetCheck(rrset_siga, rrset_a.getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigAbstractRRset) {
     EXPECT_FALSE(rrset_a.getRRsig());
     rrset_a.addRRsig(*(rrset_siga.get()));
-    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
+    rrsetCheck(rrset_siga, rrset_a.getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigConstantRRsetPointer) {
     EXPECT_FALSE(rrset_a.getRRsig());
     rrset_a.addRRsig(rrset_siga);
-    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
+    rrsetCheck(rrset_siga, rrset_a.getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, addRRsigRRsetPointer) {
@@ -245,7 +245,7 @@ TEST_F(RBNodeRRsetTest, addRRsigRRsetPointer) {
                    RRTTL(3600)));
     rrsig->addRdata(generic::RRSIG(RRSIG_TXT));
     rrset_a.addRRsig(rrsig);
-    rrsetCheck(rrset_siga, rrset_a.getUnderlyingRRset()->getRRsig());
+    rrsetCheck(rrset_siga, rrset_a.getRRsig());
 }
 
 TEST_F(RBNodeRRsetTest, removeRRsig) {