BIND 10 trac2634, updated. 3b6cd417ea4ade9d616b12e7cb0577f892bf18b6 [2634] Addressed review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Apr 4 12:38:03 UTC 2013
The branch, trac2634 has been updated
via 3b6cd417ea4ade9d616b12e7cb0577f892bf18b6 (commit)
from ff8f84b695de0e5350af325c675ab2bdbab79cc0 (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 3b6cd417ea4ade9d616b12e7cb0577f892bf18b6
Author: Thomas Markwalder <tmark at isc.org>
Date: Thu Apr 4 08:37:33 2013 -0400
[2634] Addressed review comments.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/config_parser.cc | 6 +--
src/bin/dhcp4/tests/config_parser_unittest.cc | 3 --
src/lib/dhcpsrv/dhcp_config_parser.h | 16 ++++----
src/lib/dhcpsrv/tests/cfgmgr_unittest.cc | 51 +++++++++++++++++--------
4 files changed, 46 insertions(+), 30 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index c96db32..2391dff 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -13,13 +13,13 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <config/ccsession.h>
+#include <dhcp4/config_parser.h>
#include <dhcp4/dhcp4_log.h>
#include <dhcp/libdhcp++.h>
#include <dhcp/option_definition.h>
#include <dhcpsrv/cfgmgr.h>
-#include <dhcp4/config_parser.h>
#include <dhcpsrv/dbaccess_parser.h>
-//#include <dhcpsrv/dhcp_config_parser.h>
+#include <dhcpsrv/dhcp_config_parser.h>
#include <dhcpsrv/option_space_container.h>
#include <util/encode/hex.h>
#include <util/strutil.h>
@@ -1418,7 +1418,7 @@ private:
try {
subnet_txt = string_values_.getParam("subnet");
} catch (DhcpConfigError) {
- // rethrow with precise error
+ // Rethrow with precise error.
isc_throw(DhcpConfigError,
"Mandatory subnet definition in subnet missing");
}
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index ee60ab2..41fc9ac 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -52,11 +52,8 @@ public:
// Checks if global parameter of name have expected_value
void checkGlobalUint32(string name, uint32_t expected_value) {
- //const Uint32Storage& uint32_defaults = getUint32Defaults();
- //const ValueStorage<uint32_t>& uint32_defaults = getUint32Defaults();
const Uint32Storage& uint32_defaults = getUint32Defaults();
try {
- //uint32_defaults.addParam("boo", name);
uint32_t actual_value = uint32_defaults.getParam(name);
EXPECT_EQ(expected_value, actual_value);
} catch (DhcpConfigError) {
diff --git a/src/lib/dhcpsrv/dhcp_config_parser.h b/src/lib/dhcpsrv/dhcp_config_parser.h
index 7419809..8abdfc8 100644
--- a/src/lib/dhcpsrv/dhcp_config_parser.h
+++ b/src/lib/dhcpsrv/dhcp_config_parser.h
@@ -133,23 +133,24 @@ public:
/// @brief A template class that stores named elements of a given data type.
///
/// This template class is provides data value storage for configuration parameters
-/// of a given data type. The values are stored by parmater name and as instances
+/// of a given data type. The values are stored by parameter name and as instances
/// of type "ValueType".
///
-/// @tparam ValueType is the data type of the elements to store.
+/// @param ValueType is the data type of the elements to store.
template<typename ValueType>
class ValueStorage {
public:
/// @brief Stores the the parameter and its value in the store.
///
- /// If the parmater does not exist in the store, then it will be added,
+ /// If the parameter does not exist in the store, then it will be added,
/// otherwise its data value will be updated with the given value.
///
/// @param name is the name of the paramater to store.
/// @param value is the data value to store.
- void setParam(const std::string name, const ValueType value) {
+ void setParam(const std::string name, const ValueType& value) {
values_[name] = value;
}
+
/// @brief Returns the data value for the given parameter.
///
/// Finds and returns the data value for the given parameter.
@@ -163,12 +164,11 @@ class ValueStorage {
= values_.find(name);
if (param == values_.end()) {
- isc_throw(DhcpConfigError, "missing parameter '"
+ isc_throw(DhcpConfigError, "Missing parameter '"
<< name << "'");
}
- ValueType value = param->second;
- return (value);
+ return (param->second);
}
/// @brief Remove the parameter from the store.
@@ -177,7 +177,7 @@ class ValueStorage {
/// exists.
///
/// @param name is the name of the paramater to delete.
- void delParam(const std::string name) {
+ void delParam(const std::string& name) {
values_.erase(name);
}
diff --git a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
index 6a05b56..5776888 100644
--- a/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/cfgmgr_unittest.cc
@@ -41,61 +41,74 @@ namespace {
TEST(ValueStorageTest, BooleanTesting) {
BooleanStorage testStore;
- // verify that we can add and retrieve them
+ // Verify that we can add and retrieve parameters.
testStore.setParam("firstBool", false);
testStore.setParam("secondBool", true);
EXPECT_FALSE(testStore.getParam("firstBool"));
EXPECT_TRUE(testStore.getParam("secondBool"));
- // verify that we can update them
+ // Verify that we can update paramaters.
testStore.setParam("firstBool", true);
testStore.setParam("secondBool", false);
EXPECT_TRUE(testStore.getParam("firstBool"));
EXPECT_FALSE(testStore.getParam("secondBool"));
- // verify that we can delete one
+ // Verify that we can delete a parameter and it will no longer be found.
testStore.delParam("firstBool");
ASSERT_THROW(testStore.getParam("firstBool"), isc::dhcp::DhcpConfigError);
- // verify that the delete was safe
+ // Verify that the delete was safe and the store still operates.
EXPECT_FALSE(testStore.getParam("secondBool"));
- // verify that we can empty the list
+ // Verify that looking for a parameter that never existed throws.
+ ASSERT_THROW(testStore.getParam("bogusBool"), isc::dhcp::DhcpConfigError);
+
+ // Verify that attempting to delete a parameter that never existed does not throw.
+ ASSERT_NO_THROW(testStore.delParam("bogusBool"));
+
+ // Verify that we can empty the list.
testStore.clear();
ASSERT_THROW(testStore.getParam("secondBool"), isc::dhcp::DhcpConfigError);
+
}
// This test verifies that Uint32Storage functions properly.
TEST(ValueStorageTest, Uint32Testing) {
Uint32Storage testStore;
- uint32_t intOne = -77;
+ uint32_t intOne = 77;
uint32_t intTwo = 33;
- // verify that we can add and retrieve them
+ // Verify that we can add and retrieve parameters.
testStore.setParam("firstInt", intOne);
testStore.setParam("secondInt", intTwo);
EXPECT_EQ(testStore.getParam("firstInt"), intOne);
EXPECT_EQ(testStore.getParam("secondInt"), intTwo);
- // verify that we can update them
+ // Verify that we can update parameters.
testStore.setParam("firstInt", --intOne);
testStore.setParam("secondInt", ++intTwo);
EXPECT_EQ(testStore.getParam("firstInt"), intOne);
EXPECT_EQ(testStore.getParam("secondInt"), intTwo);
- // verify that we can delete one
+ // Verify that we can delete a parameter and it will no longer be found.
testStore.delParam("firstInt");
ASSERT_THROW(testStore.getParam("firstInt"), isc::dhcp::DhcpConfigError);
- // verify that the delete was safe
+ // Verify that the delete was safe and the store still operates.
EXPECT_EQ(testStore.getParam("secondInt"), intTwo);
- // verify that we can empty the list
+ // Verify that looking for a parameter that never existed throws.
+ ASSERT_THROW(testStore.getParam("bogusInt"), isc::dhcp::DhcpConfigError);
+
+ // Verify that attempting to delete a parameter that never existed does not throw.
+ ASSERT_NO_THROW(testStore.delParam("bogusInt"));
+
+ // Verify that we can empty the list.
testStore.clear();
ASSERT_THROW(testStore.getParam("secondInt"), isc::dhcp::DhcpConfigError);
}
@@ -107,14 +120,14 @@ TEST(ValueStorageTest, StringTesting) {
std::string stringOne = "seventy-seven";
std::string stringTwo = "thirty-three";
- // verify that we can add and retrieve them
+ // Verify that we can add and retrieve parameters.
testStore.setParam("firstString", stringOne);
testStore.setParam("secondString", stringTwo);
EXPECT_EQ(testStore.getParam("firstString"), stringOne);
EXPECT_EQ(testStore.getParam("secondString"), stringTwo);
- // verify that we can update them
+ // Verify that we can update parameters.
stringOne.append("-boo");
stringTwo.append("-boo");
@@ -124,14 +137,20 @@ TEST(ValueStorageTest, StringTesting) {
EXPECT_EQ(testStore.getParam("firstString"), stringOne);
EXPECT_EQ(testStore.getParam("secondString"), stringTwo);
- // verify that we can delete one
+ // Verify that we can delete a parameter and it will no longer be found.
testStore.delParam("firstString");
ASSERT_THROW(testStore.getParam("firstString"), isc::dhcp::DhcpConfigError);
- // verify that the delete was safe
+ // Verify that the delete was safe and the store still operates.
EXPECT_EQ(testStore.getParam("secondString"), stringTwo);
- // verify that we can empty the list
+ // Verify that looking for a parameter that never existed throws.
+ ASSERT_THROW(testStore.getParam("bogusString"), isc::dhcp::DhcpConfigError);
+
+ // Verify that attempting to delete a parameter that never existed does not throw.
+ ASSERT_NO_THROW(testStore.delParam("bogusString"));
+
+ // Verify that we can empty the list.
testStore.clear();
ASSERT_THROW(testStore.getParam("secondString"), isc::dhcp::DhcpConfigError);
}
More information about the bind10-changes
mailing list