BIND 10 trac1748, updated. a8965bdfe892d5aee1edddaaaad7d67cda7ea1f0 bug #1749: Address review comments (from bug #1748)

[BIND 10 source code commits]

The branch, trac1748 has been updated
       via  a8965bdfe892d5aee1edddaaaad7d67cda7ea1f0 (commit)
       via  aeeab53252ce4987e5f00ea459b179a3d9f5c71c (commit)
       via  17f4112ba9878d15199ddfd0ad973d3915441113 (commit)
      from  10b2753984535a28a799bb07e17e3b8305e6957f (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 a8965bdfe892d5aee1edddaaaad7d67cda7ea1f0
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Mar 13 09:18:49 2012 +0530

    bug #1749: Address review comments (from bug #1748)

commit aeeab53252ce4987e5f00ea459b179a3d9f5c71c
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Mar 13 09:15:22 2012 +0530

    bug #1748: Add one more testcase comparing a RRset with itself

commit 17f4112ba9878d15199ddfd0ad973d3915441113
Author: Mukund Sivaraman <muks at isc.org>
Date:   Tue Mar 13 09:13:02 2012 +0530

    bug #1748: Change code to use a condition expression

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

Summary of changes:
 src/lib/datasrc/rbnode_rrset.h                 |   21 ++++++++++++---------
 src/lib/datasrc/tests/rbnode_rrset_unittest.cc |    4 ++--
 src/lib/dns/rrset.cc                           |   15 +++++++--------
 src/lib/dns/rrset.h                            |    2 +-
 src/lib/dns/tests/rrset_unittest.cc            |    1 +
 5 files changed, 23 insertions(+), 20 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/rbnode_rrset.h b/src/lib/datasrc/rbnode_rrset.h
index db38a3e..a1f8886 100644
--- a/src/lib/datasrc/rbnode_rrset.h
+++ b/src/lib/datasrc/rbnode_rrset.h
@@ -124,15 +124,18 @@ public:
         isc_throw(isc::NotImplemented, "RBNodeRRset::setTTL() not supported");
     }
 
-    virtual bool isSameKind(const AbstractRRset& other) {
-        const AbstractRRset* t = &other;
-        const RBNodeRRset* rb;
-
-        rb = dynamic_cast<const RBNodeRRset*>(t);
-        if (rb)
-          return (this == rb);
-        else
-          return AbstractRRset::isSameKind(other);
+    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.
+
+        const RBNodeRRset* rb = dynamic_cast<const RBNodeRRset*>(&other);
+        if (rb != NULL) {
+            return (this == rb);
+        } else {
+            return (AbstractRRset::isSameKind(other));
+        }
     }
 
     virtual std::string toText() const {
diff --git a/src/lib/datasrc/tests/rbnode_rrset_unittest.cc b/src/lib/datasrc/tests/rbnode_rrset_unittest.cc
index 005f7d2..c653f44 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));
 }
diff --git a/src/lib/dns/rrset.cc b/src/lib/dns/rrset.cc
index 40a8ac1..9a55f5f 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 4d8ca65..7ad555f 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;
 
     //@}
 };
diff --git a/src/lib/dns/tests/rrset_unittest.cc b/src/lib/dns/tests/rrset_unittest.cc
index 6570be6..a10b515 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));