From: Tomek Mrugalski <tomasz@isc.org>
Date: Wed, 2 Oct 2013 11:11:11 +0000 (+0200)
Subject: [3171] Pool::toText() implemented, exception error in AllocEngine clarified
X-Git-Tag: bind10-1.2.0beta1-release~192^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b48106123d817b95de69c2d58aeb24ef26a186e7;p=thirdparty%2Fkea.git

[3171] Pool::toText() implemented, exception error in AllocEngine clarified
---

diff --git a/src/lib/dhcpsrv/alloc_engine.cc b/src/lib/dhcpsrv/alloc_engine.cc
index 1d2e5e6641..d0bcfb70f6 100644
--- a/src/lib/dhcpsrv/alloc_engine.cc
+++ b/src/lib/dhcpsrv/alloc_engine.cc
@@ -89,7 +89,8 @@ isc::asiolink::IOAddress
 AllocEngine::IterativeAllocator::increasePrefix(const isc::asiolink::IOAddress& prefix,
                                                 uint8_t prefix_len) const {
     if (!prefix.isV6()) {
-        isc_throw(BadValue, "Prefix operations are for IPv6 only");
+        isc_throw(BadValue, "Prefix operations are for IPv6 only (attempted to "
+                  "increase prefix " << prefix.toText() << ")");
     }
 
     // Get a buffer holding an address.
@@ -100,11 +101,20 @@ AllocEngine::IterativeAllocator::increasePrefix(const isc::asiolink::IOAddress&
                   << prefix_len);
     }
 
-    // Explanation what happens here: http://www.youtube.com/watch?v=NFQCYpIHLNQ
+    // Brief explanation what happens here:
+    // http://www.youtube.com/watch?v=NFQCYpIHLNQ
+
     uint8_t n_bytes = (prefix_len - 1)/8;
     uint8_t n_bits = 8 - (prefix_len - n_bytes*8);
     uint8_t mask = 1 << n_bits;
 
+    // Longer explanation: n_bytes specifies number of full bytes that are
+    // in-prefix. They can also be used as an offset for the first byte that
+    // is not in prefix. n_bits specifies number of bits on the last byte that
+    // is (often partially) in prefix. For example for a /125 prefix, the values
+    // are 15 and 3, respectively. Mask is a bitmask that has the least
+    // significant bit from the prefix set.
+
     uint8_t packed[V6ADDRESS_LEN];
 
     // Copy the address. It must be V6, but we already checked that.
@@ -179,7 +189,8 @@ AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet,
         Pool6Ptr pool6 = boost::dynamic_pointer_cast<Pool6>(*it);
         if (!pool6) {
             // Something is gravely wrong here
-            isc_throw(InvalidParameter, "Wrong type of pool");
+            isc_throw(Unexpected, "Wrong type of pool: " << (*it)->toText()
+                      << " is not Pool6");
         }
         // Get the next prefix
         next = increasePrefix(last, (pool6)->getLength());
diff --git a/src/lib/dhcpsrv/pool.cc b/src/lib/dhcpsrv/pool.cc
index 53c4d093f5..f1ba871aa5 100644
--- a/src/lib/dhcpsrv/pool.cc
+++ b/src/lib/dhcpsrv/pool.cc
@@ -31,6 +31,14 @@ bool Pool::inRange(const isc::asiolink::IOAddress& addr) const {
     return (first_.smallerEqual(addr) && addr.smallerEqual(last_));
 }
 
+std::string
+Pool::toText() const {
+    std::stringstream tmp;
+    tmp << "type=" << Lease::typeToText(type_) << ", " << first_.toText()
+        << "-" << last_.toText();
+    return (tmp.str());
+}
+
 Pool4::Pool4(const isc::asiolink::IOAddress& first,
              const isc::asiolink::IOAddress& last)
 :Pool(Lease::TYPE_V4, first, last) {
@@ -132,5 +140,15 @@ Pool6::Pool6(Lease::Type type, const isc::asiolink::IOAddress& prefix,
     last_ = lastAddrInPrefix(prefix, prefix_len);
 }
 
+std::string
+Pool6::toText() const {
+    std::stringstream tmp;
+    tmp << "type=" << Lease::typeToText(type_) << ", " << first_.toText()
+        << "-" << last_.toText() << ", delegated_len="
+        << static_cast<int>(prefix_len_);
+    return (tmp.str());
+}
+
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
diff --git a/src/lib/dhcpsrv/pool.h b/src/lib/dhcpsrv/pool.h
index 0d73b40149..ae8cb58e69 100644
--- a/src/lib/dhcpsrv/pool.h
+++ b/src/lib/dhcpsrv/pool.h
@@ -66,6 +66,11 @@ public:
         return (type_);
     }
 
+    /// @brief returns textual representation of the pool
+    ///
+    /// @return textual representation
+    virtual std::string toText() const;
+
     /// @brief virtual destructor
     ///
     /// We need Pool to be a polymorphic class, so we could dynamic cast
@@ -204,6 +209,11 @@ public:
         return (prefix_len_);
     }
 
+    /// @brief returns textual representation of the pool
+    ///
+    /// @return textual representation
+    virtual std::string toText() const;
+
 private:
     /// @brief Defines prefix length (for TYPE_PD only)
     uint8_t prefix_len_;
diff --git a/src/lib/dhcpsrv/tests/pool_unittest.cc b/src/lib/dhcpsrv/tests/pool_unittest.cc
index bf89508034..402ca2acef 100644
--- a/src/lib/dhcpsrv/tests/pool_unittest.cc
+++ b/src/lib/dhcpsrv/tests/pool_unittest.cc
@@ -99,9 +99,16 @@ TEST(Pool4Test, unique_id) {
             }
         }
     }
-
 }
 
+// Simple check if toText returns reasonable values
+TEST(Poo4Test,toText) {
+    Pool4 pool1(IOAddress("192.0.2.7"), IOAddress("192.0.2.17"));
+    EXPECT_EQ("type=V4, 192.0.2.7-192.0.2.17", pool1.toText());
+
+    Pool4 pool2(IOAddress("192.0.2.128"), 28);
+    EXPECT_EQ("type=V4, 192.0.2.128-192.0.2.143", pool2.toText());
+}
 
 TEST(Pool6Test, constructor_first_last) {
 
@@ -244,4 +251,16 @@ TEST(Pool6Test, unique_id) {
 
 }
 
+// Simple check if toText returns reasonable values
+TEST(Poo6Test,toText) {
+    Pool6 pool1(Lease::TYPE_NA, IOAddress("2001:db8::1"),
+                IOAddress("2001:db8::2"));
+    EXPECT_EQ("type=IA_NA, 2001:db8::1-2001:db8::2, delegated_len=128",
+              pool1.toText());
+
+    Pool6 pool2(Lease::TYPE_PD, IOAddress("2001:db8:1::"), 96, 112);
+    EXPECT_EQ("type=IA_PD, 2001:db8:1::-2001:db8:1::ffff:ffff, delegated_len=112",
+              pool2.toText());
+}
+
 }; // end of anonymous namespace