BIND 10 trac2316, updated. 9bb3d27efdf09dd2e429086767cdee275f3df115 [2316] Changes according to code review.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Oct 17 19:32:35 UTC 2012
The branch, trac2316 has been updated
via 9bb3d27efdf09dd2e429086767cdee275f3df115 (commit)
from 82c0737d3d63da1a1c2b001d0306a32a1e55f5f5 (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 9bb3d27efdf09dd2e429086767cdee275f3df115
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Oct 17 21:29:08 2012 +0200
[2316] Changes according to code review.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/subnet.cc | 10 +++----
src/lib/dhcp/subnet.h | 48 +++++++++++++++++++++++----------
src/lib/dhcp/tests/subnet_unittest.cc | 21 +++++++++------
3 files changed, 52 insertions(+), 27 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/subnet.cc b/src/lib/dhcp/subnet.cc
index ada274b..9853884 100644
--- a/src/lib/dhcp/subnet.cc
+++ b/src/lib/dhcp/subnet.cc
@@ -42,7 +42,7 @@ bool Subnet::inRange(const isc::asiolink::IOAddress& addr) const {
}
void
-Subnet::addOption(OptionPtr option, bool persistent /* = false */) {
+Subnet::addOption(OptionPtr& option, bool persistent /* = false */) {
validateOption(option);
options_.push_back(OptionDescriptor(option, persistent));
}
@@ -97,11 +97,11 @@ Pool4Ptr Subnet4::getPool4(const isc::asiolink::IOAddress& hint /* = IOAddress("
}
void
-Subnet4::validateOption(OptionPtr option) {
+Subnet4::validateOption(OptionPtr option) const {
if (!option) {
isc_throw(isc::BadValue, "option configured for subnet must not be NULL");
} else if (option->getUniverse() != Option::V4) {
- isc_throw(isc::BadValue, "epected V4 option to be added to the subnet");
+ isc_throw(isc::BadValue, "expected V4 option to be added to the subnet");
}
}
@@ -152,11 +152,11 @@ Pool6Ptr Subnet6::getPool6(const isc::asiolink::IOAddress& hint /* = IOAddress("
}
void
-Subnet6::validateOption(OptionPtr option) {
+Subnet6::validateOption(OptionPtr option) const {
if (!option) {
isc_throw(isc::BadValue, "option configured for subnet must not be NULL");
} else if (option->getUniverse() != Option::V6) {
- isc_throw(isc::BadValue, "epected V6 option to be added to the subnet");
+ isc_throw(isc::BadValue, "expected V6 option to be added to the subnet");
}
}
} // end of isc::dhcp namespace
diff --git a/src/lib/dhcp/subnet.h b/src/lib/dhcp/subnet.h
index d6a1ccb..21708c5 100644
--- a/src/lib/dhcp/subnet.h
+++ b/src/lib/dhcp/subnet.h
@@ -58,30 +58,47 @@ public:
/// @brief Constructor.
///
- /// @opt option
- /// persist if true option is always sent.
+ /// @param opt option
+ /// @param persist if true option is always sent.
OptionDescriptor(OptionPtr& opt, bool persist)
: option(opt), persistent(persist) {};
};
/// @brief Extractor class to extract key with another key.
///
- /// This class extracts one multi_index_container key with
- /// another key. Within Subnet class it is used to access
- /// data inside the OptionDescriptor::option object and use
- /// this data as a index key. The standard key extractor such
- /// as mem_fun is not sufficient because it can't operate on
- /// objects wrapped with structure.
+ /// This class solves the problem of accessing index key values
+ /// that are stored in objects nested in other objects.
+ /// Each OptionDescriptor structure contains the OptionPtr object.
+ /// The value retured by one of its accessors (getType) is used
+ /// as an indexing value in the multi_index_container defined below.
+ /// There is no easy way to mark that value returned by Option::getType
+ /// should be an index of this multi_index_container. There are standard
+ /// key extractors such as 'member' or 'mem_fun' but they are not
+ /// sufficient here. The former can be used to mark that member of
+ /// the structure that is held in the container should be used as an
+ /// indexing value. The latter can be used if the indexing value is
+ /// a product of the class being held in the container. In this complex
+ /// scenario when the indexing value is a product of the function that
+ /// is wrapped by the structure, this new extractor template has to be
+ /// defined. The template class provides a 'chain' of two extractors
+ /// to access the value returned by nested object and to use it as
+ /// indexing value.
+ /// For some more examples of complex keys see:
+ /// http://www.cs.brown.edu/~jwicks/boost/libs/multi_index/doc/index.html
///
/// @tparam KeyExtractor1 extractor used to access data in
/// OptionDescriptor::option
/// @tparam KeyExtractor2 extractor used to access
/// OptionDescriptor::option member.
template<typename KeyExtractor1, typename KeyExtractor2>
- struct KeyFromKey {
+ class KeyFromKey {
+ public:
typedef typename KeyExtractor1::result_type result_type;
/// @brief Constructor.
+ ///
+ /// @param key1 first key extractor
+ /// @param key2 second key extractor
KeyFromKey(const KeyExtractor1 key1 = KeyExtractor1(),
const KeyExtractor2 key2 = KeyExtractor2())
: key1_(key1), key2_(key2) { };
@@ -134,6 +151,7 @@ public:
boost::multi_index::indexed_by<
// Sequenced index allows accessing elements in the same way
// as elements in std::list.
+ // Sequenced is an index #0.
boost::multi_index::sequenced<>,
// Start definition of index #1.
boost::multi_index::hashed_non_unique<
@@ -185,7 +203,7 @@ public:
/// requested it or not.
///
/// @throw isc::BadValue if invalid option provided.
- void addOption(OptionPtr option, bool persistent = false);
+ void addOption(OptionPtr& option, bool persistent = false);
/// @brief Delete all options configured for the subnet.
void delOptions();
@@ -207,7 +225,9 @@ public:
/// @brief Return a collection of options.
///
- /// @return collection of options configured for a subnet.
+ /// @return reference to collection of options configured for a subnet.
+ /// The returned reference is valid as long as the Subnet object which
+ /// returned it still exists.
const OptionContainer& getOptions() {
return (options_);
}
@@ -239,7 +259,7 @@ protected:
/// @brief Check if option is valid and can be added to a subnet.
///
/// @param option option to be validated.
- virtual void validateOption(OptionPtr option) = 0;
+ virtual void validateOption(OptionPtr option) const = 0;
/// @brief subnet-id
///
@@ -312,7 +332,7 @@ protected:
/// @param option option to be validated.
///
/// @throw isc::BadValue if provided option is invalid.
- virtual void validateOption(OptionPtr option);
+ virtual void validateOption(OptionPtr option) const;
/// @brief collection of pools in that list
Pool4Collection pools_;
@@ -380,7 +400,7 @@ protected:
/// @param option option to be validated.
///
/// @throw isc::BadValue if provided option is invalid.
- virtual void validateOption(OptionPtr option);
+ virtual void validateOption(OptionPtr option) const;
/// @brief collection of pools in that list
Pool6Collection pools_;
diff --git a/src/lib/dhcp/tests/subnet_unittest.cc b/src/lib/dhcp/tests/subnet_unittest.cc
index e5f2a3e..0e2e846 100644
--- a/src/lib/dhcp/tests/subnet_unittest.cc
+++ b/src/lib/dhcp/tests/subnet_unittest.cc
@@ -310,10 +310,15 @@ TEST(Subnet6Test, addPersistentOption) {
// Add 10 options to the subnet with option codes 100 - 109.
for (uint16_t code = 100; code < 110; ++code) {
OptionPtr option(new Option(Option::V6, code, OptionBuffer(10, 0xFF)));
- // Options with even codes will be flagged persistent. Persistent
- // options are those that server sends to clients regardless if
- // they ask for them or not.
- bool persistent = (code % 2) ? true : false;
+ // We create 10 options and want some of them to be flagged
+ // persistent and some non-persistent. Persistent options are
+ // those that server sends to clients regardless if they ask
+ // for them or not. We pick 3 out of 10 options and mark them
+ // non-persistent and 7 other options persistent.
+ // Code values: 102, 105 and 108 are divisable by 3
+ // and options with these codes will be flagged non-persistent.
+ // Options with other codes will be flagged persistent.
+ bool persistent = (code % 3) ? true : false;
ASSERT_NO_THROW(subnet->addOption(option, persistent));
}
@@ -328,15 +333,15 @@ TEST(Subnet6Test, addPersistentOption) {
std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
Subnet::OptionContainerPersistIndex::const_iterator> range_persistent =
idx.equal_range(true);
- // 5 out of 10 options have been flagged persistent.
- ASSERT_EQ(5, distance(range_persistent.first, range_persistent.second));
+ // 3 out of 10 options have been flagged persistent.
+ ASSERT_EQ(7, distance(range_persistent.first, range_persistent.second));
// Get all non-persistent options.
std::pair<Subnet::OptionContainerPersistIndex::const_iterator,
Subnet::OptionContainerPersistIndex::const_iterator> range_non_persistent =
idx.equal_range(false);
- // 5 out of 10 options have been flagged persistent.
- ASSERT_EQ(5, distance(range_non_persistent.first, range_non_persistent.second));
+ // 7 out of 10 options have been flagged persistent.
+ ASSERT_EQ(3, distance(range_non_persistent.first, range_non_persistent.second));
subnet->delOptions();
More information about the bind10-changes
mailing list