BIND 10 trac2342, updated. 5520594354d6170126103c973b77dc017b4b0285 [2342] Miscellaneous changes as a result of review
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Nov 12 14:59:06 UTC 2012
The branch, trac2342 has been updated
via 5520594354d6170126103c973b77dc017b4b0285 (commit)
from 4e7a8f7575704052e70432f1e2508d310e8f6bb0 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 5520594354d6170126103c973b77dc017b4b0285
Author: Stephen Morris <stephen at isc.org>
Date: Mon Nov 12 14:57:03 2012 +0000
[2342] Miscellaneous changes as a result of review
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/dhcp6_srv.cc | 33 ++++++++--------------
src/bin/dhcp6/dhcp6_srv.h | 5 +++-
src/lib/dhcp/Makefile.am | 3 +-
src/lib/dhcp/lease_mgr.cc | 7 -----
src/lib/dhcp/lease_mgr.h | 9 +++---
src/lib/dhcp/lease_mgr_factory.cc | 24 ++++++++--------
src/lib/dhcp/lease_mgr_factory.h | 8 +++---
src/lib/dhcp/memfile_lease_mgr.h | 4 +--
src/lib/dhcp/mysql_lease_mgr.h | 3 +-
src/lib/dhcp/tests/alloc_engine_unittest.cc | 16 ++++-------
src/lib/dhcp/tests/lease_mgr_factory_unittest.cc | 3 ++
11 files changed, 49 insertions(+), 66 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 5f844a3..1c76b75 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -39,10 +39,6 @@
#include <dhcp/cfgmgr.h>
#include <dhcp/option6_iaaddr.h>
-// @todo: Replace this with MySQL_LeaseMgr (or a LeaseMgr factory)
-// once it is merged
-#include <dhcp/memfile_lease_mgr.h>
-
#include <boost/foreach.hpp>
using namespace isc;
@@ -54,8 +50,8 @@ using namespace std;
namespace isc {
namespace dhcp {
-Dhcpv6Srv::Dhcpv6Srv(uint16_t port) : alloc_engine_(), serverid_(),
- shutdown_(false) {
+Dhcpv6Srv::Dhcpv6Srv(uint16_t port, const char* dbconfig)
+ : alloc_engine_(), serverid_(), shutdown_(true) {
LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_OPEN_SOCKET).arg(port);
@@ -74,7 +70,6 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) : alloc_engine_(), serverid_(),
if (port > 0) {
if (IfaceMgr::instance().countIfaces() == 0) {
LOG_ERROR(dhcp6_logger, DHCP6_NO_INTERFACES);
- shutdown_ = true;
return;
}
IfaceMgr::instance().openSockets6(port);
@@ -82,25 +77,21 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) : alloc_engine_(), serverid_(),
setServerID();
+ // Instantiate LeaseMgr
+ LeaseMgrFactory::create(dbconfig);
+ LOG_INFO(dhcp6_logger, DHCP6_DB_BACKEND_STARTED)
+ .arg(LeaseMgrFactory::instance().getName());
+
+ // Instantiate allocation engine
+ alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
+
} catch (const std::exception &e) {
LOG_ERROR(dhcp6_logger, DHCP6_SRV_CONSTRUCT_ERROR).arg(e.what());
- shutdown_ = true;
return;
}
- // Instantiate LeaseMgr
- // @todo: Replace this with MySQL_LeaseMgr (or a LeaseMgr factory)
- // once it is merged
-#ifdef HAVE_MYSQL
- LeaseMgrFactory::create("type=mysql user=kea password=kea name=kea host=localhost");
-#else
- LeaseMgrFactory::create("type=memfile");
-#endif
- LOG_INFO(dhcp6_logger, DHCP6_DB_BACKEND_STARTED)
- .arg(LeaseMgrFactory::instance().getName());
-
- // Instantiate allocation engine
- alloc_engine_.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
+ // All done, so can proceed
+ shutdown_ = false;
}
Dhcpv6Srv::~Dhcpv6Srv() {
diff --git a/src/bin/dhcp6/dhcp6_srv.h b/src/bin/dhcp6/dhcp6_srv.h
index 6574226..f4f0f0c 100644
--- a/src/bin/dhcp6/dhcp6_srv.h
+++ b/src/bin/dhcp6/dhcp6_srv.h
@@ -57,7 +57,10 @@ public:
/// old or create new DUID.
///
/// @param port port on will all sockets will listen
- Dhcpv6Srv(uint16_t port = DHCP6_SERVER_PORT);
+ /// @param dbconfig Lease manager configuration string. The default
+ /// of the "memfile" manager is used for testing.
+ Dhcpv6Srv(uint16_t port = DHCP6_SERVER_PORT,
+ const char* dbconfig = "type=memfile");
/// @brief Destructor. Used during DHCPv6 service shutdown.
virtual ~Dhcpv6Srv();
diff --git a/src/lib/dhcp/Makefile.am b/src/lib/dhcp/Makefile.am
index 904bce3..017a13e 100644
--- a/src/lib/dhcp/Makefile.am
+++ b/src/lib/dhcp/Makefile.am
@@ -59,8 +59,7 @@ libb10_dhcpsrv_la_SOURCES += triplet.h
libb10_dhcpsrv_la_CXXFLAGS = $(AM_CXXFLAGS)
libb10_dhcpsrv_la_CPPFLAGS = $(AM_CPPFLAGS) $(LOG4CPLUS_INCLUDES)
-libb10_dhcpsrv_la_LIBADD = $(top_builddir)/src/lib/dhcp/libb10-dhcp++.la
-libb10_dhcpsrv_la_LIBADD += $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
+libb10_dhcpsrv_la_LIBADD = $(top_builddir)/src/lib/asiolink/libb10-asiolink.la
libb10_dhcpsrv_la_LIBADD += $(top_builddir)/src/lib/util/libb10-util.la
libb10_dhcpsrv_la_LDFLAGS = -no-undefined -version-info 2:0:0
if HAVE_MYSQL
diff --git a/src/lib/dhcp/lease_mgr.cc b/src/lib/dhcp/lease_mgr.cc
index 8fed916..c7dba6c 100644
--- a/src/lib/dhcp/lease_mgr.cc
+++ b/src/lib/dhcp/lease_mgr.cc
@@ -96,10 +96,3 @@ Lease6::operator==(const Lease6& other) const {
subnet_id_ == other.subnet_id_
);
}
-
-LeaseMgr::LeaseMgr(const LeaseMgr::ParameterMap& parameters)
- : parameters_(parameters) {
-}
-
-LeaseMgr::~LeaseMgr() {
-}
diff --git a/src/lib/dhcp/lease_mgr.h b/src/lib/dhcp/lease_mgr.h
index 05eca4d..40c20cd 100644
--- a/src/lib/dhcp/lease_mgr.h
+++ b/src/lib/dhcp/lease_mgr.h
@@ -179,7 +179,7 @@ struct Lease4 {
/// @brief Constructor
///
/// Initialize fields that don't have a default constructor.
- /// @TODO Remove this
+ /// @todo Remove this
Lease4() : addr_(0) {}
};
@@ -345,10 +345,12 @@ public:
///
/// @param parameters A data structure relating keywords and values
/// concerned with the database.
- LeaseMgr(const ParameterMap& parameters);
+ LeaseMgr(const ParameterMap& parameters) : parameters_(parameters)
+ {}
/// @brief Destructor
- ~LeaseMgr();
+ virtual ~LeaseMgr()
+ {}
/// @brief Adds an IPv4 lease.
///
@@ -389,7 +391,6 @@ public:
/// a single lease, not a container of leases.
///
/// @param addr address of the searched lease
- /// @param subnet_id ID of the subnet the lease must belong to
///
/// @return smart pointer to the lease (or NULL if a lease is not found)
virtual Lease4Ptr getLease4(const isc::asiolink::IOAddress& addr) const = 0;
diff --git a/src/lib/dhcp/lease_mgr_factory.cc b/src/lib/dhcp/lease_mgr_factory.cc
index 08a04e4..7e75633 100644
--- a/src/lib/dhcp/lease_mgr_factory.cc
+++ b/src/lib/dhcp/lease_mgr_factory.cc
@@ -14,25 +14,23 @@
#include "config.h"
+#include <dhcp/lease_mgr_factory.h>
+#include <dhcp/memfile_lease_mgr.h>
+#ifdef HAVE_MYSQL
+#include <dhcp/mysql_lease_mgr.h>
+#endif
+
+#include <boost/algorithm/string.hpp>
+#include <boost/foreach.hpp>
+#include <boost/scoped_ptr.hpp>
+
#include <algorithm>
#include <iostream>
#include <iterator>
#include <map>
#include <sstream>
-#include <string>
#include <utility>
-#include <boost/foreach.hpp>
-#include <boost/scoped_ptr.hpp>
-#include <boost/algorithm/string.hpp>
-#include <exceptions/exceptions.h>
-#include <dhcp/lease_mgr_factory.h>
-
-#include <dhcp/memfile_lease_mgr.h>
-#ifdef HAVE_MYSQL
-#include <dhcp/mysql_lease_mgr.h>
-#endif
-
using namespace std;
namespace isc {
@@ -48,7 +46,7 @@ LeaseMgr::ParameterMap
LeaseMgrFactory::parse(const std::string& dbconfig) {
LeaseMgr::ParameterMap mapped_tokens;
- if (! dbconfig.empty()) {
+ if (!dbconfig.empty()) {
vector<string> tokens;
// We need to pass a string to is_any_of, not just char*. Otherwise
diff --git a/src/lib/dhcp/lease_mgr_factory.h b/src/lib/dhcp/lease_mgr_factory.h
index 39265c8..c691e12 100644
--- a/src/lib/dhcp/lease_mgr_factory.h
+++ b/src/lib/dhcp/lease_mgr_factory.h
@@ -51,7 +51,7 @@ public:
/// Strictly speaking these functions could be stand-alone functions. However,
/// it is convenient to encapsulate them in a class for naming purposes.
///
-/// @TODO: Will need to develop some form of registration mechanism for
+/// @todo: Will need to develop some form of registration mechanism for
/// user-supplied backends (so that there is no need to modify the code).
class LeaseMgrFactory {
public:
@@ -62,8 +62,8 @@ public:
/// appropriate type. The actual lease manager is returned by the
/// "instance" method.
///
- /// Note: when called, the current lease manager is *always* destroyed
- /// and a new one created - even if the parameters are the same.
+ /// @note When called, the current lease manager is <b>always</b> destroyed
+ /// and a new one created - even if the parameters are the same.
///
/// dbconfig is a generic way of passing parameters. Parameters are passed
/// in the "name=value" format, separated by spaces. The data MUST include
@@ -104,7 +104,7 @@ public:
///
/// @param dbconfig Database configuration string
///
- /// @return std::map<>std::string, std::string> Map of keyword/value pairs.
+ /// @return std::map<std::string, std::string> Map of keyword/value pairs.
static LeaseMgr::ParameterMap parse(const std::string& dbconfig);
private:
diff --git a/src/lib/dhcp/memfile_lease_mgr.h b/src/lib/dhcp/memfile_lease_mgr.h
index e3f415d..cb02360 100644
--- a/src/lib/dhcp/memfile_lease_mgr.h
+++ b/src/lib/dhcp/memfile_lease_mgr.h
@@ -169,7 +169,7 @@ public:
///
/// @todo Not implemented yet
///
- /// @param lease4 The lease to be updated.
+ /// @param lease6 The lease to be updated.
///
/// If no such lease is present, an exception will be thrown.
void updateLease6(const Lease6Ptr& lease6);
@@ -237,5 +237,5 @@ protected:
}; // end of isc::dhcp namespace
}; // end of isc namespace
-#endif // MEMFILE_LEASE_MGR_HSE4
+#endif // MEMFILE_LEASE_MGR
diff --git a/src/lib/dhcp/mysql_lease_mgr.h b/src/lib/dhcp/mysql_lease_mgr.h
index 5ebeadd..8712c33 100644
--- a/src/lib/dhcp/mysql_lease_mgr.h
+++ b/src/lib/dhcp/mysql_lease_mgr.h
@@ -111,7 +111,6 @@ public:
/// a single lease, not a container of leases.
///
/// @param addr address of the searched lease
- /// @param subnet_id ID of the subnet the lease must belong to
///
/// @return smart pointer to the lease (or NULL if a lease is not found)
virtual Lease4Ptr getLease4(const isc::asiolink::IOAddress& addr) const;
@@ -327,7 +326,7 @@ public:
///
/// @param expire Reference to MYSQL_TIME object from where the expiry
/// time of the lease is taken.
- /// @param lease_time lifetime of the lease.
+ /// @param valid_lifetime lifetime of the lease.
/// @param cltt Reference to location where client last transmit time
/// is put.
static
diff --git a/src/lib/dhcp/tests/alloc_engine_unittest.cc b/src/lib/dhcp/tests/alloc_engine_unittest.cc
index d4290a3..456ad53 100644
--- a/src/lib/dhcp/tests/alloc_engine_unittest.cc
+++ b/src/lib/dhcp/tests/alloc_engine_unittest.cc
@@ -95,15 +95,13 @@ public:
// This test checks if the Allocation Engine can be instantiated and that it
// parses parameters string properly.
TEST_F(AllocEngineTest, constructor) {
- AllocEngine* x = NULL;
+ boost::scoped_ptr<AllocEngine> x;
// Hashed and random allocators are not supported yet
- ASSERT_THROW(x = new AllocEngine(AllocEngine::ALLOC_HASHED, 5), NotImplemented);
- ASSERT_THROW(x = new AllocEngine(AllocEngine::ALLOC_RANDOM, 5), NotImplemented);
+ ASSERT_THROW(x.reset(new AllocEngine(AllocEngine::ALLOC_HASHED, 5)), NotImplemented);
+ ASSERT_THROW(x.reset(new AllocEngine(AllocEngine::ALLOC_RANDOM, 5)), NotImplemented);
- ASSERT_NO_THROW(x = new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100));
-
- delete x;
+ ASSERT_NO_THROW(x.reset(new AllocEngine(AllocEngine::ALLOC_ITERATIVE, 100)));
}
/// @todo: This method is taken from mysql_lease_mgr_utilities.cc from ticket
@@ -269,15 +267,13 @@ TEST_F(AllocEngineTest, allocBogusHint) {
// This test verifies that the allocator picks addresses that belong to the
// pool
TEST_F(AllocEngineTest, IterativeAllocator) {
- NakedAllocEngine::Allocator* alloc = new NakedAllocEngine::IterativeAllocator();
+ boost::scoped_ptr<NakedAllocEngine::Allocator>
+ alloc(new NakedAllocEngine::IterativeAllocator());
for (int i = 0; i < 1000; ++i) {
IOAddress candidate = alloc->pickAddress(subnet_, duid_, IOAddress("::"));
-
EXPECT_TRUE(subnet_->inPool(candidate));
}
-
- delete alloc;
}
diff --git a/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc b/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc
index c3cb5cf..37063b2 100644
--- a/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc
+++ b/src/lib/dhcp/tests/lease_mgr_factory_unittest.cc
@@ -23,6 +23,9 @@
using namespace std;
using namespace isc::dhcp;
+// This set of tests only check the parsing functions of LeaseMgrFactory.
+// Tests of the LeaseMgr create/instance/destroy are implicitly carried out
+// in the tests for the different concrete lease managers (e.g. MySqlLeaseMgr).
namespace {
// empty class for now, but may be extended once Addr6 becomes bigger
More information about the bind10-changes
mailing list