BIND 10 trac3292, updated. 4775ad43ab21203bc4802c8aac50e85979b97193 [3292] Addressed review comments.
BIND 10 source code commits
bind10-changes at lists.isc.org
Tue Jan 21 10:57:46 UTC 2014
The branch, trac3292 has been updated
via 4775ad43ab21203bc4802c8aac50e85979b97193 (commit)
from 3cb70f65383ac942c9e2b2d5d0478698d8732d67 (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 4775ad43ab21203bc4802c8aac50e85979b97193
Author: Marcin Siodelski <marcin at isc.org>
Date: Tue Jan 21 11:54:52 2014 +0100
[3292] Addressed review comments.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp4/tests/config_parser_unittest.cc | 2 +-
src/bin/dhcp6/tests/config_parser_unittest.cc | 2 +-
src/lib/dhcp/option_definition.cc | 80 +++++++++++-----------
src/lib/dhcp/option_definition.h | 33 +++++++--
src/lib/dhcp/tests/option_definition_unittest.cc | 12 +---
5 files changed, 70 insertions(+), 59 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 167214a..1af66c5 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -1857,7 +1857,7 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
testOption(*range.first, 23, foo2_expected, sizeof(foo2_expected));
}
-// The goal of this test is to check that the option carrying a bolean
+// The goal of this test is to check that the option carrying a boolean
// value can be configured using one of the values: "true", "false", "0"
// or "1".
TEST_F(Dhcp4ParserTest, optionDataBoolean) {
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 0a47b43..b07b138 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -2198,7 +2198,7 @@ TEST_F(Dhcp6ParserTest, optionDataInMultipleSubnets) {
sizeof(user_class_expected));
}
-// The goal of this test is to check that the option carrying a bolean
+// The goal of this test is to check that the option carrying a boolean
// value can be configured using one of the values: "true", "false", "0"
// or "1".
TEST_F(Dhcp6ParserTest, optionDataBoolean) {
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index adbcdb1..82a6194 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -399,33 +399,47 @@ OptionDefinition::haveVendor6Format() const {
return (getType() == OPT_UINT32_TYPE && !getEncapsulatedSpace().empty());
}
+bool
+OptionDefinition::convertToBool(const std::string& value_str) const {
+ // Case insensitve check that the input is one of: "true" or "false".
+ if (boost::iequals(value_str, "true")) {
+ return (true);
+
+ } else if (boost::iequals(value_str, "false")) {
+ return (false);
+
+ }
+
+ // The input string is neither "true" nor "false", so let's check
+ // if it is not an integer wrapped in a string.
+ int result;
+ try {
+ result = boost::lexical_cast<int>(value_str);
+
+ } catch (const boost::bad_lexical_cast&) {
+ isc_throw(BadDataTypeCast, "unable to covert the value '"
+ << value_str << "' to boolean data type");
+ }
+ // The boolean value is encoded in DHCP option as 0 or 1. Therefore,
+ // we only allow a user to specify those values for options which
+ // have boolean fields.
+ if (result != 1 && result != 0) {
+ isc_throw(BadDataTypeCast, "unable to convert '" << value_str
+ << "' to boolean data type");
+ }
+ return (static_cast<bool>(result));
+}
+
template<typename T>
T
OptionDefinition::lexicalCastWithRangeCheck(const std::string& value_str)
const {
- // Lexical cast in case of our data types make sense only
- // for uintX_t, intX_t and bool type.
- if (!OptionDataTypeTraits<T>::integer_type &&
- OptionDataTypeTraits<T>::type != OPT_BOOLEAN_TYPE) {
+ // The lexical cast should be attempted when converting to an integer
+ // value only.
+ if (!OptionDataTypeTraits<T>::integer_type) {
isc_throw(BadDataTypeCast,
- "unable to do lexical cast to non-integer and"
- << " non-boolean data type");
- }
-
- // The lexical cast will not handle conversion of the string holding
- // "true" or "false". Therefore we will do the conversion on our own.
- // If the string value doesn't match any of "true" or "false", it
- // is possible that "0" or "1" has been specified, which we are
- // ok with. For conversion of "0" or "1" we will try lexical cast
- // below.
- if (OptionDataTypeTraits<T>::type == OPT_BOOLEAN_TYPE) {
- if (value_str == "true") {
- return (true);
-
- } else if (value_str == "false") {
- return (false);
-
- }
+ "must not convert '" << value_str
+ << "' to non-integer data type");
}
// We use the 64-bit value here because it has wider range than
@@ -433,22 +447,15 @@ OptionDefinition::lexicalCastWithRangeCheck(const std::string& value_str)
// bounds conditions e.g. negative value specified for uintX_t
// data type. Obviously if the value exceeds the limits of int64
// this function will not handle that properly.
- // Note that with the conversion below we also handle boolean
- // values specified as "0" or "1".
int64_t result = 0;
try {
result = boost::lexical_cast<int64_t>(value_str);
+
} catch (const boost::bad_lexical_cast&) {
- // Prepare error message here.
- std::string data_type_str = "boolean";
- if (OptionDataTypeTraits<T>::integer_type) {
- data_type_str = "integer";
- }
isc_throw(BadDataTypeCast, "unable to convert the value '"
- << value_str << "' to " << data_type_str
- << " data type");
+ << value_str << "' to integer data type");
}
- // Perform range checks for integer values only (exclude bool values).
+ // Perform range checks.
if (OptionDataTypeTraits<T>::integer_type) {
if (result > numeric_limits<T>::max() ||
result < numeric_limits<T>::min()) {
@@ -458,12 +465,6 @@ OptionDefinition::lexicalCastWithRangeCheck(const std::string& value_str)
<< numeric_limits<T>::min()
<< ".." << numeric_limits<T>::max());
}
- // The specified value is of the boolean type and we are checking
- // that it has successfuly converted to 0 or 1. Any other numeric
- // value is not accepted to represent boolean value.
- } else if (result != 1 && result != 0) {
- isc_throw(BadDataTypeCast, "unable to convert '" << value_str
- << "' to boolean data type");
}
return (static_cast<T>(result));
}
@@ -484,8 +485,7 @@ OptionDefinition::writeToBuffer(const std::string& value,
// That way we actually waste 7 bits but it seems to be the
// simpler way to encode boolean.
// @todo Consider if any other encode methods can be used.
- OptionDataTypeUtil::writeBool(lexicalCastWithRangeCheck<bool>(value),
- buf);
+ OptionDataTypeUtil::writeBool(convertToBool(value), buf);
return;
case OPT_INT8_TYPE:
OptionDataTypeUtil::writeInt<uint8_t>
diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index 25ce5e2..73c0cdf 100644
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -573,19 +573,38 @@ private:
return (type == type_);
}
+ /// @brief Converts a string value to a boolean value.
+ ///
+ /// This function converts the value represented as string to a boolean
+ /// value. The following conversions are acceptable:
+ /// - "true" => true
+ /// - "false" => false
+ /// - "1" => true
+ /// - "0" => false
+ /// The first two conversions are case insensitive, so as conversions from
+ /// strings such as "TRUE", "trUE" etc. will be accepted. Note that the
+ /// only acceptable integer values, carried as strings are: "0" and "1".
+ /// For other values, e.g. "2", "3" etc. an exception will be thrown
+ /// during conversion.
+ ///
+ /// @param value_str Input value.
+ ///
+ /// @return boolean representation of the string specified as the parameter.
+ /// @throw isc::dhcp::BadDataTypeCast if failed to perform the conversion.
+ bool convertToBool(const std::string& value_str) const;
+
/// @brief Perform lexical cast of the value and validate its range.
///
/// This function performs lexical cast of a string value to integer
- /// or boolean value and checks if the resulting value is within a
- /// range of a target type. Note that range checks are not performed
- /// on boolean values. The target type should be one of the supported
- /// integer types or bool.
+ /// value and checks if the resulting value is within a range of a
+ /// target type. The target type should be one of the supported
+ /// integer types.
///
/// @param value_str input value given as string.
- /// @tparam T target type for lexical cast.
+ /// @tparam T target integer type for lexical cast.
///
- /// @return cast value.
- /// @throw BadDataTypeCast if cast was not successful.
+ /// @return Integer value after conversion from the string.
+ /// @throw isc::dhcp::BadDataTypeCast if conversion was not successful.
template<typename T>
T lexicalCastWithRangeCheck(const std::string& value_str) const;
diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index 3a0819a..1ba68a8 100644
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -693,8 +693,7 @@ TEST_F(OptionDefinitionTest, boolValue) {
// Try to provide zero-length buffer. Expect exception.
EXPECT_THROW(
- option_v4 = opt_def.optionFactory(Option::V6, DHO_IP_FORWARDING,
- OptionBuffer()),
+ opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING, OptionBuffer()),
InvalidOptionValue
);
@@ -711,10 +710,8 @@ TEST_F(OptionDefinitionTest, boolTokenized) {
OptionPtr option_v4;
std::vector<std::string> values;
- // Provide two boolean values. It is expected that only the first
- // one will be used.
+ // Specify a value for the option instance being created.
values.push_back("true");
- values.push_back("false");
ASSERT_NO_THROW(
option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
values);
@@ -727,7 +724,6 @@ TEST_F(OptionDefinitionTest, boolTokenized) {
// Repeat the test but for "false" value this time.
values[0] = "false";
- values[1] = "true";
ASSERT_NO_THROW(
option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
values);
@@ -739,7 +735,6 @@ TEST_F(OptionDefinitionTest, boolTokenized) {
// Check if that will work for numeric values.
values[0] = "0";
- values[1] = "1";
ASSERT_NO_THROW(
option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
values);
@@ -751,7 +746,6 @@ TEST_F(OptionDefinitionTest, boolTokenized) {
// Swap numeric values and test if it works for "true" case.
values[0] = "1";
- values[1] = "0";
ASSERT_NO_THROW(
option_v4 = opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING,
values);
@@ -764,14 +758,12 @@ TEST_F(OptionDefinitionTest, boolTokenized) {
// A conversion of non-numeric value to boolean should fail if
// this value is neither "true" nor "false".
values[0] = "garbage";
- values[1] = "false";
EXPECT_THROW(opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING, values),
isc::dhcp::BadDataTypeCast);
// A conversion of numeric value to boolean should fail if this value
// is neither "0" nor "1".
values[0] = "2";
- values[1] = "0";
EXPECT_THROW(opt_def.optionFactory(Option::V4, DHO_IP_FORWARDING, values),
isc::dhcp::BadDataTypeCast);
More information about the bind10-changes
mailing list