BIND 10 trac2304, updated. 85627bca2677b4e68d67c8ecd35fbfe409eda251 [2304] Code cleanup.
BIND 10 source code commits
bind10-changes at lists.isc.org
Wed Oct 10 15:57:13 UTC 2012
The branch, trac2304 has been updated
via 85627bca2677b4e68d67c8ecd35fbfe409eda251 (commit)
from cd86c2c845e03de172d832256ea3542203f40a5f (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 85627bca2677b4e68d67c8ecd35fbfe409eda251
Author: Marcin Siodelski <marcin at isc.org>
Date: Wed Oct 10 17:21:01 2012 +0200
[2304] Code cleanup.
-----------------------------------------------------------------------
Summary of changes:
src/lib/dhcp/option6_int.h | 9 ++--
src/lib/dhcp/option6_int_array.h | 11 +++--
src/lib/dhcp/option_data_types.h | 15 +++++-
src/lib/dhcp/option_definition.cc | 34 ++++++++++----
src/lib/dhcp/option_definition.h | 52 ++++++++++++++-------
src/lib/dhcp/tests/option6_int_array_unittest.cc | 3 --
src/lib/dhcp/tests/option6_int_unittest.cc | 3 --
src/lib/dhcp/tests/option_definition_unittest.cc | 54 ++++++++++++++++------
8 files changed, 125 insertions(+), 56 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dhcp/option6_int.h b/src/lib/dhcp/option6_int.h
index e0df4c5..3401fac 100644
--- a/src/lib/dhcp/option6_int.h
+++ b/src/lib/dhcp/option6_int.h
@@ -15,12 +15,13 @@
#ifndef OPTION6_INT_H_
#define OPTION6_INT_H_
+#include <dhcp/libdhcp++.h>
+#include <dhcp/option.h>
+#include <dhcp/option_data_types.h>
+#include <util/io_utilities.h>
+
#include <stdint.h>
#include <limits>
-#include <util/io_utilities.h>
-#include "dhcp/libdhcp++.h"
-#include "dhcp/option.h"
-#include "dhcp/option_data_types.h"
namespace isc {
namespace dhcp {
diff --git a/src/lib/dhcp/option6_int_array.h b/src/lib/dhcp/option6_int_array.h
index b87f538..2447dac 100644
--- a/src/lib/dhcp/option6_int_array.h
+++ b/src/lib/dhcp/option6_int_array.h
@@ -15,12 +15,13 @@
#ifndef OPTION6_INT_ARRAY_H_
#define OPTION6_INT_ARRAY_H_
+#include <dhcp/libdhcp++.h>
+#include <dhcp/option.h>
+#include <dhcp/option_data_types.h>
+#include <util/io_utilities.h>
+
#include <stdint.h>
#include <limits>
-#include <util/io_utilities.h>
-#include "dhcp/libdhcp++.h"
-#include "dhcp/option.h"
-#include "dhcp/option_data_types.h"
namespace isc {
namespace dhcp {
@@ -149,7 +150,7 @@ public:
/// @brief Set option values.
///
- /// @param collection of values to be set.
+ /// @param values collection of values to be set for option.
void setValues(const std::vector<T>& values) { values_ = values; }
/// @brief returns complete length of option
diff --git a/src/lib/dhcp/option_data_types.h b/src/lib/dhcp/option_data_types.h
index 52be59f..4e8d8a6 100644
--- a/src/lib/dhcp/option_data_types.h
+++ b/src/lib/dhcp/option_data_types.h
@@ -15,9 +15,10 @@
#ifndef OPTION_DATA_TYPES_H_
#define OPTION_DATA_TYPES_H_
-#include <stdint.h>
#include <exceptions/exceptions.h>
+#include <stdint.h>
+
namespace isc {
namespace dhcp {
@@ -28,43 +29,53 @@ public:
isc::Exception(file, line, what) { };
};
-/// @brief Numeric data types supported by DHCP option defintions.
+/// @brief Trait class for integer data types supported in DHCP option definitions.
+///
+/// This is useful to check whether the type specified as template parameter
+/// is supported by classes like Option6Int, Option6IntArray and some template
+/// factory functions in OptionDefinition class.
template<typename T>
struct OptionDataTypes {
static const bool valid = false;
static const int len = 0;
};
+/// int8_t type is supported.
template<>
struct OptionDataTypes<int8_t> {
static const bool valid = true;
static const int len = 1;
};
+/// int16_t type is supported.
template<>
struct OptionDataTypes<int16_t> {
static const bool valid = true;
static const int len = 2;
};
+/// int32_t type is supported.
template<>
struct OptionDataTypes<int32_t> {
static const bool valid = true;
static const int len = 4;
};
+/// uint8_t type is supported.
template<>
struct OptionDataTypes<uint8_t> {
static const bool valid = true;
static const int len = 1;
};
+/// uint16_t type is supported.
template<>
struct OptionDataTypes<uint16_t> {
static const bool valid = true;
static const int len = 2;
};
+/// uint32_t type is supported.
template<>
struct OptionDataTypes<uint32_t> {
static const bool valid = true;
diff --git a/src/lib/dhcp/option_definition.cc b/src/lib/dhcp/option_definition.cc
index 07305b9..49c5642 100644
--- a/src/lib/dhcp/option_definition.cc
+++ b/src/lib/dhcp/option_definition.cc
@@ -12,14 +12,14 @@
// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
// PERFORMANCE OF THIS SOFTWARE.
-#include "dhcp/dhcp6.h"
-#include "dhcp/option_definition.h"
-#include "dhcp/option4_addrlst.h"
-#include "dhcp/option6_addrlst.h"
-#include "dhcp/option6_ia.h"
-#include "dhcp/option6_iaaddr.h"
-#include "dhcp/option6_int.h"
-#include "dhcp/option6_int_array.h"
+#include <dhcp/dhcp6.h>
+#include <dhcp/option_definition.h>
+#include <dhcp/option4_addrlst.h>
+#include <dhcp/option6_addrlst.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp/option6_iaaddr.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_int_array.h>
using namespace std;
using namespace isc::util;
@@ -97,6 +97,10 @@ OptionDefinition::addRecordField(const DataType data_type) {
Option::Factory*
OptionDefinition::getFactory() const {
+ // @todo This function must be extended to return more factory
+ // functions that create instances of more specialized options.
+ // This requires us to first implement those more specialized
+ // options that will be derived from Option class.
if (type_ == IPV6_ADDRESS_TYPE && array_type_) {
return (factoryAddrList6);
} else if (type_ == IPV4_ADDRESS_TYPE && array_type_) {
@@ -126,6 +130,11 @@ OptionDefinition::getFactory() const {
return (factoryInteger<uint32_t>);
}
}
+ // Factory generic returns instance of Option class. However, once we
+ // implement CustomOption class we may want to return factory function
+ // that will create instance of CustomOption rather than Option.
+ // CustomOption will allow to access particular data fields within the
+ // option rather than raw data buffer.
return (factoryGeneric);
}
@@ -139,6 +148,15 @@ OptionDefinition::sanityCheckUniverse(const Option::Universe expected_universe,
void
OptionDefinition::validate() const {
+ // Option name must not be empty.
+ if (name_.empty()) {
+ isc_throw(isc::BadValue, "option name must not be empty");
+ }
+ // Option name must not contain spaces.
+ if (name_.find(" ") != string::npos) {
+ isc_throw(isc::BadValue, "option name must not contain spaces");
+ }
+ // Unsupported option types are not allowed.
if (type_ >= UNKNOWN_TYPE) {
isc_throw(isc::OutOfRange, "option type value " << type_
<< " is out of range");
diff --git a/src/lib/dhcp/option_definition.h b/src/lib/dhcp/option_definition.h
index 764fe85..ad0598c 100644
--- a/src/lib/dhcp/option_definition.h
+++ b/src/lib/dhcp/option_definition.h
@@ -15,10 +15,10 @@
#ifndef OPTION_DEFINITION_H_
#define OPTION_DEFINITION_H_
-#include "dhcp/option_data_types.h"
-#include "dhcp/option6_int.h"
-#include "dhcp/option6_int_array.h"
-#include "dhcp/option.h"
+#include <dhcp/option_data_types.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_int_array.h>
+#include <dhcp/option.h>
namespace isc {
namespace dhcp {
@@ -51,7 +51,7 @@ namespace dhcp {
/// limited by the maximal DHCPv6 option length).
/// Should option comprise data fields of different types the "record"
/// option type is used. In such cases the data field types within the
-/// record are specified using \ref OptionDefinition::AddRecordField.
+/// record are specified using \ref OptionDefinition::addRecordField.
/// When OptionDefinition object has been sucessfully created it
/// can be queried to return the appropriate option factory function
/// for the specified option format. There is a number of "standard"
@@ -76,6 +76,7 @@ namespace dhcp {
///
/// @todo Extend the comment to describe "generic factories".
/// @todo Extend this class to use custom namespaces.
+/// @todo Extend this class with more factory functions.
class OptionDefinition {
public:
@@ -105,6 +106,13 @@ public:
private:
/// @brief Utility class for operations on DataTypes.
+ ///
+ /// This class is implemented as the singleton because the list of
+ /// supported data types is in this case loaded only once
+ /// into the memory and persists for all option definitions.
+ ///
+ /// @todo This class can be extended to return the string value
+ /// representing the data type from the enum value.
class DataTypeUtil {
public:
@@ -208,6 +216,13 @@ public:
/// @return option data type.
DataType getType() const { return (type_); };
+ /// @brief Check if the option definition is valid.
+ ///
+ /// @throw isc::OutOfRange if invalid option type was specified.
+ /// @throw isc::BadValue if invalid option name was specified,
+ /// e.g. empty or containing spaces.
+ void validate() const;
+
/// @brief Check if specified format is IA_NA option format.
///
/// @return true if specified format is IA_NA option format.
@@ -223,6 +238,9 @@ public:
/// @param u universe (must be V4).
/// @param type option type.
/// @param buf option buffer with a list of IPv4 addresses.
+ ///
+ /// @throw isc::OutOfRange if length of the provided option buffer
+ /// is not multiple of IPV4 address length.
static OptionPtr factoryAddrList4(Option::Universe u, uint16_t type,
const OptionBuffer& buf);
@@ -231,6 +249,9 @@ public:
/// @param u universe (must be V6).
/// @param type option type.
/// @param buf option buffer with a list of IPv6 addresses.
+ ///
+ /// @throw isc::OutOfaRange if length of provided option buffer
+ /// is not multiple of IPV6 address length.
static OptionPtr factoryAddrList6(Option::Universe u, uint16_t type,
const OptionBuffer& buf);
@@ -242,7 +263,7 @@ public:
static OptionPtr factoryEmpty(Option::Universe u, uint16_t type,
const OptionBuffer& buf);
- /// @brief Factory to create generic option..
+ /// @brief Factory to create generic option.
///
/// @param u universe (V6 or V4).
/// @param type option type.
@@ -276,10 +297,11 @@ public:
/// @brief Factory function to create option with integer value.
///
- /// @param u universe (V6 or V4).
/// @param type option type.
/// @param buf option buffer.
- /// @param T type of the data field (must be one of the uintX_t or intX_t).
+ /// @tparam T type of the data field (must be one of the uintX_t or intX_t).
+ ///
+ /// @throw isc::OutOfRange if provided option buffer length is invalid.
template<typename T>
static OptionPtr factoryInteger(Option::Universe, uint16_t type, const OptionBuffer& buf) {
if (buf.size() > sizeof(T)) {
@@ -292,10 +314,11 @@ public:
/// @brief Factory function to create option with array of integer values.
///
- /// @param u universe (V6 or V4).
/// @param type option type.
/// @param buf option buffer.
- /// @param T type of the data field (must be one of the uintX_t or intX_t).
+ /// @tparam T type of the data field (must be one of the uintX_t or intX_t).
+ ///
+ /// @throw isc::OutOfRange if provided option buffer length is invalid.
template<typename T>
static OptionPtr factoryIntegerArray(Option::Universe, uint16_t type, const OptionBuffer& buf) {
if (buf.size() == 0) {
@@ -323,13 +346,8 @@ private:
/// @param actual_universe actual universe value.
///
/// @throw isc::BadValue if expected universe and actual universe don't match.
- static inline void sanityCheckUniverse(const Option::Universe expected_universe,
- const Option::Universe actual_universe);
-
- /// @brief Check if the option definition is valid.
- ///
- /// @todo: list exceptions it throws.
- void validate() const;
+ static inline void sanityCheckUniverse(const Option::Universe expected_universe,
+ const Option::Universe actual_universe);
/// Option name.
std::string name_;
diff --git a/src/lib/dhcp/tests/option6_int_array_unittest.cc b/src/lib/dhcp/tests/option6_int_array_unittest.cc
index 26af726..4e58c3d 100644
--- a/src/lib/dhcp/tests/option6_int_array_unittest.cc
+++ b/src/lib/dhcp/tests/option6_int_array_unittest.cc
@@ -19,9 +19,6 @@
#include <dhcp/option6_iaaddr.h>
#include <util/buffer.h>
-#include <iostream>
-#include <sstream>
-#include <arpa/inet.h>
#include <boost/pointer_cast.hpp>
#include <gtest/gtest.h>
diff --git a/src/lib/dhcp/tests/option6_int_unittest.cc b/src/lib/dhcp/tests/option6_int_unittest.cc
index 906b954..1dfc205 100644
--- a/src/lib/dhcp/tests/option6_int_unittest.cc
+++ b/src/lib/dhcp/tests/option6_int_unittest.cc
@@ -19,9 +19,6 @@
#include <dhcp/option6_iaaddr.h>
#include <util/buffer.h>
-#include <iostream>
-#include <sstream>
-#include <arpa/inet.h>
#include <boost/pointer_cast.hpp>
#include <gtest/gtest.h>
diff --git a/src/lib/dhcp/tests/option_definition_unittest.cc b/src/lib/dhcp/tests/option_definition_unittest.cc
index 229771b..c8df21d 100644
--- a/src/lib/dhcp/tests/option_definition_unittest.cc
+++ b/src/lib/dhcp/tests/option_definition_unittest.cc
@@ -13,25 +13,23 @@
// PERFORMANCE OF THIS SOFTWARE.
#include <config.h>
-#include <iostream>
-#include <sstream>
+
+#include <exceptions/exceptions.h>
+#include <asiolink/io_address.h>
+#include <dhcp/dhcp4.h>
+#include <dhcp/dhcp6.h>
+#include <dhcp/option4_addrlst.h>
+#include <dhcp/option6_addrlst.h>
+#include <dhcp/option6_ia.h>
+#include <dhcp/option6_iaaddr.h>
+#include <dhcp/option6_int.h>
+#include <dhcp/option6_int_array.h>
+#include <dhcp/option_definition.h>
#include <gtest/gtest.h>
#include <boost/shared_ptr.hpp>
#include <boost/pointer_cast.hpp>
-#include <exceptions/exceptions.h>
-#include <asiolink/io_address.h>
-#include "dhcp/dhcp4.h"
-#include "dhcp/dhcp6.h"
-#include "dhcp/option4_addrlst.h"
-#include "dhcp/option6_addrlst.h"
-#include "dhcp/option6_ia.h"
-#include "dhcp/option6_iaaddr.h"
-#include "dhcp/option6_int.h"
-#include "dhcp/option6_int_array.h"
-#include "dhcp/option_definition.h"
-
using namespace std;
using namespace isc;
using namespace isc::dhcp;
@@ -57,6 +55,7 @@ TEST_F(OptionDefinitionTest, constructor) {
EXPECT_EQ(1, opt_def1.getCode());
EXPECT_EQ(OptionDefinition::STRING_TYPE, opt_def1.getType());
EXPECT_FALSE(opt_def1.getArrayType());
+ EXPECT_NO_THROW(opt_def1.validate());
// Specify the option data type as an enum value.
OptionDefinition opt_def2("OPTION_RAPID_COMMIT", 14,
@@ -65,6 +64,7 @@ TEST_F(OptionDefinitionTest, constructor) {
EXPECT_EQ(14, opt_def2.getCode());
EXPECT_EQ(OptionDefinition::EMPTY_TYPE, opt_def2.getType());
EXPECT_FALSE(opt_def2.getArrayType());
+ EXPECT_NO_THROW(opt_def1.validate());
// Check if it is possible to set that option is an array.
OptionDefinition opt_def3("OPTION_NIS_SERVERS", 27,
@@ -74,6 +74,7 @@ TEST_F(OptionDefinitionTest, constructor) {
EXPECT_EQ(27, opt_def3.getCode());
EXPECT_EQ(OptionDefinition::IPV6_ADDRESS_TYPE, opt_def3.getType());
EXPECT_TRUE(opt_def3.getArrayType());
+ EXPECT_NO_THROW(opt_def3.validate());
// The created object is invalid if invalid data type is specified but
// constructor shouldn't throw exception. The object is validated after
@@ -121,6 +122,31 @@ TEST_F(OptionDefinitionTest, addRecordField) {
EXPECT_THROW(opt_def.addRecordField(invalid_type), isc::BadValue);
}
+TEST_F(OptionDefinitionTest, validate) {
+ // Not supported option type string is not allowed.
+ OptionDefinition opt_def1("OPTION_CLIENTID", D6O_CLIENTID, "non-existent-type");
+ EXPECT_THROW(opt_def1.validate(), isc::OutOfRange);
+
+ // Not supported option type enum value is not allowed.
+ OptionDefinition opt_def2("OPTION_CLIENTID", D6O_CLIENTID, OptionDefinition::UNKNOWN_TYPE);
+ EXPECT_THROW(opt_def2.validate(), isc::OutOfRange);
+
+ OptionDefinition opt_def3("OPTION_CLIENTID", D6O_CLIENTID,
+ static_cast<OptionDefinition::DataType>(OptionDefinition::UNKNOWN_TYPE
+ + 2));
+ EXPECT_THROW(opt_def3.validate(), isc::OutOfRange);
+
+ // Empty option name is not allowed.
+ OptionDefinition opt_def4("", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def4.validate(), isc::BadValue);
+
+ // Option name must not contain spaces.
+ OptionDefinition opt_def5(" OPTION_CLIENTID", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def5.validate(), isc::BadValue);
+
+ OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string");
+ EXPECT_THROW(opt_def6.validate(), isc::BadValue);
+}
TEST_F(OptionDefinitionTest, factoryAddrList6) {
OptionDefinition opt_def("OPTION_NIS_SERVERS", D6O_NIS_SERVERS,
More information about the bind10-changes
mailing list