[2237] Tests for addr utilities added, other minor fixes after review.

src/lib/dhcp/addr_utilities.cc

@@ -40,6 +40,13 @@ const uint8_t bitMask6[]= { 0, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff };
 namespace isc {
 namespace dhcp {
 
+/// @brief calculates the first IPv6 address in a IPv6 prefix
+///
+/// Note: This is a private function. Do not use it directly.
+/// Please use firstAddrInPrefix() instead.
+///
+/// @param prefix IPv6 prefix
+/// @param len prefix length
 isc::asiolink::IOAddress firstAddrInPrefix6(const isc::asiolink::IOAddress& prefix,
                                             uint8_t len) {
 
@@ -74,9 +81,12 @@ isc::asiolink::IOAddress firstAddrInPrefix6(const isc::asiolink::IOAddress& pref
     return (isc::asiolink::IOAddress::from_bytes(AF_INET6, packed));
 }
 
-/// @brief IPv4 version of firstAddrInPrefix4
+/// @brief calculates the first IPv4 address in a IPv4 prefix
 ///
-/// @param prefix IPv4 prefix that we calculate first address for
+/// Note: This is a private function. Do not use it directly.
+/// Please use firstAddrInPrefix() instead.
+///
+/// @param prefix IPv4 prefix
 /// @param len netmask length (0-32)
 isc::asiolink::IOAddress firstAddrInPrefix4(const isc::asiolink::IOAddress& prefix,
                                             uint8_t len) {
@@ -97,6 +107,13 @@ isc::asiolink::IOAddress firstAddrInPrefix(const isc::asiolink::IOAddress& prefi
     }
 }
 
+/// @brief calculates the last IPv4 address in a IPv4 prefix
+///
+/// Note: This is a private function. Do not use it directly.
+/// Please use firstAddrInPrefix() instead.
+///
+/// @param prefix IPv4 prefix that we calculate first address for
+/// @param len netmask length (0-32)
 isc::asiolink::IOAddress lastAddrInPrefix4(const isc::asiolink::IOAddress& prefix,
                                            uint8_t len) {
     uint32_t addr = prefix;
@@ -107,6 +124,13 @@ isc::asiolink::IOAddress lastAddrInPrefix4(const isc::asiolink::IOAddress& prefi
     return (IOAddress(addr | bitMask4[len]));
 }
 
+/// @brief calculates the last IPv6 address in a IPv6 prefix
+///
+/// Note: This is a private function. Do not use it directly.
+/// Please use lastAddrInPrefix() instead.
+///
+/// @param prefix IPv6 prefix that we calculate first address for
+/// @param len netmask length (0-128)
 isc::asiolink::IOAddress lastAddrInPrefix6(const isc::asiolink::IOAddress& prefix,
                                            uint8_t len) {
 

src/lib/dhcp/tests/addr_utilities_unittest.cc

@@ -26,7 +26,67 @@ using namespace std;
 using namespace isc::dhcp;
 using namespace isc::asiolink;
 
-TEST(Pool6Test, lastAddrInPrefix) {
+// This test verifies that lastAddrInPrefix is able to handle IPv4 operations.
+TEST(AddrUtilitiesTest, lastAddrInPrefix4) {
+    IOAddress addr1("192.0.2.1");
+
+    // Prefixes rounded to addresses are easy...
+    EXPECT_EQ("192.255.255.255", lastAddrInPrefix(addr1, 8).toText());
+    EXPECT_EQ("192.0.255.255",   lastAddrInPrefix(addr1, 16).toText());
+    EXPECT_EQ("192.0.2.255",     lastAddrInPrefix(addr1, 24).toText());
+
+    // these are trickier
+    EXPECT_EQ("192.0.2.127", lastAddrInPrefix(addr1, 25).toText());
+    EXPECT_EQ("192.0.2.63",  lastAddrInPrefix(addr1, 26).toText());
+    EXPECT_EQ("192.0.2.31",  lastAddrInPrefix(addr1, 27).toText());
+    EXPECT_EQ("192.0.2.15",  lastAddrInPrefix(addr1, 28).toText());
+    EXPECT_EQ("192.0.2.7",   lastAddrInPrefix(addr1, 29).toText());
+    EXPECT_EQ("192.0.2.3",   lastAddrInPrefix(addr1, 30).toText());
+
+    // that doesn't make much sense as /31 subnet consists of network address
+    // and a broadcast address, with 0 usable addresses.
+    EXPECT_EQ("192.0.2.1",   lastAddrInPrefix(addr1, 31).toText());
+    EXPECT_EQ("192.0.2.1",   lastAddrInPrefix(addr1, 32).toText());
+
+    // Let's check extreme cases
+    IOAddress anyAddr("0.0.0.0");
+    EXPECT_EQ("127.255.255.255", lastAddrInPrefix(anyAddr, 1).toText());
+    EXPECT_EQ("255.255.255.255", lastAddrInPrefix(anyAddr, 0).toText());
+    EXPECT_EQ("0.0.0.0", lastAddrInPrefix(anyAddr, 32).toText());
+}
+
+// This test checks if firstAddrInPrefix is able to handle IPv4 operations.
+TEST(AddrUtilitiesTest, firstAddrInPrefix4) {
+    IOAddress addr1("192.223.2.255");
+
+    // Prefixes rounded to addresses are easy...
+    EXPECT_EQ("192.0.0.0",   firstAddrInPrefix(addr1, 8).toText());
+    EXPECT_EQ("192.223.0.0", firstAddrInPrefix(addr1, 16).toText());
+    EXPECT_EQ("192.223.2.0", firstAddrInPrefix(addr1, 24).toText());
+
+    // these are trickier
+    EXPECT_EQ("192.223.2.128", firstAddrInPrefix(addr1, 25).toText());
+    EXPECT_EQ("192.223.2.192", firstAddrInPrefix(addr1, 26).toText());
+    EXPECT_EQ("192.223.2.224", firstAddrInPrefix(addr1, 27).toText());
+    EXPECT_EQ("192.223.2.240", firstAddrInPrefix(addr1, 28).toText());
+    EXPECT_EQ("192.223.2.248", firstAddrInPrefix(addr1, 29).toText());
+    EXPECT_EQ("192.223.2.252", firstAddrInPrefix(addr1, 30).toText());
+
+    // that doesn't make much sense as /31 subnet consists of network address
+    // and a broadcast address, with 0 usable addresses.
+    EXPECT_EQ("192.223.2.254", firstAddrInPrefix(addr1, 31).toText());
+    EXPECT_EQ("192.223.2.255", firstAddrInPrefix(addr1, 32).toText());
+
+    // Let's check extreme cases.
+    IOAddress bcast("255.255.255.255");
+    EXPECT_EQ("128.0.0.0", firstAddrInPrefix(bcast, 1).toText());
+    EXPECT_EQ("0.0.0.0", firstAddrInPrefix(bcast, 0).toText());
+    EXPECT_EQ("255.255.255.255", firstAddrInPrefix(bcast, 32).toText());
+
+}
+
+/// This test checks if lastAddrInPrefix properly supports IPv6 operations
+TEST(AddrUtilitiesTest, lastAddrInPrefix6) {
     IOAddress addr1("2001:db8:1:1234:5678:abcd:1234:beef");
 
     // Prefixes rounded to nibbles are easy...
@@ -63,7 +123,8 @@ TEST(Pool6Test, lastAddrInPrefix) {
     EXPECT_EQ("::", lastAddrInPrefix(anyAddr, 128).toText());
 }
 
-TEST(Pool6Test, firstAddrInPrefix) {
+/// This test checks if firstAddrInPrefix properly supports IPv6 operations
+TEST(AddrUtilitiesTest, firstAddrInPrefix6) {
     IOAddress addr1("2001:db8:1:1234:5678:1234:abcd:beef");
 
     // Prefixes rounded to nibbles are easy...

src/lib/dhcp/tests/cfgmgr_unittest.cc

@@ -51,11 +51,16 @@ TEST(CfgMgrTest, subnet4) {
     // Now we have only one subnet, any request will be served from it
     EXPECT_EQ(subnet1, cfg_mgr.getSubnet4(IOAddress("192.0.2.63")));
 
+    // Now we add more subnets and check that both old and new subnets
+    // are accessible.
     cfg_mgr.addSubnet4(subnet2);
     cfg_mgr.addSubnet4(subnet3);
 
     EXPECT_EQ(subnet3, cfg_mgr.getSubnet4(IOAddress("192.0.2.191")));
+    EXPECT_EQ(subnet1, cfg_mgr.getSubnet4(IOAddress("192.0.2.15")));
     EXPECT_EQ(subnet2, cfg_mgr.getSubnet4(IOAddress("192.0.2.85")));
+
+    // Try to find an address that does not belong to any subnet
     EXPECT_EQ(Subnet4Ptr(), cfg_mgr.getSubnet4(IOAddress("192.0.2.192")));
 }