From 64eef65d27bf5310afa53cac418c18bc8a989b31 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Fri, 9 Mar 2012 12:29:23 +0530 Subject: [PATCH 1/6] [1748] Define AbstractRRset::isSameKind() and implement the default version --- src/lib/dns/rrset.cc | 11 +++++++++++ src/lib/dns/rrset.h | 8 ++++++++ src/lib/dns/tests/rrset_unittest.cc | 13 +++++++++++++ 3 files changed, 32 insertions(+) diff --git a/src/lib/dns/rrset.cc b/src/lib/dns/rrset.cc index 776d49f650..40a8ac1115 100644 --- a/src/lib/dns/rrset.cc +++ b/src/lib/dns/rrset.cc @@ -113,6 +113,17 @@ AbstractRRset::toWire(AbstractMessageRenderer& renderer) const { return (rrs_written); } +bool +AbstractRRset::isSameKind(const AbstractRRset& other) { + if (getType() != other.getType()) + return false; + if (getClass() != other.getClass()) + return false; + if (getName() != other.getName()) + return false; + return true; +} + ostream& operator<<(ostream& os, const AbstractRRset& rrset) { os << rrset.toText(); diff --git a/src/lib/dns/rrset.h b/src/lib/dns/rrset.h index 5042a98f55..4d8ca6549c 100644 --- a/src/lib/dns/rrset.h +++ b/src/lib/dns/rrset.h @@ -475,6 +475,14 @@ public: /// \brief Clear the RRSIGs for this RRset virtual void removeRRsig() = 0; + /// \brief Check whether two RRsets are of the same kind + /// + /// Checks if two RRsets have the same name, RR type, and RR class. + /// + /// \param other Pointer to another AbstractRRset to compare + /// against. + virtual bool isSameKind(const AbstractRRset& other); + //@} }; diff --git a/src/lib/dns/tests/rrset_unittest.cc b/src/lib/dns/tests/rrset_unittest.cc index a6bfe5695b..6570be678c 100644 --- a/src/lib/dns/tests/rrset_unittest.cc +++ b/src/lib/dns/tests/rrset_unittest.cc @@ -112,6 +112,19 @@ TEST_F(RRsetTest, setName) { EXPECT_EQ(test_nsname, rrset_a.getName()); } +TEST_F(RRsetTest, isSameKind) { + RRset rrset_w(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)); + RRset rrset_x(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)); + RRset rrset_y(test_name, RRClass::IN(), RRType::NS(), RRTTL(3600)); + RRset rrset_z(test_name, RRClass::CH(), RRType::A(), RRTTL(3600)); + RRset rrset_p(test_nsname, RRClass::IN(), RRType::A(), RRTTL(3600)); + + EXPECT_TRUE(rrset_w.isSameKind(rrset_x)); + EXPECT_FALSE(rrset_w.isSameKind(rrset_y)); + EXPECT_FALSE(rrset_w.isSameKind(rrset_z)); + EXPECT_FALSE(rrset_w.isSameKind(rrset_p)); +} + void addRdataTestCommon(const RRset& rrset) { EXPECT_EQ(2, rrset.getRdataCount()); From 216b2e18c9811ec25dc99ae9b320140033e26a36 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Sun, 11 Mar 2012 14:14:51 +0530 Subject: [PATCH 2/6] [1749] Implement a specialized RBNodeRRset::isSameKind() --- src/lib/datasrc/rbnode_rrset.h | 11 +++++++++++ src/lib/datasrc/tests/rbnode_rrset_unittest.cc | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/lib/datasrc/rbnode_rrset.h b/src/lib/datasrc/rbnode_rrset.h index f687919630..f0e1f6fd21 100644 --- a/src/lib/datasrc/rbnode_rrset.h +++ b/src/lib/datasrc/rbnode_rrset.h @@ -127,6 +127,17 @@ public: virtual std::string toText() const; + virtual bool isSameKind(const AbstractRRset& other) { + const AbstractRRset* t = &other; + const RBNodeRRset* rb; + + rb = dynamic_cast(t); + if (rb) + return (this == rb); + else + return AbstractRRset::isSameKind(other); + } + virtual unsigned int toWire( isc::dns::AbstractMessageRenderer& renderer) const; diff --git a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc index a46d9cfe3f..005f7d2c91 100644 --- a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc +++ b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc @@ -138,6 +138,23 @@ TEST_F(RBNodeRRsetTest, toText) { EXPECT_THROW(rrset_a_empty.toText(), EmptyRRset); } +TEST_F(RBNodeRRsetTest, isSameKind) { + RBNodeRRset rrset_p(ConstRRsetPtr(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)))); + RBNodeRRset rrset_q(ConstRRsetPtr(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)))); + RRset rrset_w(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)); + RRset rrset_x(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)); + RRset rrset_y(test_name, RRClass::IN(), RRType::NS(), RRTTL(3600)); + RRset rrset_z(test_name, RRClass::CH(), RRType::A(), RRTTL(3600)); + + EXPECT_TRUE(rrset_p.isSameKind(rrset_p)); + EXPECT_FALSE(rrset_p.isSameKind(rrset_q)); + + EXPECT_TRUE(rrset_p.isSameKind(rrset_w)); + EXPECT_TRUE(rrset_p.isSameKind(rrset_x)); + EXPECT_FALSE(rrset_p.isSameKind(rrset_y)); + EXPECT_FALSE(rrset_p.isSameKind(rrset_z)); +} + // 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. From 1007c99c026d5d7f51a95061d25f59d621441b39 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 13 Mar 2012 09:13:02 +0530 Subject: [PATCH 3/6] bug #1748: Change code to use a condition expression --- src/lib/dns/rrset.cc | 15 +++++++-------- src/lib/dns/rrset.h | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/lib/dns/rrset.cc b/src/lib/dns/rrset.cc index 40a8ac1115..9a55f5f55c 100644 --- a/src/lib/dns/rrset.cc +++ b/src/lib/dns/rrset.cc @@ -114,14 +114,13 @@ AbstractRRset::toWire(AbstractMessageRenderer& renderer) const { } bool -AbstractRRset::isSameKind(const AbstractRRset& other) { - if (getType() != other.getType()) - return false; - if (getClass() != other.getClass()) - return false; - if (getName() != other.getName()) - return false; - return true; +AbstractRRset::isSameKind(const AbstractRRset& other) const { + // Compare classes last as they're likely to be identical. Compare + // names late in the list too, as these are expensive. So we compare + // types first, names second and classes last. + return (getType() == other.getType() && + getName() == other.getName() && + getClass() == other.getClass()); } ostream& diff --git a/src/lib/dns/rrset.h b/src/lib/dns/rrset.h index 4d8ca6549c..7ad555ff74 100644 --- a/src/lib/dns/rrset.h +++ b/src/lib/dns/rrset.h @@ -481,7 +481,7 @@ public: /// /// \param other Pointer to another AbstractRRset to compare /// against. - virtual bool isSameKind(const AbstractRRset& other); + virtual bool isSameKind(const AbstractRRset& other) const; //@} }; From 02db7759b655b9eb3e5d4e750e5f5bf212e1157f Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 13 Mar 2012 09:15:22 +0530 Subject: [PATCH 4/6] bug #1748: Add one more testcase comparing a RRset with itself --- src/lib/dns/tests/rrset_unittest.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/dns/tests/rrset_unittest.cc b/src/lib/dns/tests/rrset_unittest.cc index 6570be678c..a10b515339 100644 --- a/src/lib/dns/tests/rrset_unittest.cc +++ b/src/lib/dns/tests/rrset_unittest.cc @@ -119,6 +119,7 @@ TEST_F(RRsetTest, isSameKind) { RRset rrset_z(test_name, RRClass::CH(), RRType::A(), RRTTL(3600)); RRset rrset_p(test_nsname, RRClass::IN(), RRType::A(), RRTTL(3600)); + EXPECT_TRUE(rrset_w.isSameKind(rrset_w)); EXPECT_TRUE(rrset_w.isSameKind(rrset_x)); EXPECT_FALSE(rrset_w.isSameKind(rrset_y)); EXPECT_FALSE(rrset_w.isSameKind(rrset_z)); From ac2c63f07c78ba94fb8b76ce1051a5810b97a99a Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Tue, 13 Mar 2012 09:18:49 +0530 Subject: [PATCH 5/6] bug #1749: Address review comments (from bug #1748) --- src/lib/datasrc/rbnode_rrset.h | 19 +++++++++++-------- .../datasrc/tests/rbnode_rrset_unittest.cc | 4 ++-- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/lib/datasrc/rbnode_rrset.h b/src/lib/datasrc/rbnode_rrset.h index f0e1f6fd21..3e5d20a803 100644 --- a/src/lib/datasrc/rbnode_rrset.h +++ b/src/lib/datasrc/rbnode_rrset.h @@ -127,15 +127,18 @@ public: virtual std::string toText() const; - virtual bool isSameKind(const AbstractRRset& other) { - const AbstractRRset* t = &other; - const RBNodeRRset* rb; + virtual bool isSameKind(const AbstractRRset& other) const { + // This code is an optimisation for comparing + // RBNodeRRsets. However, in doing this optimisation, + // semantically the code is not "is same kind" but is instead + // "is identical object" in the case where RBNodeRRsets are compared. - rb = dynamic_cast(t); - if (rb) - return (this == rb); - else - return AbstractRRset::isSameKind(other); + const RBNodeRRset* rb = dynamic_cast(&other); + if (rb != NULL) { + return (this == rb); + } else { + return (AbstractRRset::isSameKind(other)); + } } virtual unsigned int toWire( diff --git a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc index 005f7d2c91..c653f44653 100644 --- a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc +++ b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc @@ -142,7 +142,7 @@ TEST_F(RBNodeRRsetTest, isSameKind) { RBNodeRRset rrset_p(ConstRRsetPtr(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)))); RBNodeRRset rrset_q(ConstRRsetPtr(new RRset(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)))); RRset rrset_w(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)); - RRset rrset_x(test_name, RRClass::IN(), RRType::A(), RRTTL(3600)); + RRset rrset_x(test_nsname, RRClass::IN(), RRType::A(), RRTTL(3600)); RRset rrset_y(test_name, RRClass::IN(), RRType::NS(), RRTTL(3600)); RRset rrset_z(test_name, RRClass::CH(), RRType::A(), RRTTL(3600)); @@ -150,7 +150,7 @@ TEST_F(RBNodeRRsetTest, isSameKind) { EXPECT_FALSE(rrset_p.isSameKind(rrset_q)); EXPECT_TRUE(rrset_p.isSameKind(rrset_w)); - EXPECT_TRUE(rrset_p.isSameKind(rrset_x)); + EXPECT_FALSE(rrset_p.isSameKind(rrset_x)); EXPECT_FALSE(rrset_p.isSameKind(rrset_y)); EXPECT_FALSE(rrset_p.isSameKind(rrset_z)); } From 06d963e3098ea6ead0eae1b54f178222b9fff479 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Wed, 14 Mar 2012 09:02:36 +0530 Subject: [PATCH 6/6] Update .gitignore files --- src/lib/dns/benchmarks/.gitignore | 1 + src/lib/log/tests/.gitignore | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/lib/dns/benchmarks/.gitignore b/src/lib/dns/benchmarks/.gitignore index a6f1fac443..c1c840b4de 100644 --- a/src/lib/dns/benchmarks/.gitignore +++ b/src/lib/dns/benchmarks/.gitignore @@ -1 +1,2 @@ +/message_renderer_bench /rdatarender_bench diff --git a/src/lib/log/tests/.gitignore b/src/lib/log/tests/.gitignore index 8d7e5645a0..41b863bbef 100644 --- a/src/lib/log/tests/.gitignore +++ b/src/lib/log/tests/.gitignore @@ -2,6 +2,8 @@ /destination_test.sh /init_logger_test /init_logger_test.sh +/initializer_unittests_1 +/initializer_unittests_2 /local_file_test.sh /logger_example /run_unittests