[svn] commit: r3389 - in /branches/trac356/src/lib/nsas: ./ tests/

BIND 10 source code commits bind10-changes at lists.isc.org
Thu Oct 28 10:53:42 UTC 2010


Author: stephen
Date: Thu Oct 28 10:53:42 2010
New Revision: 3389

Log:
a) Take account of the DNS class in calculating the hash.  The HashKey class has
been created for this.
b) Move the IOAddress object into the correct namespace.

Added:
    branches/trac356/src/lib/nsas/hash_key.h
    branches/trac356/src/lib/nsas/tests/hash_key_unittest.cc
Modified:
    branches/trac356/src/lib/nsas/Makefile.am
    branches/trac356/src/lib/nsas/address_entry.h
    branches/trac356/src/lib/nsas/asiolink.h
    branches/trac356/src/lib/nsas/hash.cc
    branches/trac356/src/lib/nsas/hash.h
    branches/trac356/src/lib/nsas/hash_table.h
    branches/trac356/src/lib/nsas/nameserver_entry.cc
    branches/trac356/src/lib/nsas/nameserver_entry.h
    branches/trac356/src/lib/nsas/tests/Makefile.am
    branches/trac356/src/lib/nsas/tests/address_entry_unittest.cc
    branches/trac356/src/lib/nsas/tests/hash_table_unittest.cc
    branches/trac356/src/lib/nsas/tests/hash_unittest.cc
    branches/trac356/src/lib/nsas/tests/nameserver_entry_unittest.cc
    branches/trac356/src/lib/nsas/zone_entry.h

Modified: branches/trac356/src/lib/nsas/Makefile.am
==============================================================================
--- branches/trac356/src/lib/nsas/Makefile.am (original)
+++ branches/trac356/src/lib/nsas/Makefile.am Thu Oct 28 10:53:42 2010
@@ -9,6 +9,7 @@
 lib_LTLIBRARIES = libnsas.la
 libnsas_la_SOURCES  = address_entry.h address_entry.cc
 libnsas_la_SOURCES += hash.cc hash.h
+libnsas_la_SOURCES += hash_key.h
 libnsas_la_SOURCES += hash_table.h
 libnsas_la_SOURCES += lru_list.h
 libnsas_la_SOURCES += nameserver_entry.cc nameserver_entry.h

Modified: branches/trac356/src/lib/nsas/address_entry.h
==============================================================================
--- branches/trac356/src/lib/nsas/address_entry.h (original)
+++ branches/trac356/src/lib/nsas/address_entry.h Thu Oct 28 10:53:42 2010
@@ -36,12 +36,12 @@
     ///
     /// \param address Address object representing this address
     /// \param rtt Initial round-trip time
-    AddressEntry(const IOAddress& address, uint32_t rtt = 0) :
+    AddressEntry(const asiolink::IOAddress& address, uint32_t rtt = 0) :
         address_(address), rtt_(rtt)
     {}
 
     /// \return Address object
-    IOAddress getAddress() const {
+    asiolink::IOAddress getAddress() const {
         return address_;
     }
 
@@ -83,7 +83,7 @@
     static const uint32_t UNREACHABLE;  ///< RTT indicating unreachable address
 
 private:
-    IOAddress       address_;           ///< Address
+    asiolink::IOAddress address_;       ///< Address
     uint32_t        rtt_;               ///< Round-trip time
 };
 

Modified: branches/trac356/src/lib/nsas/asiolink.h
==============================================================================
--- branches/trac356/src/lib/nsas/asiolink.h (original)
+++ branches/trac356/src/lib/nsas/asiolink.h Thu Oct 28 10:53:42 2010
@@ -19,6 +19,8 @@
 
 #include <string>
 #include <sys/socket.h>
+
+namespace asiolink {
 
 /// \brief IO Address Dummy Class
 ///
@@ -50,4 +52,6 @@
     std::string     address_;       ///< Address represented
 };
 
+}   // namespace asiolink
+
 #endif // __ASIOLINK_H

Modified: branches/trac356/src/lib/nsas/hash.cc
==============================================================================
--- branches/trac356/src/lib/nsas/hash.cc (original)
+++ branches/trac356/src/lib/nsas/hash.cc Thu Oct 28 10:53:42 2010
@@ -54,6 +54,7 @@
 if advised of the possibility of such damage.
 */
 
+#include <cassert>
 #include <stdlib.h>
 #include <algorithm>
 #include <cassert>
@@ -71,17 +72,22 @@
 // Constructor.
 
 Hash::Hash(uint32_t tablesize, uint32_t maxkeylen, bool randomise) :
-    tablesize_(tablesize), maxkeylen_(maxkeylen)
+    tablesize_(tablesize), maxkeylen_(min(maxkeylen, (255 - sizeof(uint16_t))))
 {
-    // Check to see that we can cope with the maximum key length.
-    // (This code adapted from BIND-9)
+    // (Code taken from BIND-9)
+    //
+    // Check to see that we can cope with the maximum key length which, due
+    // to the limitations, should not be more than 255 in total.  The actual
+    // number of characters in the name that are considered is reduced to
+    // ensure that the class is taken into account in the hash.  (This accounts
+    // for the "+ sizeof(uint16_t)" in the calculations below.
     //
     // Overflow check.  Since our implementation only does a modulo
     // operation at the last stage of hash calculation, the accumulator
     // must not overflow.
     hash_accum_t overflow_limit =
         1 << (((sizeof(hash_accum_t) - sizeof(hash_random_t))) * 8);
-    if (overflow_limit < (maxkeylen + 1) * 0xff) {
+    if (overflow_limit < (maxkeylen_ + sizeof(uint16_t) + 1) * 0xff) {
         isc_throw(KeyLengthTooLong, "Hash key length too long for Hash class");
     }
 
@@ -101,8 +107,8 @@
     srandom(init_value.seed);
 
     // Fill in the random vector.
-    randvec_.reserve(maxkeylen + 1);
-    for (uint32_t i = 0; i < (maxkeylen + 1); ++i) {
+    randvec_.reserve(maxkeylen_ + sizeof(uint16_t) + 1);
+    for (uint32_t i = 0; i < (maxkeylen + sizeof(uint16_t) + 1); ++i) {
         randvec_.push_back(static_cast<hash_random_t>(random() & 0xffff));
     }
     assert(sizeof(hash_random_t) == 2); // So that the "& 0xffff" is valid
@@ -120,10 +126,8 @@
 }
 
 
-uint32_t Hash::operator()(const char* key, uint32_t keylen,
-    bool ignorecase) const
+uint32_t Hash::operator()(const HashKey& key, bool ignorecase) const
 {
-
     // Calculation as given in BIND-9.
     hash_accum_t partial_sum = 0;
     uint32_t i = 0;                 // Used after the end of the loop
@@ -131,14 +135,27 @@
     // Perform the hashing.  If the key length if more than the maximum we set
     // up this hash for, ignore the excess.
     if (ignorecase) {
-        for (i = 0; i < min(keylen, maxkeylen_); ++i) {
-            partial_sum += mapLower(key[i]) * randvec_[i];
+        for (i = 0; i < min(key.keylen, maxkeylen_); ++i) {
+            partial_sum += mapLower(key.key[i]) * randvec_[i];
         }
     } else {
-        for (i = 0; i < min(keylen, maxkeylen_); ++i) {
-            partial_sum += key[i] * randvec_[i];
+        for (i = 0; i < min(key.keylen, maxkeylen_); ++i) {
+            partial_sum += key.key[i] * randvec_[i];
         }
     }
+
+    // Add the hash of the class code
+    union {
+        uint16_t    class_code;                 // Copy of the class code
+        char        bytes[sizeof(uint16_t)];    // Byte equivalent
+    } convert;
+
+    convert.class_code = key.class_code;
+    for (int j = 0; j < sizeof(uint16_t); ++j, ++i) {
+        partial_sum += convert.bytes[j] * randvec_[i];
+    }
+
+    // ... and finish up.
     partial_sum += randvec_[i];
 
     // Determine the hash value

Modified: branches/trac356/src/lib/nsas/hash.h
==============================================================================
--- branches/trac356/src/lib/nsas/hash.h (original)
+++ branches/trac356/src/lib/nsas/hash.h Thu Oct 28 10:53:42 2010
@@ -21,6 +21,8 @@
 #include <vector>
 
 #include "exceptions/exceptions.h"
+
+#include "hash_key.h"
 
 namespace isc {
 namespace nsas {
@@ -79,14 +81,12 @@
 
     /// \brief Hash Value
     ///
-    /// \param key String value or array of bytes for which a hash is to be
-    /// calculated.
-    /// \param ketlen Number of characters in the string
+    /// \param key Parameters comprising the key to be hashed.
     /// \param ignorecase true for case to be ignored when calculating the
     /// hash value, false for it to be taken into account.
     ///
     /// \return Hash value, a number between 0 and N-1.
-    virtual uint32_t operator()(const char* key, uint32_t keylen,
+    virtual uint32_t operator()(const HashKey& key, 
         bool ignorecase = true) const;
 
     /// \brief Map Lower Case to Upper Case

Modified: branches/trac356/src/lib/nsas/hash_table.h
==============================================================================
--- branches/trac356/src/lib/nsas/hash_table.h (original)
+++ branches/trac356/src/lib/nsas/hash_table.h Thu Oct 28 10:53:42 2010
@@ -27,6 +27,7 @@
 #include "config.h"
 
 #include "hash.h"
+#include "hash_key.h"
 
 // Maximum key length if the maximum size of a DNS name
 #define MAX_KEY_LENGTH 255
@@ -90,11 +91,10 @@
     /// object's name is the same.
     ///
     /// \param object Pointer to the object
-    /// \param key Pointer to the name of the object
-    /// \param keylen Length of the key
+    /// \param key Key describing the object
     ///
     /// \return bool true of the name of the object is equal to the name given.
-    virtual bool operator()(T* object, const char* key, uint32_t keylen) = 0;
+    virtual bool operator()(T* object, const HashKey& key) = 0;
 };
 
 
@@ -131,11 +131,11 @@
     ///
     /// Returns a shared_ptr object pointing to the table entry
     ///
-    /// \param key Name of the object.  The hash of this is calculated and
-    /// used to index the table.
+    /// \param key Name of the object (and class).  The hash of this is
+    /// calculated and used to index the table.
     ///
     /// \return Shared pointer to the object.
-    virtual boost::shared_ptr<T> get(const char* key, uint32_t keylen);
+    virtual boost::shared_ptr<T> get(const HashKey& key);
 
     /// \brief Remove Entry
     ///
@@ -143,12 +143,11 @@
     /// destroyed, so if this is the last pointer, the object itself is also
     /// destroyed.
     ///
-    /// \param key Name of the object.  The hash of this is calculated and
-    /// used to index the table.
-    /// \param keylen Length of the key
+    /// \param key Name of the object (and class).  The hash of this is
+    /// calculated and used to index the table.
     ///
     /// \return true if the object was deleted, false if it was not found.
-    virtual bool remove(const char* key, uint32_t keylen);
+    virtual bool remove(const HashKey& key);
 
     /// \brief Add Entry
     ///
@@ -160,13 +159,12 @@
     /// successful, this object will have a shared pointer pointing to it; it
     /// should not be deleted by the caller.
     /// \param key Key to use to calculate the hash.
-    /// \patam keylen Length of "key"
     /// \param replace If true, when an object is added and an object with the
     /// same name already exists, the existing object is replaced.  If false,
     // the addition fails and a status is returned.
     /// \return true if the object was successfully added, false otherwise.
-    virtual bool add(boost::shared_ptr<T>& object, const char* key,
-        uint32_t keylen, bool replace = false);
+    virtual bool add(boost::shared_ptr<T>& object, const HashKey& key,
+        bool replace = false);
 
     /// \brief Returns Size of Hash Table
     ///
@@ -190,10 +188,10 @@
 
 // Lookup an object in the table
 template <typename T>
-boost::shared_ptr<T> HashTable<T>::get(const char* key, uint32_t keylen) {
+boost::shared_ptr<T> HashTable<T>::get(const HashKey& key) {
 
     // Calculate the hash value
-    uint32_t index = hash_(key, keylen);
+    uint32_t index = hash_(key);
 
     // Take out a read lock on this hash slot.  The lock is released when this
     // object goes out of scope.
@@ -202,7 +200,7 @@
     // Locate the object.
     typename HashTableSlot<T>::iterator i;
     for (i = table_[index].list_.begin(); i != table_[index].list_.end(); ++i) {
-        if ((*compare_)(i->get(), key, keylen)) {
+        if ((*compare_)(i->get(), key)) {
 
             // Found it, so return the shared pointer object
             return (*i);
@@ -215,10 +213,10 @@
 
 // Remove an entry from the hash table
 template <typename T>
-bool HashTable<T>::remove(const char* key, uint32_t keylen) {
+bool HashTable<T>::remove(const HashKey& key) {
 
     // Calculate the hash value
-    uint32_t index = hash_(key, keylen);
+    uint32_t index = hash_(key);
 
     // Access to the elements of this hash slot are accessed under a mutex.
     // The mutex will be released when this object goes out of scope and is
@@ -228,7 +226,7 @@
     // Now search this list to see if the element already exists.
     typename HashTableSlot<T>::iterator i;
     for (i = table_[index].list_.begin(); i != table_[index].list_.end(); ++i) {
-        if ((*compare_)(i->get(), key, keylen)) {
+        if ((*compare_)(i->get(), key)) {
 
             // Object found so delete it.
             table_[index].list_.erase(i);
@@ -243,11 +241,12 @@
 
 // Add an entry to the hash table
 template <typename T>
-bool HashTable<T>::add(boost::shared_ptr<T>& object, const char* key,
-    uint32_t keylen, bool replace) {
+bool HashTable<T>::add(boost::shared_ptr<T>& object, const HashKey& key,
+    bool replace)
+{
 
     // Calculate the hash value
-    uint32_t index = hash_(key, keylen);
+    uint32_t index = hash_(key);
 
     // Access to the elements of this hash slot are accessed under a mutex.
     scoped_lock<typename HashTableSlot<T>::mutex_type> lock(table_[index].mutex_);
@@ -255,7 +254,7 @@
     // Now search this list to see if the element already exists.
     typename HashTableSlot<T>::iterator i;
     for (i = table_[index].list_.begin(); i != table_[index].list_.end(); ++i) {
-        if ((*compare_)(i->get(), key, keylen)) {
+        if ((*compare_)(i->get(), key)) {
 
             // Object found.  If we are not allowed to replace the element,
             // return an error.  Otherwise erase it from the list and exit the

Modified: branches/trac356/src/lib/nsas/nameserver_entry.cc
==============================================================================
--- branches/trac356/src/lib/nsas/nameserver_entry.cc (original)
+++ branches/trac356/src/lib/nsas/nameserver_entry.cc Thu Oct 28 10:53:42 2010
@@ -32,6 +32,7 @@
 #include "address_entry.h"
 #include "nameserver_entry.h"
 
+using namespace asiolink;
 using namespace isc::nsas;
 using namespace isc::dns;
 using namespace std;

Modified: branches/trac356/src/lib/nsas/nameserver_entry.h
==============================================================================
--- branches/trac356/src/lib/nsas/nameserver_entry.h (original)
+++ branches/trac356/src/lib/nsas/nameserver_entry.h Thu Oct 28 10:53:42 2010
@@ -24,6 +24,7 @@
 #include "address_entry.h"
 #include "asiolink.h"
 #include "exceptions/exceptions.h"
+#include "lru_list.h"
 #include "rrset.h"
 
 namespace isc {
@@ -71,9 +72,11 @@
 /// different TTLs) there is one expiration time for all address records.
 /// When that is reached, all records are declared expired and new fetches
 /// started for the information.
+///
+/// As this object will be stored in the nameserver address store LRU list,
+/// it is derived from the LRU list element class.
 
-
-class NameserverEntry {
+class NameserverEntry : public LruList<NameserverEntry>::Element {
 public:
     /// List of addresses associated with this nameserver
     typedef std::vector<AddressEntry>   AddressVector;
@@ -126,14 +129,14 @@
     ///
     /// \param address Address to update
     /// \param RTT New RTT for the address
-    virtual void setAddressRTT(const IOAddress& address, uint32_t rtt);
+    virtual void setAddressRTT(const asiolink::IOAddress& address, uint32_t rtt);
 
     /// \brief Set Address Unreachable
     ///
     /// Sets the specified address to be unreachable
     ///
     /// \param address Address to update
-    virtual void setAddressUnreachable(const IOAddress& address);
+    virtual void setAddressUnreachable(const asiolink::IOAddress& address);
 
     /// \return Owner Name of RRset
     virtual std::string getName() const {

Modified: branches/trac356/src/lib/nsas/tests/Makefile.am
==============================================================================
--- branches/trac356/src/lib/nsas/tests/Makefile.am (original)
+++ branches/trac356/src/lib/nsas/tests/Makefile.am Thu Oct 28 10:53:42 2010
@@ -20,6 +20,7 @@
 run_unittests_SOURCES += nsas_test_utilities.h 
 run_unittests_SOURCES += address_entry_unittest.cc
 run_unittests_SOURCES += hash_unittest.cc
+run_unittests_SOURCES += hash_key_unittest.cc
 run_unittests_SOURCES += hash_table_unittest.cc
 run_unittests_SOURCES += lru_list_unittest.cc
 run_unittests_SOURCES += nameserver_entry_unittest.cc

Modified: branches/trac356/src/lib/nsas/tests/address_entry_unittest.cc
==============================================================================
--- branches/trac356/src/lib/nsas/tests/address_entry_unittest.cc (original)
+++ branches/trac356/src/lib/nsas/tests/address_entry_unittest.cc Thu Oct 28 10:53:42 2010
@@ -32,6 +32,7 @@
 static std::string V6A_TEXT("2001:dead:beef::0");
 static std::string V6B_TEXT("1984:1985::1986:1987");
 
+using namespace asiolink;
 using namespace std;
 using namespace isc::nsas;
 

Modified: branches/trac356/src/lib/nsas/tests/hash_table_unittest.cc
==============================================================================
--- branches/trac356/src/lib/nsas/tests/hash_table_unittest.cc (original)
+++ branches/trac356/src/lib/nsas/tests/hash_table_unittest.cc Thu Oct 28 10:53:42 2010
@@ -21,6 +21,7 @@
 #include <iostream>
 
 #include "hash_table.h"
+#include "hash_key.h"
 
 
 namespace isc {
@@ -37,8 +38,10 @@
     /// Constructor
     ///
     /// \param name Name of the object
+    /// \param class_code Class of the object
     /// \param value Integer value
-    DummyObject(const char* name, int value) : name_(name), value_(value)
+    DummyObject(const char* name, uint16_t class_code, int value) :
+        name_(name), classCode_(class_code), value_(value)
     {}
 
     /// \return Name of the object
@@ -51,9 +54,20 @@
         return value_;
     }
 
+    /// \return Class of the object
+    uint16_t getClass() const {
+        return classCode_;
+    }
+
+    /// \return Hash Key of the Object
+    HashKey getKey() {
+        return HashKey(name_.c_str(), name_.size(), classCode_);
+    }
+
 private:
-    string  name_;      ///< Object name
-    int     value_;     ///< Object value
+    string      name_;      ///< Object name
+    uint16_t    classCode_; ///< Object class
+    int         value_;     ///< Object value
 };
 
 /// \brief Comparison Class
@@ -70,34 +84,51 @@
     /// \param object Pointer to the object being tested
     /// \param key Key string
     /// \param keylen Length of the key string
-    bool operator()(DummyObject* object, const char* key, uint32_t keylen) {
+    bool operator()(DummyObject* object, const HashKey& key) {
         // Do a quick check on size first
-        if (keylen != object->getName().size()) {
-            return false;
+        if (key.keylen == object->getName().size()) {
+
+            // Size matches, does the class?
+            if (key.class_code == object->getClass()) {
+
+                // So does the memory?
+                return (memcmp(object->getName().c_str(), key.key,
+                    key.keylen) == 0);
+            }
         }
-        return (memcmp(object->getName().c_str(), key, keylen) == 0);
+
+        // Size or class do not match.
+        return (false);
     }
 };
 
 
 
 /// \brief Text Fixture Class
+///
+/// Many of the tests are based on checking reference counts.  In all tests,
+/// objects named dummyN_ have a reference count of 1 because they are a member
+/// of this class which has been instantiated for the test.  The reference count
+/// is increased as they are added to the hash table for testing and decreased
+/// as they are removed.
 class HashTableTest : public ::testing::Test {
 protected:
 
     // Constructor - initialize the objects
     HashTableTest() :
         table_(new DummyObjectCompare()),
-        dummy1_(new DummyObject("test", 42)),
-        dummy2_(new DummyObject("test", 47)),
-        dummy3_(new DummyObject("Something_Else", 1332))
+        dummy1_(new DummyObject("test", 1, 42)),
+        dummy2_(new DummyObject("test", 1, 47)),
+        dummy3_(new DummyObject("Something_Else", 1, 1332)),
+        dummy4_(new DummyObject("test", 3, 42))
     {}
 
-    // Members
+    // Members.
     HashTable<DummyObject> table_;
     boost::shared_ptr<DummyObject> dummy1_;
     boost::shared_ptr<DummyObject> dummy2_;
     boost::shared_ptr<DummyObject> dummy3_;
+    boost::shared_ptr<DummyObject> dummy4_;
 };
 
 
@@ -117,27 +148,26 @@
 // Basic test of addition
 TEST_F(HashTableTest, AddTest) {
 
-    // Using two objects with the same key.
+    // Using two objects with the same name and class,
+    EXPECT_EQ(dummy1_->getName(), dummy2_->getName());
+    EXPECT_EQ(dummy1_->getClass(), dummy2_->getClass());
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(1, dummy2_.use_count());
 
     // Add first one to the hash table_
-    bool result = table_.add(dummy1_, dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
+    bool result = table_.add(dummy1_, dummy1_->getKey());
     EXPECT_TRUE(result);
     EXPECT_EQ(2, dummy1_.use_count());
     EXPECT_EQ(1, dummy2_.use_count());
 
-    // Attempt to add the second object with the same name should fail.
-    result = table_.add(dummy2_, dummy2_.get()->getName().c_str(),
-        dummy2_.get()->getName().size());
+    // Attempt to add the second object with the same name and class fails.
+    result = table_.add(dummy2_, dummy2_->getKey());
     EXPECT_FALSE(result);
     EXPECT_EQ(2, dummy1_.use_count());
     EXPECT_EQ(1, dummy2_.use_count());
 
     // Replacing an entry should work though
-    result = table_.add(dummy2_, dummy2_.get()->getName().c_str(),
-        dummy2_.get()->getName().size(), true);
+    result = table_.add(dummy2_, dummy2_->getKey(), true);
     EXPECT_TRUE(result);
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(2, dummy2_.use_count());
@@ -146,44 +176,40 @@
 // Test the remove functionality
 TEST_F(HashTableTest, RemoveTest) {
 
-    // Using two objects with different keys
+    // Using two objects with different names but the same class
+    EXPECT_NE(dummy1_->getName(), dummy3_->getName());
+    EXPECT_EQ(dummy1_->getClass(), dummy3_->getClass());
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(1, dummy3_.use_count());
 
     // Add first one to the hash table_
-    bool result = table_.add(dummy1_, dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
+    bool result = table_.add(dummy1_, dummy1_->getKey());
     EXPECT_TRUE(result);
     EXPECT_EQ(2, dummy1_.use_count());
     EXPECT_EQ(1, dummy3_.use_count());
 
     // Now remove it.
-    result = table_.remove(dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
+    result = table_.remove(dummy1_->getKey());
     EXPECT_TRUE(result);
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(1, dummy3_.use_count());
 
     // Attempt to remove it again.
-    result = table_.remove(dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
+    result = table_.remove(dummy1_->getKey());
     EXPECT_FALSE(result);
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(1, dummy3_.use_count());
 
-    // Add both entries to the table_, then remove one (checks that it will
+    // Add both entries to table_, then remove one (checks that it will
     // remove the correct one).
-    result = table_.add(dummy1_, dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
-    EXPECT_TRUE(result);
-    result = table_.add(dummy3_, dummy3_.get()->getName().c_str(),
-        dummy3_.get()->getName().size());
+    result = table_.add(dummy1_, dummy1_->getKey());
+    EXPECT_TRUE(result);
+    result = table_.add(dummy3_, dummy3_->getKey());
     EXPECT_TRUE(result);
     EXPECT_EQ(2, dummy1_.use_count());
     EXPECT_EQ(2, dummy3_.use_count());
 
-    result = table_.remove(dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
+    result = table_.remove(dummy1_->getKey());
     EXPECT_TRUE(result);
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(2, dummy3_.use_count());
@@ -192,39 +218,77 @@
 // Test the "get" functionality
 TEST_F(HashTableTest, GetTest) {
 
-    // Using two objects with different keys
+    // Using two objects with different names but the same class
+    EXPECT_NE(dummy1_->getName(), dummy3_->getName());
+    EXPECT_EQ(dummy1_->getClass(), dummy3_->getClass());
     EXPECT_EQ(1, dummy1_.use_count());
     EXPECT_EQ(1, dummy3_.use_count());
 
     // Add both to the hash table
-    bool result = table_.add(dummy1_, dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
-    EXPECT_TRUE(result);
-    result = table_.add(dummy3_, dummy3_.get()->getName().c_str(),
-        dummy3_.get()->getName().size());
+    bool result = table_.add(dummy1_, dummy1_->getKey());
+    EXPECT_TRUE(result);
+    result = table_.add(dummy3_, dummy3_->getKey());
     EXPECT_TRUE(result);
 
     // Lookup the first
-    boost::shared_ptr<DummyObject> value =
-        table_.get(dummy1_.get()->getName().c_str(), 
-        dummy1_.get()->getName().size());
+    boost::shared_ptr<DummyObject> value = table_.get(dummy1_->getKey());
     EXPECT_EQ(value.get(), dummy1_.get());
 
     // ... and the second
-    value = table_.get(dummy3_.get()->getName().c_str(), 
-        dummy3_.get()->getName().size());
+    value = table_.get(dummy3_->getKey());
     EXPECT_EQ(value.get(), dummy3_.get());
 
     // Remove the first
-    result = table_.remove(dummy1_.get()->getName().c_str(),
-        dummy1_.get()->getName().size());
+    result = table_.remove(dummy1_->getKey());
     EXPECT_TRUE(result);
 
     // ... and a lookup should return empty
-    value = table_.get(dummy1_.get()->getName().c_str(), 
-        dummy1_.get()->getName().size());
+    value = table_.get(dummy1_->getKey());
     EXPECT_TRUE(value.get() == NULL);
-
+}
+
+// Test that objects with the same name and different classes are distinct.
+TEST_F(HashTableTest, ClassTest) {
+
+    // Using two objects with the same name and different classes
+    EXPECT_EQ(dummy1_->getName(), dummy4_->getName());
+    EXPECT_NE(dummy1_->getClass(), dummy4_->getClass());
+    EXPECT_EQ(1, dummy1_.use_count());
+    EXPECT_EQ(1, dummy4_.use_count());
+
+    // Add both to the hash table
+    bool result = table_.add(dummy1_, dummy1_->getKey());
+    EXPECT_TRUE(result);
+    EXPECT_EQ(2, dummy1_.use_count());
+
+    result = table_.add(dummy4_, dummy4_->getKey());
+    EXPECT_TRUE(result);
+    EXPECT_EQ(2, dummy4_.use_count());
+
+    // Lookup the first
+    boost::shared_ptr<DummyObject> value1 = table_.get(dummy1_->getKey());
+    EXPECT_EQ(value1.get(), dummy1_.get());
+    EXPECT_EQ(3, dummy1_.use_count());
+    EXPECT_EQ(2, dummy4_.use_count());
+
+    // ... and the second
+    boost::shared_ptr<DummyObject> value4 = table_.get(dummy4_->getKey());
+    EXPECT_EQ(value4.get(), dummy4_.get());
+    EXPECT_EQ(3, dummy1_.use_count());
+    EXPECT_EQ(3, dummy4_.use_count());
+
+    // ... and check they are different
+    EXPECT_NE(dummy1_.get(), dummy4_.get());
+
+    // Remove the first
+    result = table_.remove(dummy1_->getKey());
+    EXPECT_TRUE(result);
+    EXPECT_EQ(2, dummy1_.use_count());
+    EXPECT_EQ(3, dummy4_.use_count());
+
+    // ... and a lookup should return empty
+    boost::shared_ptr<DummyObject> value1a = table_.get(dummy1_->getKey());
+    EXPECT_TRUE(value1a.get() == NULL);
 }
 
 

Modified: branches/trac356/src/lib/nsas/tests/hash_unittest.cc
==============================================================================
--- branches/trac356/src/lib/nsas/tests/hash_unittest.cc (original)
+++ branches/trac356/src/lib/nsas/tests/hash_unittest.cc Thu Oct 28 10:53:42 2010
@@ -28,8 +28,27 @@
 namespace isc {
 namespace nsas {
 
+// Utility function to count the number of unique values in a vector
+static uint32_t CountUnique(const vector<uint32_t>& input) {
 
-/// \brief Text Fixture Class
+    // Duplicate the vector as this will be destroyed in the process.
+    vector<uint32_t>  vcopy(input);
+
+    // Check to see if values are unique.  Do this by sorting the values into
+    // ascending order, removing duplicates, and checking the size again.
+    //
+    // Note that unique only shifts elements around - it does not remove non-
+    // unique values, so it does change the size of the array.  The call to
+    // erase removes the elements left at the end of the array.
+    sort(vcopy.begin(), vcopy.end());
+    vcopy.erase(unique(vcopy.begin(), vcopy.end()), vcopy.end());
+
+    // ... and return the count of elements remaining.
+    return (vcopy.size());
+}
+
+
+/// \brief Test Fixture Class
 class HashTest : public ::testing::Test {
 };
 
@@ -60,22 +79,16 @@
     // Generate hash values
     for (int i = 0; i < 50; ++i) {
         string name = base + boost::lexical_cast<string>(i);
-        uint32_t hashval = hash(name.c_str(), name.size());
+        uint32_t hashval = hash(HashKey(name.c_str(), name.size(), 0));
         EXPECT_LT(hashval, size);
         values.push_back(hashval);
     }
     uint32_t cursize = values.size();
 
-    // Check to see if they are unique.  Do this by sorting the values into
-    // ascending order, removing duplicates, and checking the size again.
-    //
-    // Note that unique only shifts elements around - it does not remove non-
-    // unique values, so it does change the size of the array.  The call to
-    // erase removes the elements left at the end of the array.
-    sort(values.begin(), values.end());
-    values.erase(unique(values.begin(), values.end()), values.end());
+    // Count the unique values in the array
+    uint32_t newsize = CountUnique(values);
 
-    uint32_t newsize = values.size();
+    // We don't expect many clashes.
     EXPECT_GE(newsize + 2, cursize);
 }
 
@@ -98,33 +111,57 @@
 }
 
 // Test the mapping of mixed-case names
-
 TEST_F(HashTest, MixedCase) {
 
     std::string test1 = "example1234.co.uk.";
     std::string test2 = "EXAmple1234.co.uk.";
 
-    Hash hash(1009, 255);
+    Hash hash(1009, 255, false);    // Disable randomisation for testing
 
     // Case not ignored, hashes should be different
-    uint32_t value1 = hash(test1.c_str(), test1.size(), false);
-    uint32_t value2 = hash(test2.c_str(), test2.size(), false);
+    uint32_t value1 = hash(HashKey(test1.c_str(), test1.size(), 0), false);
+    uint32_t value2 = hash(HashKey(test2.c_str(), test2.size(), 0), false);
     EXPECT_NE(value1, value2);
 
     // Case ignored, hashes should be the same
-    uint32_t value3 = hash(test1.c_str(), test1.size(), true);
-    uint32_t value4 = hash(test2.c_str(), test2.size(), true);
+    uint32_t value3 = hash(HashKey(test1.c_str(), test1.size(), 0), true);
+    uint32_t value4 = hash(HashKey(test2.c_str(), test2.size(), 0), true);
     EXPECT_EQ(value3, value4);
 
     // Check the default setting.
-    uint32_t value5 = hash(test1.c_str(), test1.size());
-    uint32_t value6 = hash(test2.c_str(), test2.size());
+    uint32_t value5 = hash(HashKey(test1.c_str(), test1.size(), 0));
+    uint32_t value6 = hash(HashKey(test2.c_str(), test2.size(), 0));
     EXPECT_EQ(value5, value6);
 
     // ... and just for good measure
     EXPECT_EQ(value1, value3);
     EXPECT_EQ(value1, value5);
 }
+
+// Test that the same name but different classes get mapped differently.
+TEST_F(HashTest, ClassCodes) {
+
+    std::string test1 = "example1234.co.uk.";
+    Hash hash(1009, 255, false);    // Disable randomisation for testing
+
+    // Just try codes in the range 0 to 9 - more than covers the allocated
+    // codes.
+    vector<uint32_t> values;
+    for (uint32_t i = 0; i < 10; ++i) {
+        values.push_back(hash(HashKey(test1.c_str(), test1.size(), i)));
+    }
+
+    // find the number of unique values in the array.  Although there can
+    // be some clashes, we don't expect too many.
+    uint32_t cursize = values.size();
+
+    // Count the unique values in the array
+    uint32_t newsize = CountUnique(values);
+
+    // We don't expect many clashes.
+    EXPECT_GE(newsize + 2, cursize);
+}
+
 
 // Test that the code performs when the length of the key is excessive
 TEST_F(HashTest, Overlong) {
@@ -137,8 +174,8 @@
     Hash hash(1009, string1.size());
 
     // Do two hashes
-    uint32_t value1 = hash(string1.c_str(), string1.size());
-    uint32_t value2 = hash(string2.c_str(), string2.size());
+    uint32_t value1 = hash(HashKey(string1.c_str(), string1.size(), 0));
+    uint32_t value2 = hash(HashKey(string2.c_str(), string2.size(), 0));
     EXPECT_EQ(value1, value2);
 }
 

Modified: branches/trac356/src/lib/nsas/tests/nameserver_entry_unittest.cc
==============================================================================
--- branches/trac356/src/lib/nsas/tests/nameserver_entry_unittest.cc (original)
+++ branches/trac356/src/lib/nsas/tests/nameserver_entry_unittest.cc Thu Oct 28 10:53:42 2010
@@ -33,7 +33,7 @@
 
 #include "nsas_test_utilities.h"
 
-
+using namespace asiolink;
 using namespace std;
 using namespace isc::dns;
 using namespace rdata;

Modified: branches/trac356/src/lib/nsas/zone_entry.h
==============================================================================
--- branches/trac356/src/lib/nsas/zone_entry.h (original)
+++ branches/trac356/src/lib/nsas/zone_entry.h Thu Oct 28 10:53:42 2010
@@ -22,6 +22,8 @@
 #include <boost/thread.h>
 #include <boost/shared_ptr.h>
 
+#include "asiolink.h"
+
 class NameserverEntry;
 
 /// \brief Zone Entry
@@ -45,7 +47,7 @@
     /// \brief Lookup Address
     ///
     /// Returns the address with the lowest RTT.
-    IOAddress getAddress() const;
+    asiolink::IOAddress getAddress() const;
 
 public:
     void updateNS




More information about the bind10-changes mailing list