BIND 10 trac2898, updated. a19cdcff7accbce09eab6bf7ea359a830564c1f8 [2898] First set of changes after review
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue May 7 17:47:11 UTC 2013
The branch, trac2898 has been updated
via a19cdcff7accbce09eab6bf7ea359a830564c1f8 (commit)
from df3ee4b9e04459e2ba6dbc50702b3c7f29c0ceb7 (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 a19cdcff7accbce09eab6bf7ea359a830564c1f8
Author: Tomek Mrugalski <tomasz at isc.org>
Date: Tue May 7 19:46:57 2013 +0200
[2898] First set of changes after review
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/config_parser.cc | 4 +--
src/bin/dhcp6/dhcp6_srv.cc | 34 +++++++++-----------
src/bin/dhcp6/tests/config_parser_unittest.cc | 38 ++++++++++------------
src/bin/dhcp6/tests/dhcp6_srv_unittest.cc | 42 +++++++++++++++----------
src/lib/dhcpsrv/cfgmgr.cc | 2 +-
5 files changed, 59 insertions(+), 61 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc
index 243d217..f386243 100644
--- a/src/bin/dhcp6/config_parser.cc
+++ b/src/bin/dhcp6/config_parser.cc
@@ -1490,7 +1490,7 @@ private:
std::string ifaceid;
try {
ifaceid = string_values_.getParam("interface-id");
- } catch (DhcpConfigError) {
+ } catch (const DhcpConfigError&) {
// interface-id is not mandatory
}
@@ -1503,7 +1503,7 @@ private:
}
stringstream tmp;
- tmp << addr.toText() << "/" << (int)len
+ tmp << addr.toText() << "/" << static_cast<int>(len)
<< " with params t1=" << t1 << ", t2=" << t2 << ", pref="
<< pref << ", valid=" << valid;
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index ec74bb1..0252c51 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -405,7 +405,7 @@ Dhcpv6Srv::copyDefaultOptions(const Pkt6Ptr& question, Pkt6Ptr& answer) {
}
/// @todo: Should throw if there is no client-id (except anonymous INF-REQUEST)
- // if this is a relayed message, we need to copy relay information
+ // If this is a relayed message, we need to copy relay information
if (!question->relay_info_.empty()) {
answer->copyRelayInfo(question);
}
@@ -534,14 +534,11 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
// This is a direct (non-relayed) message
// Try to find a subnet if received packet from a directly connected client
- Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(question->getIface());
- if (subnet) {
- return (subnet);
+ subnet = CfgMgr::instance().getSubnet6(question->getIface());
+ if (!subnet) {
+ // If no subnet was found, try to find it based on remote address
+ subnet = CfgMgr::instance().getSubnet6(question->getRemoteAddr());
}
-
- // If no subnet was found, try to find it based on remote address
- subnet = CfgMgr::instance().getSubnet6(question->getRemoteAddr());
- return (subnet);
} else {
// This is a relayed message
@@ -551,20 +548,19 @@ Dhcpv6Srv::selectSubnet(const Pkt6Ptr& question) {
subnet = CfgMgr::instance().getSubnet6(interface_id);
}
- if (subnet) {
- return (subnet);
- }
-
- // If no interface-id was specified (or not configured on server), let's
- // try address matching
- IOAddress link_addr = question->relay_info_.back().linkaddr_;
+ if (!subnet) {
+ // If no interface-id was specified (or not configured on server), let's
+ // try address matching
+ IOAddress link_addr = question->relay_info_.back().linkaddr_;
- // if relay filled in link_addr field, then let's use it
- if (link_addr != IOAddress("::")) {
- subnet = CfgMgr::instance().getSubnet6(link_addr);
+ // if relay filled in link_addr field, then let's use it
+ if (link_addr != IOAddress("::")) {
+ subnet = CfgMgr::instance().getSubnet6(link_addr);
+ }
}
- return (subnet);
}
+
+ return (subnet);
}
void
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 37dd783..be3e3be 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -277,13 +277,13 @@ public:
expected_data_len));
}
- int rcode_;
- Dhcpv6Srv srv_;
+ int rcode_; ///< return core (see @ref isc::config::parseAnswer)
+ Dhcpv6Srv srv_; ///< instance of the Dhcp6Srv used during tests
- ConstElementPtr comment_;
+ ConstElementPtr comment_; ///< comment (see @ref isc::config::parseAnswer)
- string valid_iface_;
- string bogus_iface_;
+ string valid_iface_; ///< name of a valid network interface (present in system)
+ string bogus_iface_; ///< name of a invalid network interface (not present in system)
};
// Goal of this test is a verification if a very simple config update
@@ -508,11 +508,9 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) {
const string valid_interface_id = "foobar";
const string bogus_interface_id = "blah";
- ConstElementPtr status;
-
// There should be at least one interface
- string config = "{ "
+ const string config = "{ "
"\"preferred-lifetime\": 3000,"
"\"rebind-timer\": 2000, "
"\"renew-timer\": 1000, "
@@ -521,24 +519,24 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) {
" \"interface-id\": \"" + valid_interface_id + "\","
" \"subnet\": \"2001:db8:1::/64\" } ],"
"\"valid-lifetime\": 4000 }";
- cout << config << endl;
ElementPtr json = Element::fromJSON(config);
+ ConstElementPtr status;
EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
- // returned value should be 0 (configuration success)
+ // Returned value should be 0 (configuration success)
ASSERT_TRUE(status);
comment_ = parseAnswer(rcode_, status);
EXPECT_EQ(0, rcode_);
- // try to get a subnet based on bogus interface-id option
+ // Try to get a subnet based on bogus interface-id option
OptionBuffer tmp(bogus_interface_id.begin(), bogus_interface_id.end());
OptionPtr ifaceid(new Option(Option::V6, D6O_INTERFACE_ID, tmp));
Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(ifaceid);
EXPECT_FALSE(subnet);
- // now try to get subnet for valid interface-id value
+ // Now try to get subnet for valid interface-id value
tmp = OptionBuffer(valid_interface_id.begin(), valid_interface_id.end());
ifaceid.reset(new Option(Option::V6, D6O_INTERFACE_ID, tmp));
subnet = CfgMgr::instance().getSubnet6(ifaceid);
@@ -551,9 +549,7 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceId) {
// parameter.
TEST_F(Dhcp6ParserTest, interfaceIdGlobal) {
- ConstElementPtr status;
-
- string config = "{ \"interface\": [ \"all\" ],"
+ const string config = "{ \"interface\": [ \"all\" ],"
"\"preferred-lifetime\": 3000,"
"\"rebind-timer\": 2000, "
"\"renew-timer\": 1000, "
@@ -562,13 +558,13 @@ TEST_F(Dhcp6ParserTest, interfaceIdGlobal) {
" \"pool\": [ \"2001:db8:1::1 - 2001:db8:1::ffff\" ],"
" \"subnet\": \"2001:db8:1::/64\" } ],"
"\"valid-lifetime\": 4000 }";
- cout << config << endl;
ElementPtr json = Element::fromJSON(config);
+ ConstElementPtr status;
EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
- // returned value should be 1 (parse error)
+ // Returned value should be 1 (parse error)
ASSERT_TRUE(status);
comment_ = parseAnswer(rcode_, status);
EXPECT_EQ(1, rcode_);
@@ -578,9 +574,7 @@ TEST_F(Dhcp6ParserTest, interfaceIdGlobal) {
// interface (i.e. local subnet) and interface-id (remote subnet) defined.
TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) {
- ConstElementPtr status;
-
- string config = "{ \"preferred-lifetime\": 3000,"
+ const string config = "{ \"preferred-lifetime\": 3000,"
"\"rebind-timer\": 2000, "
"\"renew-timer\": 1000, "
"\"subnet6\": [ { "
@@ -589,13 +583,13 @@ TEST_F(Dhcp6ParserTest, subnetInterfaceAndInterfaceId) {
" \"interface-id\": \"foobar\","
" \"subnet\": \"2001:db8:1::/64\" } ],"
"\"valid-lifetime\": 4000 }";
- cout << config << endl;
ElementPtr json = Element::fromJSON(config);
+ ConstElementPtr status;
EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
- // returned value should be 0 (configuration success)
+ // Returned value should be 0 (configuration success)
ASSERT_TRUE(status);
comment_ = parseAnswer(rcode_, status);
EXPECT_EQ(1, rcode_);
diff --git a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
index d63d518..53b1daf 100644
--- a/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/dhcp6_srv_unittest.cc
@@ -80,7 +80,7 @@ public:
static const char* DUID_FILE = "server-id-test.txt";
// test fixture for any tests requiring blank/empty configuration
-// serves as base class for additional tests
+// serves as base class for additional tests
class NakedDhcpv6SrvTest : public ::testing::Test {
public:
@@ -146,12 +146,12 @@ public:
// Checks if server response is a NAK
void checkNakResponse(const Pkt6Ptr& rsp, uint8_t expected_message_type,
- uint32_t expected_transid,
+ uint32_t expected_transid,
uint16_t expected_status_code) {
// Check if we get response at all
checkResponse(rsp, expected_message_type, expected_transid);
- // Check that IA_NA was returned
+ // Check that IA_NA was returned
OptionPtr option_ia_na = rsp->getOption(D6O_IA_NA);
ASSERT_TRUE(option_ia_na);
@@ -237,7 +237,7 @@ public:
ConstElementPtr comment_;
};
-// Provides suport for tests against a preconfigured subnet6
+// Provides suport for tests against a preconfigured subnet6
// extends upon NakedDhcp6SrvTest
class Dhcpv6SrvTest : public NakedDhcpv6SrvTest {
public:
@@ -264,7 +264,7 @@ public:
ADD_FAILURE() << "IA_NA option not present in response";
return (boost::shared_ptr<Option6IAAddr>());
}
-
+
boost::shared_ptr<Option6IA> ia = boost::dynamic_pointer_cast<Option6IA>(tmp);
if (!ia) {
ADD_FAILURE() << "IA_NA cannot convert option ptr to Option6";
@@ -274,7 +274,7 @@ public:
EXPECT_EQ(expected_iaid, ia->getIAID());
EXPECT_EQ(expected_t1, ia->getT1());
EXPECT_EQ(expected_t2, ia->getT2());
-
+
tmp = ia->getOption(D6O_IAADDR);
boost::shared_ptr<Option6IAAddr> addr = boost::dynamic_pointer_cast<Option6IAAddr>(tmp);
return (addr);
@@ -330,10 +330,10 @@ public:
};
// This test verifies that incoming SOLICIT can be handled properly when
-// there are no subnets configured.
+// there are no subnets configured.
//
-// This test sends a SOLICIT and the expected response
-// is an ADVERTISE with STATUS_NoAddrsAvail and no address provided in the
+// This test sends a SOLICIT and the expected response
+// is an ADVERTISE with STATUS_NoAddrsAvail and no address provided in the
// response
TEST_F(NakedDhcpv6SrvTest, SolicitNoSubnet) {
NakedDhcpv6Srv srv(0);
@@ -352,10 +352,10 @@ TEST_F(NakedDhcpv6SrvTest, SolicitNoSubnet) {
}
// This test verifies that incoming REQUEST can be handled properly when
-// there are no subnets configured.
+// there are no subnets configured.
//
-// This test sends a REQUEST and the expected response
-// is an REPLY with STATUS_NoAddrsAvail and no address provided in the
+// This test sends a REQUEST and the expected response
+// is an REPLY with STATUS_NoAddrsAvail and no address provided in the
// response
TEST_F(NakedDhcpv6SrvTest, RequestNoSubnet) {
NakedDhcpv6Srv srv(0);
@@ -386,8 +386,8 @@ TEST_F(NakedDhcpv6SrvTest, RequestNoSubnet) {
// This test verifies that incoming RENEW can be handled properly, even when
// no subnets are configured.
//
-// This test sends a RENEW and the expected response
-// is an REPLY with STATUS_NoBinding and no address provided in the
+// This test sends a RENEW and the expected response
+// is an REPLY with STATUS_NoBinding and no address provided in the
// response
TEST_F(NakedDhcpv6SrvTest, RenewNoSubnet) {
NakedDhcpv6Srv srv(0);
@@ -421,8 +421,8 @@ TEST_F(NakedDhcpv6SrvTest, RenewNoSubnet) {
// This test verifies that incoming RELEASE can be handled properly, even when
// no subnets are configured.
//
-// This test sends a RELEASE and the expected response
-// is an REPLY with STATUS_NoBinding and no address provided in the
+// This test sends a RELEASE and the expected response
+// is an REPLY with STATUS_NoBinding and no address provided in the
// response
TEST_F(NakedDhcpv6SrvTest, ReleaseNoSubnet) {
NakedDhcpv6Srv srv(0);
@@ -951,6 +951,8 @@ TEST_F(Dhcpv6SrvTest, RequestBasic) {
TEST_F(Dhcpv6SrvTest, ManyRequests) {
NakedDhcpv6Srv srv(0);
+ ASSERT_TRUE(subnet_);
+
Pkt6Ptr req1 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 1234));
Pkt6Ptr req2 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 2345));
Pkt6Ptr req3 = Pkt6Ptr(new Pkt6(DHCPV6_REQUEST, 3456));
@@ -995,6 +997,10 @@ TEST_F(Dhcpv6SrvTest, ManyRequests) {
boost::shared_ptr<Option6IAAddr> addr3 = checkIA_NA(reply3, 3, subnet_->getT1(),
subnet_->getT2());
+ ASSERT_TRUE(addr1);
+ ASSERT_TRUE(addr2);
+ ASSERT_TRUE(addr3);
+
// Check that the assigned address is indeed from the configured pool
checkIAAddr(addr1, addr1->getAddress(), subnet_->getPreferred(), subnet_->getValid());
checkIAAddr(addr2, addr2->getAddress(), subnet_->getPreferred(), subnet_->getValid());
@@ -1083,6 +1089,8 @@ TEST_F(Dhcpv6SrvTest, RenewBasic) {
boost::shared_ptr<Option6IAAddr> addr_opt = checkIA_NA(reply, 234, subnet_->getT1(),
subnet_->getT2());
+ ASSERT_TRUE(addr_opt);
+
// Check that we've got the address we requested
checkIAAddr(addr_opt, addr, subnet_->getPreferred(), subnet_->getValid());
@@ -1649,7 +1657,7 @@ TEST_F(Dhcpv6SrvTest, selectSubnetRelayLinkaddr) {
CfgMgr::instance().addSubnet6(subnet2);
CfgMgr::instance().addSubnet6(subnet3);
- // source of the packet should have no meaning. Selection is based
+ // Source of the packet should have no meaning. Selection is based
// on linkaddr field in the relay
pkt->setRemoteAddr(IOAddress("2001:db8:1::baca"));
selected = srv.selectSubnet(pkt);
diff --git a/src/lib/dhcpsrv/cfgmgr.cc b/src/lib/dhcpsrv/cfgmgr.cc
index 8c0742c..745322e 100644
--- a/src/lib/dhcpsrv/cfgmgr.cc
+++ b/src/lib/dhcpsrv/cfgmgr.cc
@@ -147,7 +147,7 @@ CfgMgr::getSubnet6(const isc::asiolink::IOAddress& hint) {
// If there's only one subnet configured, let's just use it
// The idea is to keep small deployments easy. In a small network - one
- // router that also runs DHCPv6 server. Users specifies a single pool and
+ // router that also runs DHCPv6 server. User specifies a single pool and
// expects it to just work. Without this, the server would complain that it
// doesn't have IP address on its interfaces that matches that
// configuration. Such requirement makes sense in IPv4, but not in IPv6.
More information about the bind10-changes
mailing list