BIND 10 trac2786, updated. 85b0a75a0499f2a7a08d6f597e3f90ee4f7be636 [2786] Option factory function uses iterators to create OptionCustom.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed May 15 19:31:21 UTC 2013
The branch, trac2786 has been updated
via 85b0a75a0499f2a7a08d6f597e3f90ee4f7be636 (commit)
via 0f3dd4e12dc74c1c5985e9b701b59fe2355af931 (commit)
from b3c562f2bd85fa9f825d59f8601ba99e04e661f3 (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 85b0a75a0499f2a7a08d6f597e3f90ee4f7be636
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed May 15 21:31:07 2013 +0200
[2786] Option factory function uses iterators to create OptionCustom.
commit 0f3dd4e12dc74c1c5985e9b701b59fe2355af931
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed May 15 14:07:48 2013 +0200
[2786] Address review comments.
-----------------------------------------------------------------------
Summary of changes:
src/bin/dhcp6/dhcp6_srv.cc | 7 +++---
src/lib/dhcp/option_definition.cc | 2 +-
src/lib/dhcp/option_string.cc | 34 ++++++++++++++++++--------
src/lib/dhcp/option_string.h | 20 +++++----------
src/lib/dhcp/tests/libdhcp++_unittest.cc | 20 +++++++--------
src/lib/dhcp/tests/option_string_unittest.cc | 6 +++++
src/lib/dhcp/tests/pkt4_unittest.cc | 4 +--
7 files changed, 52 insertions(+), 41 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc
index 86f4d8f..4f5133c 100644
--- a/src/bin/dhcp6/dhcp6_srv.cc
+++ b/src/bin/dhcp6/dhcp6_srv.cc
@@ -463,10 +463,9 @@ Dhcpv6Srv::createStatusCode(uint16_t code, const std::string& text) {
assert(status_code_def);
// As there is no dedicated class to represent Status Code
- // the OptionCustom class should be returned here.
- boost::shared_ptr<OptionCustom> option_status =
- boost::dynamic_pointer_cast<
- OptionCustom>(status_code_def->optionFactory(Option::V6, D6O_STATUS_CODE));
+ // the OptionCustom class is used here instead.
+ OptionCustomPtr option_status =
+ OptionCustomPtr(new OptionCustom(*status_code_def, Option::V6));
assert(option_status);
// Set status code to 'code' (0 - means data field #0).
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index e165f77..9db99f4 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -186,7 +186,7 @@ OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
}
}
}
- return (OptionPtr(new OptionCustom(*this, u, OptionBuffer(begin, end))));
+ return (OptionPtr(new OptionCustom(*this, u, begin, end)));
} catch (const Exception& ex) {
isc_throw(InvalidOptionValue, ex.what());
diff --git a/src/lib/dhcp/option_string.cc b/src/lib/dhcp/option_string.cc
index 9c3eae3..ac1e4b5 100644
--- a/src/lib/dhcp/option_string.cc
+++ b/src/lib/dhcp/option_string.cc
@@ -19,7 +19,10 @@ namespace dhcp {
OptionString::OptionString(const Option::Universe u, const uint16_t type,
const std::string& value)
- : Option(u, type), value_(value) {
+ : Option(u, type) {
+ // Try to assign the provided string value. This will throw exception
+ // if the provided value is empty.
+ setValue(value);
}
OptionString::OptionString(const Option::Universe u, const uint16_t type,
@@ -31,25 +34,36 @@ OptionString::OptionString(const Option::Universe u, const uint16_t type,
unpack(begin, end);
}
-uint16_t
-OptionString::len() {
- return (getHeaderLen() + value_.size());
+std::string
+OptionString::getValue() const {
+ return std::string(data_.begin(), data_.end());
}
void
-OptionString::pack(isc::util::OutputBuffer& buf) {
+OptionString::setValue(const std::string& value) {
// Sanity check that the string value is at least one byte long.
// This is a requirement for all currently defined options which
// carry a string value.
- if (value_.empty()) {
- isc_throw(isc::OutOfRange, "string value carried in the option"
- << " must not be empty");
+ if (value.empty()) {
+ isc_throw(isc::OutOfRange, "string value carried by the option '"
+ << getType() << "' must not be empty");
}
+ data_.assign(value.begin(), value.end());
+}
+
+
+uint16_t
+OptionString::len() {
+ return (getHeaderLen() + data_.size());
+}
+
+void
+OptionString::pack(isc::util::OutputBuffer& buf) {
// Pack option header.
packHeader(buf);
// Pack data.
- buf.writeData(value_.c_str(), value_.size());
+ buf.writeData(&data_[0], data_.size());
// That's it. We don't pack any sub-options here, because this option
// must not contain sub-options.
@@ -62,7 +76,7 @@ OptionString::unpack(OptionBufferConstIter begin,
isc_throw(isc::OutOfRange, "failed to parse an option holding string value"
<< " - empty value is not accepted");
}
- value_.assign(begin, end);
+ data_.assign(begin, end);
}
} // end of isc::dhcp namespace
diff --git a/src/lib/dhcp/option_string.h b/src/lib/dhcp/option_string.h
index ca72769..3d0aa9a 100644
--- a/src/lib/dhcp/option_string.h
+++ b/src/lib/dhcp/option_string.h
@@ -72,14 +72,14 @@ public:
/// @brief Returns the string value held by the option.
///
/// @return string value held by the option.
- std::string getValue() const {
- return (value_);
- }
+ std::string getValue() const;
/// @brief Sets the string value to be held by the option.
- void setValue(const std::string& value) {
- value_ = value;
- }
+ ///
+ /// @param value string value to be set.
+ ///
+ /// @throw isc::OutOfRange if a string value to be set is empty.
+ void setValue(const std::string& value);
/// @brief Creates on-wire format of the option.
///
@@ -103,14 +103,6 @@ public:
/// @throw isc::OutOfRange if provided buffer is truncated.
virtual void unpack(OptionBufferConstIter begin, OptionBufferConstIter end);
-private:
- /// String value being held by the option.
- std::string value_;
-
- // Change scope of the getData function to private as we want
- // getValue is called instead.
- using Option::getData;
-
};
/// Pointer to the OptionString object.
diff --git a/src/lib/dhcp/tests/libdhcp++_unittest.cc b/src/lib/dhcp/tests/libdhcp++_unittest.cc
index 511cd7e..d523158 100644
--- a/src/lib/dhcp/tests/libdhcp++_unittest.cc
+++ b/src/lib/dhcp/tests/libdhcp++_unittest.cc
@@ -373,7 +373,7 @@ TEST_F(LibDhcpTest, unpackOptions6) {
/// is no restriction on the data length being carried by them.
/// For simplicity, we assign data of the length 3 for each
/// of them.
-static uint8_t v4Opts[] = {
+static uint8_t v4_opts[] = {
12, 3, 0, 1, 2, // Hostname
60, 3, 10, 11, 12, // Class Id
14, 3, 20, 21, 22, // Merit Dump File
@@ -404,18 +404,18 @@ TEST_F(LibDhcpTest, packOptions4) {
opts.insert(make_pair(opt1->getType(), opt4));
opts.insert(make_pair(opt1->getType(), opt5));
- vector<uint8_t> expVect(v4Opts, v4Opts + sizeof(v4Opts));
+ vector<uint8_t> expVect(v4_opts, v4_opts + sizeof(v4_opts));
OutputBuffer buf(100);
EXPECT_NO_THROW(LibDHCP::packOptions(buf, opts));
- ASSERT_EQ(buf.getLength(), sizeof(v4Opts));
- EXPECT_EQ(0, memcmp(v4Opts, buf.getData(), sizeof(v4Opts)));
+ ASSERT_EQ(buf.getLength(), sizeof(v4_opts));
+ EXPECT_EQ(0, memcmp(v4_opts, buf.getData(), sizeof(v4_opts)));
}
TEST_F(LibDhcpTest, unpackOptions4) {
- vector<uint8_t> v4packed(v4Opts, v4Opts + sizeof(v4Opts));
+ vector<uint8_t> v4packed(v4_opts, v4_opts + sizeof(v4_opts));
isc::dhcp::Option::OptionCollection options; // list of options
ASSERT_NO_THROW(
@@ -430,14 +430,14 @@ TEST_F(LibDhcpTest, unpackOptions4) {
EXPECT_EQ(12, option12->getType()); // this should be option 12
ASSERT_EQ(3, option12->getValue().length()); // it should be of length 3
EXPECT_EQ(5, option12->len()); // total option length 5
- EXPECT_EQ(0, memcmp(&option12->getValue()[0], v4Opts+2, 3)); // data len=3
+ EXPECT_EQ(0, memcmp(&option12->getValue()[0], v4_opts + 2, 3)); // data len=3
x = options.find(60);
ASSERT_FALSE(x == options.end()); // option 2 should exist
EXPECT_EQ(60, x->second->getType()); // this should be option 60
ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
EXPECT_EQ(5, x->second->len()); // total option length 5
- EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+7, 3)); // data len=3
+ EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 7, 3)); // data len=3
x = options.find(14);
ASSERT_FALSE(x == options.end()); // option 3 should exist
@@ -446,21 +446,21 @@ TEST_F(LibDhcpTest, unpackOptions4) {
EXPECT_EQ(14, option14->getType()); // this should be option 14
ASSERT_EQ(3, option14->getValue().length()); // it should be of length 3
EXPECT_EQ(5, option14->len()); // total option length 5
- EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4Opts+12, 3)); // data len=3
+ EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4_opts + 12, 3)); // data len=3
x = options.find(254);
ASSERT_FALSE(x == options.end()); // option 3 should exist
EXPECT_EQ(254, x->second->getType()); // this should be option 254
ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
EXPECT_EQ(5, x->second->len()); // total option length 5
- EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+17, 3)); // data len=3
+ EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 17, 3)); // data len=3
x = options.find(128);
ASSERT_FALSE(x == options.end()); // option 3 should exist
EXPECT_EQ(128, x->second->getType()); // this should be option 254
ASSERT_EQ(3, x->second->getData().size()); // it should be of length 3
EXPECT_EQ(5, x->second->len()); // total option length 5
- EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4Opts+22, 3)); // data len=3
+ EXPECT_EQ(0, memcmp(&x->second->getData()[0], v4_opts + 22, 3)); // data len=3
x = options.find(0);
EXPECT_TRUE(x == options.end()); // option 0 not found
diff --git a/src/lib/dhcp/tests/option_string_unittest.cc b/src/lib/dhcp/tests/option_string_unittest.cc
index 24c5992..0be3c3d 100644
--- a/src/lib/dhcp/tests/option_string_unittest.cc
+++ b/src/lib/dhcp/tests/option_string_unittest.cc
@@ -59,6 +59,10 @@ TEST_F(OptionStringTest, constructorFromString) {
EXPECT_EQ(234, optv6.getType());
EXPECT_EQ("other option", optv6.getValue());
EXPECT_EQ(Option::OPTION6_HDR_LEN + optv6_value.size(), optv6.len());
+
+ // Check that an attempt to use empty string in the constructor
+ // will result in an exception.
+ EXPECT_THROW(OptionString(Option::V6, 123, ""), isc::OutOfRange);
}
// This test verifies that the constructor which creates an option instance
@@ -119,6 +123,8 @@ TEST_F(OptionStringTest, setValue) {
// been successful.
EXPECT_NO_THROW(optv4.setValue("new option value"));
EXPECT_EQ("new option value", optv4.getValue());
+ // Try to set to an empty string. It should throw exception.
+ EXPECT_THROW(optv4.setValue(""), isc::OutOfRange);
}
// This test verifies that the pack function encodes the option in
diff --git a/src/lib/dhcp/tests/pkt4_unittest.cc b/src/lib/dhcp/tests/pkt4_unittest.cc
index 67145e9..e9c7dd2 100644
--- a/src/lib/dhcp/tests/pkt4_unittest.cc
+++ b/src/lib/dhcp/tests/pkt4_unittest.cc
@@ -560,12 +560,12 @@ TEST(Pkt4Test, unpackOptions) {
EXPECT_EQ(0, memcmp(&option12->getValue()[0], v4Opts + 2, 3)); // data len=3
x = pkt->getOption(14);
- ASSERT_TRUE(x); // option 13 should exist
+ ASSERT_TRUE(x); // option 14 should exist
// Option 14 is represented by the OptionString class so let's do
// the appropriate conversion.
OptionStringPtr option14 = boost::static_pointer_cast<OptionString>(x);
ASSERT_TRUE(option14);
- EXPECT_EQ(14, option14->getType()); // this should be option 13
+ EXPECT_EQ(14, option14->getType()); // this should be option 14
ASSERT_EQ(3, option14->getValue().length()); // it should be of length 3
EXPECT_EQ(5, option14->len()); // total option length 5
EXPECT_EQ(0, memcmp(&option14->getValue()[0], v4Opts + 7, 3)); // data len=3
More information about the bind10-changes
mailing list