[2414] All outstanding changes after review.

src/bin/dhcp6/dhcp6_srv.cc

@@ -32,8 +32,9 @@
 #include <dhcp/cfgmgr.h>
 #include <dhcp/option6_iaaddr.h>
 
-// @todo: Replace this with MySQL_LeaseMgr once it is merged
-#include <dhcp/tests/memfile_lease_mgr.h>
+// @todo: Replace this with MySQL_LeaseMgr (or a LeaseMgr factory)
+// once it is merged
+#include <dhcp/memfile_lease_mgr.h>
 
 using namespace isc;
 using namespace isc::asiolink;
@@ -75,7 +76,8 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) {
     }
 
     // Instantiate LeaseMgr
-    // @todo: Replace this with MySQL_LeaseMgr once it is merged
+    // @todo: Replace this with MySQL_LeaseMgr (or a LeaseMgr factory)
+    // once it is merged
     new isc::dhcp::test::Memfile_LeaseMgr("");
 
     LOG_INFO(dhcp6_logger, DHCP6_DB_BACKEND_STARTED)

src/lib/dhcp/Makefile.am

@@ -34,14 +34,18 @@ libb10_dhcp___la_SOURCES += pkt6.cc pkt6.h
 libb10_dhcp___la_SOURCES += pkt4.cc pkt4.h
 libb10_dhcp___la_SOURCES += duid.cc duid.h
 
+libb10_dhcp___la_CXXFLAGS = $(AM_CXXFLAGS)
+libb10_dhcp___la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
+libb10_dhcp___la_LIBADD   = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
+libb10_dhcp___la_LIBADD  += $(top_builddir)/src/lib/util/libb10-util.la
+libb10_dhcp___la_LDFLAGS  = -no-undefined -version-info 2:0:0
+
 libb10_dhcpsrv_la_SOURCES  = cfgmgr.cc cfgmgr.h
 libb10_dhcpsrv_la_SOURCES += pool.cc pool.h
 libb10_dhcpsrv_la_SOURCES += subnet.cc subnet.h
 libb10_dhcpsrv_la_SOURCES += triplet.h
 libb10_dhcpsrv_la_SOURCES += lease_mgr.cc lease_mgr.h
-
-# @todo: Remove this once MySQL LeaseMgr is merged
-libb10_dhcpsrv_la_SOURCES += tests/memfile_lease_mgr.cc tests/memfile_lease_mgr.h
+libb10_dhcpsrv_la_SOURCES += memfile_lease_mgr.cc memfile_lease_mgr.h
 
 libb10_dhcpsrv_la_SOURCES += addr_utilities.cc addr_utilities.h
 libb10_dhcpsrv_la_SOURCES += alloc_engine.cc alloc_engine.h
@@ -54,12 +58,6 @@ libb10_dhcpsrv_la_LDFLAGS  = -no-undefined -version-info 2:0:0
 
 EXTRA_DIST  = README
 
-libb10_dhcp___la_CXXFLAGS = $(AM_CXXFLAGS)
-libb10_dhcp___la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libb10_dhcp___la_LIBADD   = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
-libb10_dhcp___la_LIBADD  += $(top_builddir)/src/lib/util/libb10-util.la
-libb10_dhcp___la_LDFLAGS  = -no-undefined -version-info 2:0:0
-
 if USE_CLANGPP
 # Disable unused parameter warning caused by some of the
 # Boost headers when compiling with clang.

src/lib/dhcp/addr_utilities.cc

@@ -16,6 +16,7 @@
 #include <exceptions/exceptions.h>
 #include <dhcp/addr_utilities.h>
 
+using namespace isc;
 using namespace isc::asiolink;
 
 namespace {
@@ -46,6 +47,10 @@ const uint8_t bitMask6[]= { 0, 0x80, 0xc0, 0xe0, 0xf0, 0xf8, 0xfc, 0xfe, 0xff };
 isc::asiolink::IOAddress firstAddrInPrefix6(const isc::asiolink::IOAddress& prefix,
                                             uint8_t len) {
 
+    if (len>128) {
+        isc_throw(BadValue, "IPv6 prefix length too large:" << len);
+    }
+
     uint8_t packed[V6ADDRESS_LEN];
 
     // First we copy the whole address as 16 bytes.
@@ -120,6 +125,9 @@ isc::asiolink::IOAddress lastAddrInPrefix4(const isc::asiolink::IOAddress& prefi
 /// @param len netmask length (0-128)
 isc::asiolink::IOAddress lastAddrInPrefix6(const isc::asiolink::IOAddress& prefix,
                                            uint8_t len) {
+    if (len>128) {
+        isc_throw(BadValue, "IPv6 prefix length too large:" << len);
+    }
 
     uint8_t packed[V6ADDRESS_LEN];
 
@@ -162,14 +170,8 @@ namespace dhcp {
 isc::asiolink::IOAddress firstAddrInPrefix(const isc::asiolink::IOAddress& prefix,
                                             uint8_t len) {
     if (prefix.getFamily() == AF_INET) {
-        if (len>32) {
-            isc_throw(BadValue, "IPv4 prefix length too large:" << len);
-        }
         return firstAddrInPrefix4(prefix, len);
     } else {
-        if (len>128) {
-            isc_throw(BadValue, "IPv6 prefix length too large:" << len);
-        }
         return firstAddrInPrefix6(prefix, len);
     }
 }
@@ -177,14 +179,8 @@ isc::asiolink::IOAddress firstAddrInPrefix(const isc::asiolink::IOAddress& prefi
 isc::asiolink::IOAddress lastAddrInPrefix(const isc::asiolink::IOAddress& prefix,
                                            uint8_t len) {
     if (prefix.getFamily() == AF_INET) {
-        if (len>32) {
-            isc_throw(BadValue, "IPv4 prefix length too large:" << len);
-        }
         return lastAddrInPrefix4(prefix, len);
     } else {
-        if (len>128) {
-            isc_throw(BadValue, "IPv6 prefix length too large:" << len);
-        }
         return lastAddrInPrefix6(prefix, len);
     }
 }

src/lib/dhcp/cfgmgr.cc

@@ -41,7 +41,7 @@ CfgMgr::getSubnet6(const isc::asiolink::IOAddress& hint) {
     // configuration. Such requirement makes sense in IPv4, but not in IPv6.
     // The server does not need to have a global address (using just link-local
     // is ok for DHCPv6 server) from the pool it serves.
-    if ( (subnets6_.size() == 1) && hint.getAddress().to_v6().is_link_local()) {
+    if ((subnets6_.size() == 1) && hint.getAddress().to_v6().is_link_local()) {
         return (subnets6_[0]);
     }
 

src/lib/dhcp/duid.cc

@@ -57,15 +57,14 @@ DUID::DUIDType DUID::getType() const {
 
 std::string DUID::toText() const {
     std::stringstream tmp;
-
+    tmp << std::hex;
     bool delim = false;
     for (std::vector<uint8_t>::const_iterator it = duid_.begin();
          it != duid_.end(); ++it) {
         if (delim) {
             tmp << ":";
         }
-        tmp.width(2);
-        tmp << std::hex << std::setfill('0') << static_cast<unsigned int>(*it);
+        tmp << std::setw(2) << std::setfill('0') << static_cast<unsigned int>(*it);
         delim = true;
     }
     return (tmp.str());

src/lib/dhcp/duid.h

@@ -65,10 +65,10 @@ class DUID {
     std::string toText() const;
 
     /// compares two DUIDs
-    bool operator == (const DUID& other) const;
+    bool operator==(const DUID& other) const;
 
     /// compares two DUIDs
-    bool operator != (const DUID& other) const;
+    bool operator!=(const DUID& other) const;
 
  protected:
     /// the actual content of the DUID

src/lib/dhcp/tests/memfile_lease_mgr.cc

@@ -19,6 +19,9 @@ using namespace isc::dhcp::test;
 
 Memfile_LeaseMgr::Memfile_LeaseMgr(const std::string& dbconfig)
     : LeaseMgr(dbconfig) {
+    std::cout << "Warning: Using memfile database backend. It is usable for" << std::endl;
+    std::cout << "Warning: limited testing only. File support not implemented yet." << std::endl;
+    std::cout << "Warning: Leases will be lost after restart." << std::endl;
 }
 
 Memfile_LeaseMgr::~Memfile_LeaseMgr() {

src/lib/dhcp/subnet.cc

@@ -53,7 +53,7 @@ Subnet::delOptions() {
     options_.clear();
 }
 
-std::string Subnet::toText() {
+std::string Subnet::toText() const {
     std::stringstream tmp;
     tmp << prefix_.toText() << "/" << static_cast<unsigned int>(prefix_len_);
     return (tmp.str());

src/lib/dhcp/subnet.h

@@ -282,14 +282,14 @@ public:
     /// @brief returns subnet parameters (prefix and prefix length)
     ///
     /// @return (prefix, prefix length) pair
-    std::pair<isc::asiolink::IOAddress, uint8_t> get() {
-        return (std::pair<isc::asiolink::IOAddress, uint8_t>(prefix_, prefix_len_));
+    std::pair<isc::asiolink::IOAddress, uint8_t> get() const {
+        return (std::make_pair(prefix_, prefix_len_));
     }
 
     /// @brief returns textual representation of the subnet (e.g. "2001:db8::/64")
     ///
     /// @return textual representation
-    virtual std::string toText();
+    virtual std::string toText() const;
 
 protected:
     /// @brief protected constructor

src/lib/dhcp/tests/Makefile.am

@@ -48,7 +48,6 @@ libdhcpsrv_unittests_SOURCES  = run_unittests.cc
 libdhcpsrv_unittests_SOURCES += cfgmgr_unittest.cc triplet_unittest.cc
 libdhcpsrv_unittests_SOURCES += pool_unittest.cc subnet_unittest.cc
 libdhcpsrv_unittests_SOURCES += addr_utilities_unittest.cc
-libdhcpsrv_unittests_SOURCES += memfile_lease_mgr.cc memfile_lease_mgr.h
 libdhcpsrv_unittests_SOURCES += lease_mgr_unittest.cc
 libdhcpsrv_unittests_SOURCES += alloc_engine_unittest.cc
 
@@ -60,7 +59,6 @@ libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/exceptions/libb10-exceptio
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcpsrv.la
-libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
 libdhcpsrv_unittests_LDADD += $(top_builddir)/src/lib/log/libb10-log.la
 
 

src/lib/dhcp/tests/alloc_engine_unittest.cc

@@ -18,7 +18,7 @@
 #include <dhcp/duid.h>
 #include <dhcp/alloc_engine.h>
 #include <dhcp/cfgmgr.h>
-#include "memfile_lease_mgr.h"
+#include <dhcp/memfile_lease_mgr.h>
 #include <boost/shared_ptr.hpp>
 #include <iostream>
 #include <sstream>

src/lib/dhcp/tests/duid_unittest.cc

@@ -170,8 +170,8 @@ TEST(ClientIdTest, operators) {
 TEST(ClientIdTest, toText) {
     uint8_t data1[] = {0, 1, 2, 3, 4, 0xff, 0xfe};
 
-    scoped_ptr<DUID> duid1(new DUID(data1, sizeof(data1)));
-    EXPECT_EQ("00:01:02:03:04:ff:fe", duid1->toText());
+    DUID duid(data1, sizeof(data1));
+    EXPECT_EQ("00:01:02:03:04:ff:fe", duid.toText());
 }
 
 } // end of anonymous namespace

src/lib/dhcp/tests/lease_mgr_unittest.cc

@@ -19,7 +19,7 @@
 #include <asiolink/io_address.h>
 #include <dhcp/lease_mgr.h>
 #include <dhcp/duid.h>
-#include "memfile_lease_mgr.h"
+#include <dhcp/memfile_lease_mgr.h>
 
 using namespace std;
 using namespace isc;

src/lib/dhcp/tests/subnet_unittest.cc

@@ -433,15 +433,15 @@ TEST(Subnet6Test, inRangeinPool) {
 
 // This test checks if the toText() method returns text representation
 TEST(Subnet6Test, toText) {
-    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8::"), 32, 1, 2, 3, 4));
-    EXPECT_EQ("2001:db8::/32", subnet->toText());
+    Subnet6 subnet(IOAddress("2001:db8::"), 32, 1, 2, 3, 4);
+    EXPECT_EQ("2001:db8::/32", subnet.toText());
 }
 
 // This test checks if the get() method returns proper parameters
 TEST(Subnet6Test, get) {
-    Subnet6Ptr subnet(new Subnet6(IOAddress("2001:db8::"), 32, 1, 2, 3, 4));
-    EXPECT_EQ("2001:db8::", subnet->get().first.toText());
-    EXPECT_EQ(32, subnet->get().second);
+    Subnet6 subnet(IOAddress("2001:db8::"), 32, 1, 2, 3, 4);
+    EXPECT_EQ("2001:db8::", subnet.get().first.toText());
+    EXPECT_EQ(32, subnet.get().second);
 }
 
 };