Made some modifnication as review feedback.

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/trac356@3740 e5f2f494-b856-4b98-b285-d166d9295462

src/lib/nsas/Makefile.am

@@ -19,6 +19,7 @@ libnsas_la_SOURCES += nameserver_address.h
 libnsas_la_SOURCES += nameserver_entry.cc nameserver_entry.h
 libnsas_la_SOURCES += nsas_entry_compare.h
 libnsas_la_SOURCES += nsas_entry.h
+libnsas_la_SOURCES += random_number_generator.h
 libnsas_la_SOURCES += zone_entry.h
 
 CLEANFILES = *.gcno *.gcda

src/lib/nsas/nameserver_address.h

@@ -60,7 +60,9 @@ public:
     NameserverAddress(boost::shared_ptr<NameserverEntry>& nameserver, uint32_t index, short family):
         ns_(nameserver), index_(index), family_(family)
     {
-        if(!ns_.get()) isc_throw(NullNameserverEntryPointer, "NULL NameserverEntry pointer.");
+        if(!ns_.get()) {
+            isc_throw(NullNameserverEntryPointer, "NULL NameserverEntry pointer.");
+        }
     }
 
     /// \brief Default Constructor

src/lib/nsas/nameserver_entry.cc

@@ -37,6 +37,7 @@ using namespace asiolink;
 using namespace isc::nsas;
 using namespace isc::dns;
 using namespace std;
+using namespace boost;
 
 namespace isc {
 namespace nsas {
@@ -151,22 +152,20 @@ void NameserverEntry::getAddresses(AddressVector& addresses, short family) const
 }
 
 // Return one address matching the given family
-bool NameserverEntry::getAddress(boost::shared_ptr<NameserverEntry>& nameserver, 
-        NameserverAddress& address, short family)
+bool NameserverEntry::getAddress(NameserverAddress& address, short family)
 {
-
-    // The shared_ptr must contain this pointer
-    assert(nameserver.get() == this);
+    // Get the shared_ptr object that point to "this" object
+    shared_ptr<NameserverEntry> shared_ptr_to_this = shared_from_this();
 
     if(family == AF_INET){
         if(v4_addresses_.size() == 0) return false;
 
-        address = NameserverAddress(nameserver, v4_address_selector_(), AF_INET);
+        address = NameserverAddress(shared_ptr_to_this, v4_address_selector_(), AF_INET);
         return true;
     } else if(family == AF_INET6){
         if(v6_addresses_.size() == 0) return false;
 
-        address = NameserverAddress(nameserver, v6_address_selector_(), AF_INET6);
+        //address = NameserverAddress(shared_from_this(), v6_address_selector_(), AF_INET6);
         return true;
     }
     return false;
@@ -230,8 +229,11 @@ void NameserverEntry::updateAddressRTTAtIndex(uint32_t rtt, uint32_t index, shor
     (*addresses)[index].setRTT(new_rtt);
 
     // Update the selector
-    if(family == AF_INET) updateAddressSelector(v4_addresses_, v4_address_selector_);
-    else if(family == AF_INET6) updateAddressSelector(v6_addresses_, v6_address_selector_);
+    if(family == AF_INET) { 
+        updateAddressSelector(v4_addresses_, v4_address_selector_);
+    } else if(family == AF_INET6) {
+        updateAddressSelector(v6_addresses_, v6_address_selector_);
+    }
 }
 
 // Sets the address to be unreachable

src/lib/nsas/nameserver_entry.h

@@ -91,7 +91,7 @@ public:
 /// As this object will be stored in the nameserver address store LRU list,
 /// it is derived from the LRU list entry class.
 
-class NameserverEntry : public NsasEntry<NameserverEntry> {
+class NameserverEntry : public NsasEntry<NameserverEntry>{
 public:
     /// List of addresses associated with this nameserver
     typedef std::vector<AddressEntry>   AddressVector;
@@ -142,13 +142,10 @@ public:
     /// \brief Return one address
     ///
     /// Return one address corresponding to this nameserver
-    /// \param nameserver The NamerserverEntry shared_ptr object. The NameserverAddress
-    ///        need to hold it to avoid NameserverEntry being released
     /// \param address NameserverAddress object used to receive the address
     /// \param family The family of user request, AF_INET or AF_INET6
     /// \return true if one address is found, false otherwise
-    virtual bool getAddress(boost::shared_ptr<NameserverEntry>& nameserver, 
-            NameserverAddress& address, short family);
+    virtual bool getAddress(NameserverAddress& address, short family);
 
     /// \brief Return Address that corresponding to the index
     ///

src/lib/nsas/nsas_entry.h

@@ -17,6 +17,7 @@
 #ifndef __NSAS_ENTRY_H
 #define __NSAS_ENTRY_H
 
+#include <boost/enable_shared_from_this.hpp>
 #include <iostream>
 
 #include "exceptions/exceptions.h"
@@ -39,7 +40,6 @@ public:
     {}
 };
 
-
 /// \brief Element of NSAS Internal Lists
 ///
 /// This defines an element of the NSAS lists.  All elements stored in these
@@ -64,9 +64,11 @@ public:
 /// pointers, but a shared pointer to a base class is not a subclass of a
 /// shared pointer to a derived class.  For this reason, the type of element
 /// being stored is a template parameter.
-
+///
+/// This class is inherited from boost::enable_shared_from_this class
+/// So within a member function a shared_ptr to current object can be obtained
 template <typename T>
-class NsasEntry {
+class NsasEntry : public boost::enable_shared_from_this <T>  {
 public:
 
     /// \brief Default Constructor

src/lib/nsas/random_number_generator.h

@@ -17,6 +17,7 @@
 #ifndef __NSAS_RANDOM_NUMBER_GENERATOR_H
 #define __NSAS_RANDOM_NUMBER_GENERATOR_H
 
+#include <cmath>
 #include <numeric>
 #include <boost/random/mersenne_twister.hpp>
 #include <boost/random/uniform_int.hpp>
@@ -133,19 +134,19 @@ private:
     bool isProbabilitiesValid(const std::vector<double>& probabilities) const
     {
         typedef std::vector<double>::const_iterator Iterator;
-        double sum = probabilities.empty() ? 1 : 0;
+        double sum = probabilities.empty() ? 1.0 : 0.0;
         for(Iterator it = probabilities.begin(); it != probabilities.end(); ++it){
             //The probability must be in [0, 1.0]
-            if(*it < 0) return false;
-
-            if(*it > 1) return false;
+            if(*it < 0.0 || *it > 1.0) {
+                return false;
+            }
 
             sum += *it;
         }
 
         double epsilon = 0.0001;
         // The sum must be equal to 1
-        return fabs(sum - 1) < epsilon;
+        return fabs(sum - 1.0) < epsilon;
     }
 
     // Shortcut typedefs

src/lib/nsas/tests/nameserver_entry_unittest.cc

@@ -437,7 +437,7 @@ TEST_F(NameserverEntryTest, AddressSelection) {
     int c3 = 0;
     NameserverAddress ns_address;
     for(int i = 0; i < 10000; ++i){
-        ns.get()->getAddress(ns, ns_address, AF_INET);
+        ns.get()->getAddress(ns_address, AF_INET);
         asiolink::IOAddress io_address = ns_address.getAddress();
         if(io_address.toText() == v4Addresses[0].getAddress().toText()) ++c1;
         else if(io_address.toText() == v4Addresses[1].getAddress().toText()) ++c2;
@@ -453,7 +453,7 @@ TEST_F(NameserverEntryTest, AddressSelection) {
     ns->setAddressRTT(v4Addresses[2].getAddress(), 3);
     c1 = c2 = c3 = 0; 
     for(int i = 0; i < 100000; ++i){
-        ns.get()->getAddress(ns, ns_address, AF_INET);
+        ns.get()->getAddress(ns_address, AF_INET);
         asiolink::IOAddress io_address = ns_address.getAddress();
         if(io_address.toText() == v4Addresses[0].getAddress().toText()) ++c1;
         else if(io_address.toText() == v4Addresses[1].getAddress().toText()) ++c2;
@@ -471,7 +471,7 @@ TEST_F(NameserverEntryTest, AddressSelection) {
     ns->setAddressUnreachable(v4Addresses[2].getAddress());
     c1 = c2 = c3 = 0;
     for(int i = 0; i < 100000; ++i){
-        ns.get()->getAddress(ns, ns_address, AF_INET);
+        ns.get()->getAddress(ns_address, AF_INET);
         asiolink::IOAddress io_address = ns_address.getAddress();
         if(io_address.toText() == v4Addresses[0].getAddress().toText()) ++c1;
         else if(io_address.toText() == v4Addresses[1].getAddress().toText()) ++c2;

src/lib/nsas/tests/random_number_generator_unittest.cc

@@ -52,11 +52,13 @@ private:
     const static int max_ = 10;
 };
 
+#ifndef NDEBUG
 // Test of the constructor
 TEST_F(UniformRandomIntegerGeneratorTest, Constructor) {
     // The range must be min<=max
     ASSERT_DEATH(UniformRandomIntegerGenerator(3, 2), "");
 }
+#endif
 
 // Test of the generated integers are in the range [min, max]
 TEST_F(UniformRandomIntegerGeneratorTest, IntegerRange) {
@@ -97,6 +99,7 @@ TEST_F(WeightedRandomIntegerGeneratorTest, Constructor)
         ASSERT_EQ(gen(), 123);
     }
 
+#ifndef NDEBUG
     //The probability must be >= 0
     probabilities.push_back(-0.1);
     probabilities.push_back(1.1);
@@ -120,6 +123,7 @@ TEST_F(WeightedRandomIntegerGeneratorTest, Constructor)
     probabilities.push_back(0.2);
     probabilities.push_back(0.1);
     ASSERT_DEATH(WeightedRandomIntegerGenerator gen5(probabilities), "");
+#endif
 }
 
 // Test the randomization of the generator