[3150] Changes after review:

 - lease_type_ renamed to pool_type_
 - exception message clarified
 - prefix lengths comment updated
 - couple EXPECT_NO_THROWs added to subnet unit-tests

src/lib/dhcpsrv/alloc_engine.cc

@@ -94,9 +94,9 @@ AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet,
     // Let's get the last allocated address. It is usually set correctly,
     // but there are times when it won't be (like after removing a pool or
     // perhaps restarting the server).
-    IOAddress last = subnet->getLastAllocated(lease_type_);
+    IOAddress last = subnet->getLastAllocated(pool_type_);
 
-    const PoolCollection& pools = subnet->getPools(lease_type_);
+    const PoolCollection& pools = subnet->getPools(pool_type_);
 
     if (pools.empty()) {
         isc_throw(AllocFailed, "No pools defined in selected subnet");
@@ -117,7 +117,7 @@ AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet,
     if (it == pools.end()) {
         // ok to access first element directly. We checked that pools is non-empty
         IOAddress next = pools[0]->getFirstAddress();
-        subnet->setLastAllocated(lease_type_, next);
+        subnet->setLastAllocated(pool_type_, next);
         return (next);
     }
 
@@ -126,7 +126,7 @@ AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet,
     IOAddress next = increaseAddress(last); // basically addr++
     if ((*it)->inRange(next)) {
         // the next one is in the pool as well, so we haven't hit pool boundary yet
-        subnet->setLastAllocated(lease_type_, next);
+        subnet->setLastAllocated(pool_type_, next);
         return (next);
     }
 
@@ -136,13 +136,13 @@ AllocEngine::IterativeAllocator::pickAddress(const SubnetPtr& subnet,
         // Really out of luck today. That was the last pool. Let's rewind
         // to the beginning.
         next = pools[0]->getFirstAddress();
-        subnet->setLastAllocated(lease_type_, next);
+        subnet->setLastAllocated(pool_type_, next);
         return (next);
     }
 
     // there is a next pool, let's try first address from it
     next = (*it)->getFirstAddress();
-    subnet->setLastAllocated(lease_type_, next);
+    subnet->setLastAllocated(pool_type_, next);
     return (next);
 }
 

src/lib/dhcpsrv/alloc_engine.h

@@ -80,8 +80,9 @@ protected:
         /// @brief Default constructor.
         ///
         /// Specifies which type of leases this allocator will assign
-        Allocator(Pool::PoolType lease_type)
-            :lease_type_(lease_type) {
+        /// @param pool_type specifies pool type (addresses, temp. addr or prefixes)
+        Allocator(Pool::PoolType pool_type)
+            :pool_type_(pool_type) {
         }
 
         /// @brief virtual destructor
@@ -90,7 +91,7 @@ protected:
     protected:
 
         /// @brief defines lease type allocation
-        Pool::PoolType lease_type_;
+        Pool::PoolType pool_type_;
     };
 
     /// @brief Address/prefix allocator that iterates over all addresses

src/lib/dhcpsrv/pool.cc

@@ -114,7 +114,7 @@ Pool6::Pool6(PoolType type, const isc::asiolink::IOAddress& prefix,
 
     if (prefix_len > delegated_len) {
         isc_throw(BadValue, "Delegated length (" << static_cast<int>(delegated_len)
-                  << ") must be smaller than prefix length ("
+                  << ") must be longer than prefix length ("
                   << static_cast<int>(prefix_len) << ")");
     }
 

src/lib/dhcpsrv/pool.h

@@ -45,10 +45,10 @@ public:
     /// There is a new one being worked on (IA_PA, see draft-ietf-dhc-host-gen-id), but
     /// support for it is not planned for now.
     typedef enum {
-        TYPE_IA = 0,
-        TYPE_TA = 1,
-        TYPE_PD = 2,
-        TYPE_V4 = 3
+        TYPE_IA,
+        TYPE_TA,
+        TYPE_PD,
+        TYPE_V4
     }  PoolType;
 
     /// @brief returns Pool-id
@@ -184,10 +184,16 @@ public:
     /// pool 2001:db8::/56. It will be split into 256 prefixes of length /64,
     /// e.g. 2001:db8:0:1::/64, 2001:db8:0:2::/64 etc.
     ///
-    /// Obviously, prefix_len must define bigger prefix than delegated_len,
-    /// so prefix_len < delegated_len. Note that it is slightly confusing:
-    /// bigger (larger) prefix actually has smaller prefix length, e.g.
-    /// /56 is a bigger prefix than /64.
+    /// Naming convention:
+    /// A smaller prefix length yields a shorter prefix which describes a larger
+    /// set of addresses. A larger length yields a longer prefix which describes
+    /// a smaller set of addresses.
+    ///
+    /// Obviously, prefix_len must define shorter or equal prefix length than
+    /// delegated_len, so prefix_len <= delegated_len. Note that it is slightly
+    /// confusing: bigger (larger) prefix actually has smaller prefix length,
+    /// e.g. /56 is a bigger prefix than /64, but has shorter (smaller) prefix
+    /// length.
     ///
     /// @throw BadValue if delegated_len is defined for non-PD types or
     ///        when delegated_len < prefix_len

src/lib/dhcpsrv/tests/subnet_unittest.cc

@@ -66,24 +66,25 @@ TEST(Subnet4Test, Pool4InSubnet4) {
     PoolPtr pool2(new Pool4(IOAddress("192.1.2.128"), 26));
     PoolPtr pool3(new Pool4(IOAddress("192.1.2.192"), 30));
 
-    subnet->addPool(pool1);
+    EXPECT_NO_THROW(subnet->addPool(pool1));
 
     // If there's only one pool, get that pool
     PoolPtr mypool = subnet->getAnyPool(Pool::TYPE_V4);
     EXPECT_EQ(mypool, pool1);
 
 
-    subnet->addPool(pool2);
-    subnet->addPool(pool3);
+    EXPECT_NO_THROW(subnet->addPool(pool2));
+    EXPECT_NO_THROW(subnet->addPool(pool3));
 
     // If there are more than one pool and we didn't provide hint, we
     // should get the first pool
-    mypool = subnet->getAnyPool(Pool::TYPE_V4);
+    EXPECT_NO_THROW(mypool = subnet->getAnyPool(Pool::TYPE_V4));
 
     EXPECT_EQ(mypool, pool1);
 
     // If we provide a hint, we should get a pool that this hint belongs to
-    mypool = subnet->getPool(Pool::TYPE_V4, IOAddress("192.1.2.195"));
+    EXPECT_NO_THROW(mypool = subnet->getPool(Pool::TYPE_V4,
+                                             IOAddress("192.1.2.195")));
 
     EXPECT_EQ(mypool, pool3);
 
@@ -218,14 +219,14 @@ TEST(Subnet4Test, PoolType) {
     EXPECT_THROW(subnet->getAnyPool(Pool::TYPE_PD), BadValue);
 
     // Let's add a single V4 pool and check that it can be retrieved
-    subnet->addPool(pool1);
+    EXPECT_NO_THROW(subnet->addPool(pool1));
 
     // If there's only one IA pool, get that pool (without and with hint)
     EXPECT_EQ(pool1, subnet->getAnyPool(Pool::TYPE_V4));
     EXPECT_EQ(pool1, subnet->getPool(Pool::TYPE_V4, IOAddress("192.0.1.167")));
 
     // Let's add additional V4 pool
-    subnet->addPool(pool2);
+    EXPECT_NO_THROW(subnet->addPool(pool2));
 
     // Try without hints
     EXPECT_EQ(pool1, subnet->getAnyPool(Pool::TYPE_V4));
@@ -322,7 +323,7 @@ TEST(Subnet6Test, PoolTypes) {
     EXPECT_THROW(subnet->getAnyPool(Pool::TYPE_V4), BadValue);
 
     // Let's add a single IA pool and check that it can be retrieved
-    subnet->addPool(pool1);
+    EXPECT_NO_THROW(subnet->addPool(pool1));
 
     // If there's only one IA pool, get that pool
     EXPECT_EQ(pool1, subnet->getAnyPool(Pool::TYPE_IA));
@@ -337,8 +338,8 @@ TEST(Subnet6Test, PoolTypes) {
     EXPECT_EQ(PoolPtr(), subnet->getPool(Pool::TYPE_TA, IOAddress("2001:db8:1:3::1")));
 
     // Let's add TA and PD pools
-    subnet->addPool(pool2);
-    subnet->addPool(pool3);
+    EXPECT_NO_THROW(subnet->addPool(pool2));
+    EXPECT_NO_THROW(subnet->addPool(pool3));
 
     // Try without hints
     EXPECT_EQ(pool1, subnet->getAnyPool(Pool::TYPE_IA));
@@ -356,7 +357,7 @@ TEST(Subnet6Test, PoolTypes) {
     EXPECT_EQ(pool3, subnet->getPool(Pool::TYPE_PD, IOAddress("2001:db8:1:7::1")));
 
     // Let's add a second PD pool
-    subnet->addPool(pool4);
+    EXPECT_NO_THROW(subnet->addPool(pool4));
 
     // Without hints, it should return the first pool
     EXPECT_EQ(pool3, subnet->getAnyPool(Pool::TYPE_PD));