BIND 10 trac2545, updated. 9065335de2f05d3bc1d874b933ba80b51feb68f6 [2545] Added a test that checks whether value in CSV format is accepted.

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Dec 21 12:27:02 UTC 2012


The branch, trac2545 has been updated
       via  9065335de2f05d3bc1d874b933ba80b51feb68f6 (commit)
       via  caf95c197aa4abb916d8e27d3a5004ebed2e6108 (commit)
       via  a32568b7b3006315e79ee411b50db8cc4dc435a7 (commit)
       via  3c702b8966081eb2d73914754abd247350957bdd (commit)
      from  8b1a0145b79147510ccde00b28e5124b8951e4d1 (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 9065335de2f05d3bc1d874b933ba80b51feb68f6
Author: Marcin Siodelski <marcin at isc.org>
Date:   Fri Dec 21 13:26:52 2012 +0100

    [2545] Added a test that checks whether value in CSV format is accepted.

commit caf95c197aa4abb916d8e27d3a5004ebed2e6108
Author: Marcin Siodelski <marcin at isc.org>
Date:   Fri Dec 21 13:11:53 2012 +0100

    [2545] Added new configuration parameter: csv-format for DHCPv4 server.

commit a32568b7b3006315e79ee411b50db8cc4dc435a7
Author: Marcin Siodelski <marcin at isc.org>
Date:   Fri Dec 21 12:53:31 2012 +0100

    [2545] Separated build and commit phase for all DHCPv4 config parsers.
    
    ... also added new BooleanParser.

commit 3c702b8966081eb2d73914754abd247350957bdd
Author: Marcin Siodelski <marcin at isc.org>
Date:   Fri Dec 21 12:07:43 2012 +0100

    [2545] Moved the DHCPv4 parser classes to the anonymous namespace.

-----------------------------------------------------------------------

Summary of changes:
 src/bin/dhcp4/config_parser.cc                |  559 +++++++++++++++++++------
 src/bin/dhcp4/config_parser.h                 |  105 +----
 src/bin/dhcp4/dhcp4.spec                      |   10 +
 src/bin/dhcp4/tests/config_parser_unittest.cc |  108 ++++-
 src/bin/dhcp6/config_parser.cc                |    7 +-
 src/bin/dhcp6/tests/config_parser_unittest.cc |    1 -
 src/lib/dhcpsrv/subnet.cc                     |    4 +-
 7 files changed, 564 insertions(+), 230 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc
index 76181c2..1f03c9f 100644
--- a/src/bin/dhcp4/config_parser.cc
+++ b/src/bin/dhcp4/config_parser.cc
@@ -19,6 +19,7 @@
 #include <dhcp/option_definition.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <util/encode/hex.h>
+#include <util/strutil.h>
 #include <boost/foreach.hpp>
 #include <boost/lexical_cast.hpp>
 #include <boost/algorithm/string.hpp>
@@ -28,11 +29,26 @@
 #include <map>
 
 using namespace std;
+using namespace isc;
+using namespace isc::dhcp;
 using namespace isc::data;
 using namespace isc::asiolink;
 
-namespace isc {
-namespace dhcp {
+namespace {
+
+/// @brief Forward declaration to Dhcp4ConfigParser class.
+///
+/// It is only needed here to define types that are
+/// based on this class before the class definition.
+class Dhcp4ConfigParser;
+
+/// @brief a pointer to configuration parser
+typedef boost::shared_ptr<Dhcp4ConfigParser> ParserPtr;
+
+/// @brief a collection of parsers
+///
+/// This container is used to store pointer to parsers for a given scope.
+typedef std::vector<ParserPtr> ParserCollection;
 
 /// @brief auxiliary type used for storing element name and its parser
 typedef pair<string, ConstElementPtr> ConfigPair;
@@ -43,6 +59,9 @@ typedef Dhcp4ConfigParser* ParserFactory(const std::string& config_id);
 /// @brief a collection of factories that creates parsers for specified element names
 typedef std::map<std::string, ParserFactory*> FactoryMap;
 
+/// @brief Storage for parsed boolean values.
+typedef std::map<string, bool> BooleanStorage;
+
 /// @brief a collection of pools
 ///
 /// That type is used as intermediate storage, when pools are parsed, but there is
@@ -63,6 +82,82 @@ StringStorage string_defaults;
 /// @brief Global storage for options that will be used as defaults.
 OptionStorage option_defaults;
 
+/// @brief Base abstract class for all DHCPv4 parsers
+///
+/// Each instance of a class derived from this class parses one specific config
+/// element. Sometimes elements are simple (e.g. a string) and sometimes quite
+/// complex (e.g. a subnet). In such case, it is likely that a parser will
+/// spawn child parsers to parse child elements in the configuration.
+/// @todo: Merge this class with DhcpConfigParser in src/bin/dhcp6
+class Dhcp4ConfigParser {
+    ///
+    /// @name Constructors and Destructor
+    ///
+    /// Note: The copy constructor and the assignment operator are
+    /// intentionally defined as private to make it explicit that this is a
+    /// pure base class.
+    //@{
+private:
+
+    // Private construtor and assignment operator assures that nobody
+    // will be able to copy or assign a parser. There are no defined
+    // bodies for them.
+    Dhcp4ConfigParser(const Dhcp4ConfigParser& source);
+    Dhcp4ConfigParser& operator=(const Dhcp4ConfigParser& source);
+protected:
+    /// @brief The default constructor.
+    ///
+    /// This is intentionally defined as @c protected as this base class should
+    /// never be instantiated (except as part of a derived class).
+    Dhcp4ConfigParser() {}
+public:
+    /// The destructor.
+    virtual ~Dhcp4ConfigParser() {}
+    //@}
+
+    /// @brief Prepare configuration value.
+    ///
+    /// This method parses the "value part" of the configuration identifier
+    /// that corresponds to this derived class and prepares a new value to
+    /// apply to the server.
+    ///
+    /// This method must validate the given value both in terms of syntax
+    /// and semantics of the configuration, so that the server will be
+    /// validly configured at the time of @c commit().  Note: the given
+    /// configuration value is normally syntactically validated, but the
+    /// @c build() implementation must also expect invalid input.  If it
+    /// detects an error it may throw an exception of a derived class
+    /// of @c isc::Exception.
+    ///
+    /// Preparing a configuration value will often require resource
+    /// allocation.  If it fails, it may throw a corresponding standard
+    /// exception.
+    ///
+    /// This method is not expected to be called more than once in the
+    /// life of the object. Although multiple calls are not prohibited
+    /// by the interface, the behavior is undefined.
+    ///
+    /// @param config_value The configuration value for the identifier
+    /// corresponding to the derived class.
+    virtual void build(isc::data::ConstElementPtr config_value) = 0;
+
+    /// @brief Apply the prepared configuration value to the server.
+    ///
+    /// This method is expected to be exception free, and, as a consequence,
+    /// it should normally not involve resource allocation.
+    /// Typically it would simply perform exception free assignment or swap
+    /// operation on the value prepared in @c build().
+    /// In some cases, however, it may be very difficult to meet this
+    /// condition in a realistic way, while the failure case should really
+    /// be very rare.  In such a case it may throw, and, if the parser is
+    /// called via @c configureDhcp4Server(), the caller will convert the
+    /// exception as a fatal error.
+    ///
+    /// This method is expected to be called after @c build(), and only once.
+    /// The result is undefined otherwise.
+    virtual void commit() = 0;
+};
+
 /// @brief a dummy configuration parser
 ///
 /// It is a debugging parser. It does not configure anything,
@@ -74,7 +169,7 @@ public:
 
     /// @brief Constructor
     ///
-    /// See \ref Dhcp4ConfigParser class for details.
+    /// See @ref Dhcp4ConfigParser class for details.
     ///
     /// @param param_name name of the parsed parameter
     DebugParser(const std::string& param_name)
@@ -83,7 +178,7 @@ public:
 
     /// @brief builds parameter value
     ///
-    /// See \ref Dhcp4ConfigParser class for details.
+    /// See @ref Dhcp4ConfigParser class for details.
     ///
     /// @param new_config pointer to the new configuration
     virtual void build(ConstElementPtr new_config) {
@@ -97,7 +192,7 @@ public:
     /// This is a method required by base class. It pretends to apply the
     /// configuration, but in fact it only prints the parameter out.
     ///
-    /// See \ref Dhcp4ConfigParser class for details.
+    /// See @ref Dhcp4ConfigParser class for details.
     virtual void commit() {
         // Debug message. The whole DebugParser class is used only for parser
         // debugging, and is not used in production code. It is very convenient
@@ -121,6 +216,80 @@ private:
     ConstElementPtr value_;
 };
 
+/// @brief A boolean value parser.
+///
+/// This parser handles configuration values of the boolean type.
+/// Parsed values are stored in a provided storage. If no storage
+/// is provided then the build function throws an exception.
+class BooleanParser : public Dhcp4ConfigParser {
+public:
+    /// @brief Constructor.
+    ///
+    /// @param param_name name of the parameter.
+    BooleanParser(const std::string& param_name)
+        : storage_(NULL),
+          param_name_(param_name),
+          value_(false) {
+    }
+
+    /// @brief Parse a boolean value.
+    ///
+    /// @param value a value to be parsed.
+    ///
+    /// @throw isc::InvalidOperation if a storage has not been set
+    ///        prior to calling this function
+    /// @throw isc::dhcp::Dhcp4ConfigError if a provided parameter's
+    ///        name is empty.
+    virtual void build(ConstElementPtr value) {
+        if (storage_ == NULL) {
+            isc_throw(isc::InvalidOperation, "parser logic error:"
+                      << " storage for the " << param_name_
+                      << " value has not been set");
+        } else if (param_name_.empty()) {
+            isc_throw(isc::dhcp::Dhcp4ConfigError, "parser logic error:"
+                      << "empty parameter name provided");
+        }
+        // The Config Manager checks if user specified a
+        // valid value for a boolean parameter: True or False.
+        // It is then ok to assume that if str() does not return
+        // 'true' the value is 'false'.
+        value_ = (value->str() == "true") ? true : false;
+    }
+
+    /// @brief Put a parsed value to the storage.
+    virtual void commit() {
+        if (storage_ != NULL && !param_name_.empty()) {
+            (*storage_)[param_name_] = value_;
+        }
+    }
+
+    /// @brief Create an instance of the boolean parser.
+    ///
+    /// @param param_name name of the parameter for which the
+    ///        parser is created.
+    static Dhcp4ConfigParser* Factory(const std::string& param_name) {
+        return (new BooleanParser(param_name));
+    }
+
+    /// @brief Set the storage for parsed value.
+    ///
+    /// This function must be called prior to calling build.
+    ///
+    /// @param storage a pointer to the storage where parsed data
+    ///        is to be stored.
+    void setStorage(BooleanStorage* storage) {
+        storage_ = storage;
+    }
+
+private:
+    /// Pointer to the storage where parsed value is stored.
+    BooleanStorage* storage_;
+    /// Name of the parameter which value is parsed with this parser.
+    std::string param_name_;
+    /// Parsed value.
+    bool value_;
+};
+
 /// @brief Configuration parser for uint32 parameters
 ///
 /// This class is a generic parser that is able to handle any uint32 integer
@@ -128,27 +297,31 @@ private:
 /// (uint32_defaults). If used in smaller scopes (e.g. to parse parameters
 /// in subnet config), it can be pointed to a different storage, using
 /// setStorage() method. This class follows the parser interface, laid out
-/// in its base class, \ref Dhcp4ConfigParser.
+/// in its base class, @ref Dhcp4ConfigParser.
 ///
 /// For overview of usability of this generic purpose parser, see
-/// \ref dhcpv4ConfigInherit page.
+/// @ref dhcpv4ConfigInherit page.
 class Uint32Parser : public Dhcp4ConfigParser {
 public:
 
     /// @brief constructor for Uint32Parser
     /// @param param_name name of the configuration parameter being parsed
     Uint32Parser(const std::string& param_name)
-        :storage_(&uint32_defaults), param_name_(param_name) {
+        : storage_(&uint32_defaults),
+          param_name_(param_name) {
     }
 
-    /// @brief builds parameter value
-    ///
-    /// Parses configuration entry and stores it in a storage. See
-    /// \ref setStorage() for details.
+    /// @brief Parses configuration configuration parameter as uint32_t.
     ///
     /// @param value pointer to the content of parsed values
     /// @throw BadValue if supplied value could not be base to uint32_t
+    ///        or the parameter name is empty.
     virtual void build(ConstElementPtr value) {
+        if (param_name_.empty()) {
+            isc_throw(BadValue, "parser logic error:"
+                      << "empty parameter name provided");
+        }
+
         int64_t check;
         string x = value->str();
         try {
@@ -168,19 +341,15 @@ public:
 
         // value is small enough to fit
         value_ = static_cast<uint32_t>(check);
-
-        (*storage_)[param_name_] = value_;
     }
 
-    /// @brief does nothing
-    ///
-    /// This method is required for all parsers. The value itself
-    /// is not commited anywhere. Higher level parsers are expected to
-    /// use values stored in the storage, e.g. renew-timer for a given
-    /// subnet is stored in subnet-specific storage. It is not commited
-    /// here, but is rather used by \ref Subnet4ConfigParser when constructing
-    /// the subnet.
+    /// @brief Stores the parsed uint32_t value in a storage.
     virtual void commit() {
+        if (storage_ != NULL && !param_name_.empty()) {
+            // If a given parameter already exists in the storage we override
+            // its value. If it doesn't we insert a new element.
+            (*storage_)[param_name_] = value_;
+        }
     }
 
     /// @brief factory that constructs Uint32Parser objects
@@ -192,7 +361,7 @@ public:
 
     /// @brief sets storage for value of this parameter
     ///
-    /// See \ref dhcpv4ConfigInherit for details.
+    /// See @ref dhcpv4ConfigInherit for details.
     ///
     /// @param storage pointer to the storage container
     void setStorage(Uint32Storage* storage) {
@@ -217,10 +386,10 @@ private:
 /// (string_defaults). If used in smaller scopes (e.g. to parse parameters
 /// in subnet config), it can be pointed to a different storage, using
 /// setStorage() method. This class follows the parser interface, laid out
-/// in its base class, \ref Dhcp4ConfigParser.
+/// in its base class, @ref Dhcp4ConfigParser.
 ///
 /// For overview of usability of this generic purpose parser, see
-/// \ref dhcpv4ConfigInherit page.
+/// @ref dhcpv4ConfigInherit page.
 class StringParser : public Dhcp4ConfigParser {
 public:
 
@@ -233,25 +402,21 @@ public:
     /// @brief parses parameter value
     ///
     /// Parses configuration entry and stores it in storage. See
-    /// \ref setStorage() for details.
+    /// @ref setStorage() for details.
     ///
     /// @param value pointer to the content of parsed values
     virtual void build(ConstElementPtr value) {
         value_ = value->str();
         boost::erase_all(value_, "\"");
-
-        (*storage_)[param_name_] = value_;
     }
 
-    /// @brief does nothing
-    ///
-    /// This method is required for all parser. The value itself
-    /// is not commited anywhere. Higher level parsers are expected to
-    /// use values stored in the storage, e.g. renew-timer for a given
-    /// subnet is stored in subnet-specific storage. It is not commited
-    /// here, but is rather used by its parent parser when constructing
-    /// an object, e.g. the subnet.
+    /// @brief Stores the parsed value in a storage.
     virtual void commit() {
+        if (storage_ != NULL && !param_name_.empty()) {
+            // If a given parameter already exists in the storage we override
+            // its value. If it doesn't we insert a new element.
+            (*storage_)[param_name_] = value_;
+        }
     }
 
     /// @brief factory that constructs StringParser objects
@@ -408,7 +573,7 @@ public:
                 }
 
                 Pool4Ptr pool(new Pool4(addr, len));
-                pools_->push_back(pool);
+                local_pools_.push_back(pool);
                 continue;
             }
 
@@ -421,7 +586,7 @@ public:
 
                 Pool4Ptr pool(new Pool4(min, max));
 
-                pools_->push_back(pool);
+                local_pools_.push_back(pool);
                 continue;
             }
 
@@ -440,12 +605,17 @@ public:
         pools_ = storage;
     }
 
-    /// @brief does nothing.
-    ///
-    /// This method is required for all parsers. The value itself
-    /// is not commited anywhere. Higher level parsers (for subnet) are expected
-    /// to use values stored in the storage.
-    virtual void commit() {}
+    /// @brief Stores the parsed values in a storage provided
+    ///        by an upper level parser.
+    virtual void commit() {
+        if (pools_) {
+            // local_pools_ holds the values produced by the build function.
+            // At this point parsing should have completed successfuly so
+            // we can append new data to the supplied storage.
+            pools_->insert(pools_->end(), local_pools_.begin(),
+                           local_pools_.end());
+        }
+    }
 
     /// @brief factory that constructs PoolParser objects
     ///
@@ -460,6 +630,9 @@ private:
     /// That is typically a storage somewhere in Subnet parser
     /// (an upper level parser).
     PoolStorage* pools_;
+    /// A temporary storage for pools configuration. It is a
+    /// storage where pools are stored by build function.
+    PoolStorage local_pools_;
 };
 
 /// @brief Parser for option data value.
@@ -531,12 +704,26 @@ public:
                     value_parser->setStorage(&string_values_);
                     parser = value_parser;
                 }
+            } else if (param.first == "csv-format") {
+                boost::shared_ptr<BooleanParser>
+                    value_parser(dynamic_cast<BooleanParser*>(BooleanParser::Factory(param.first)));
+                if (value_parser) {
+                    value_parser->setStorage(&boolean_values_);
+                    parser = value_parser;
+                }
             } else {
                 isc_throw(Dhcp4ConfigError,
                           "Parser error: option-data parameter not supported: "
                           << param.first);
             }
             parser->build(param.second);
+            // Before we can create an option we need to get the data from
+            // the child parsers. The only way to do it is to invoke commit
+            // on them so as they store the values in appropriate storages
+            // that this class provided to them. Note that this will not
+            // modify values stored in the global storages so the configuration
+            // will remain consistent even parsing fails somewhere further on.
+            parser->commit();
         }
         // Try to create the option instance.
         createOption();
@@ -627,16 +814,27 @@ private:
         }
 
         // Get option data from the configuration database ('data' field).
-        // Option data is specified by the user as case insensitive string
-        // of hexadecimal digits for each option.
-        std::string option_data = getStringParam("data");
+        const std::string option_data = getStringParam("data");
+        const bool csv_format = getBooleanParam("csv-format");
         // Transform string of hexadecimal digits into binary format.
         std::vector<uint8_t> binary;
-        try {
-            util::encode::decodeHex(option_data, binary);
-        } catch (...) {
-            isc_throw(Dhcp4ConfigError, "Parser error: option data is not a valid"
-                      << " string of hexadecimal digits: " << option_data);
+        std::vector<std::string> data_tokens;
+
+        if (csv_format) {
+            // If the option data is specified as a string of comma
+            // separated values then we need to split this string into
+            // individual values - each value will be used to initialize
+            // one data field of an option.
+            data_tokens = isc::util::str::tokens(option_data, ",");
+        } else {
+            // Otherwise, the option data is specified as a string of
+            // hexadecimal digits that we have to turn into binary format.
+            try {
+                util::encode::decodeHex(option_data, binary);
+            } catch (...) {
+                isc_throw(Dhcp4ConfigError, "Parser error: option data is not a valid"
+                          << " string of hexadecimal digits: " << option_data);
+            }
         }
         // Get all existing DHCPv4 option definitions. The one that matches
         // our option will be picked and used to create it.
@@ -656,6 +854,13 @@ private:
                       << " for the same option code. This will be supported once"
                       << " there option spaces are implemented.");
         } else if (num_defs == 0) {
+            if (csv_format) {
+                isc_throw(Dhcp4ConfigError, "the CSV option data format can be"
+                          " used to specify values for an option that has a"
+                          " definition. The option with code " << option_code
+                          << " does not have a definition.");
+            }
+
             // @todo We have a limited set of option definitions intiialized at the moment.
             // In the future we want to initialize option definitions for all options.
             // Consequently an error will be issued if an option definition does not exist
@@ -673,7 +878,9 @@ private:
             // use it to create the option instance.
             const OptionDefinitionPtr& def = *(range.first);
             try {
-                OptionPtr option = def->optionFactory(Option::V4, option_code, binary);
+                OptionPtr option = csv_format ?
+                    def->optionFactory(Option::V4, option_code, data_tokens) :
+                    def->optionFactory(Option::V4, option_code, binary);
                 Subnet::OptionDescriptor desc(option, false);
                 option_descriptor_.option = option;
                 option_descriptor_.persistent = false;
@@ -711,10 +918,27 @@ private:
         return (param->second);
     }
 
+    /// @brief Get a parameter from the boolean values storage.
+    ///
+    /// @param param_id parameter identifier.
+    ///
+    /// @throw isc::dhcp::Dhcp6ConfigError if a parameter has not been found.
+    /// @return a value of the boolean parameter.
+    bool getBooleanParam(const std::string& param_id) const {
+        BooleanStorage::const_iterator param = boolean_values_.find(param_id);
+        if (param == boolean_values_.end()) {
+            isc_throw(isc::dhcp::Dhcp4ConfigError, "parser error: option-data parameter"
+                      << " '" << param_id << "' not specified");
+        }
+        return (param->second);
+    }
+
     /// Storage for uint32 values (e.g. option code).
     Uint32Storage uint32_values_;
     /// Storage for string values (e.g. option name or data).
     StringStorage string_values_;
+    /// Storage for boolean values.
+    BooleanStorage boolean_values_;
     /// Pointer to options storage. This storage is provided by
     /// the calling class and is shared by all OptionDataParser objects.
     OptionStorage* options_;
@@ -844,7 +1068,20 @@ public:
                 isc_throw(Dhcp4ConfigError, "failed to find suitable parser");
             }
         }
-        // Ok, we now have subnet parsed
+        // In order to create new subnet we need to get the data out
+        // of the child parsers first. The only way to do it is to
+        // invoke commit on them because it will make them write
+        // parsed data into storages we have supplied.
+        // Note that triggering commits on child parsers does not
+        // affect global data because we supplied pointers to storages
+        // local to this object. Thus, even if this method fails
+        // later on, the configuration remains consistent.
+        BOOST_FOREACH(ParserPtr parser, parsers_) {
+            parser->commit();
+        }
+
+        // Create a subnet.
+        createSubnet();
     }
 
     /// @brief commits received configuration.
@@ -855,11 +1092,51 @@ public:
     /// objects. Subnet4 are then added to DHCP CfgMgr.
     /// @throw Dhcp4ConfigError if there are any issues encountered during commit
     void commit() {
-        // Invoke commit on all sub-data parsers.
-        BOOST_FOREACH(ParserPtr parser, parsers_) {
-            parser->commit();
+        if (subnet_) {
+            CfgMgr::instance().addSubnet4(subnet_);
         }
+    }
 
+private:
+
+    /// @brief Set storage for a parser and invoke build.
+    ///
+    /// This helper method casts the provided parser pointer to the specified
+    /// type. If the cast is successful it sets the corresponding storage for
+    /// this parser, invokes build on it and saves the parser.
+    ///
+    /// @tparam T parser type to which parser argument should be cast.
+    /// @tparam Y storage type for the specified parser type.
+    /// @param parser parser on which build must be invoked.
+    /// @param storage reference to a storage that will be set for a parser.
+    /// @param subnet subnet element read from the configuration and being parsed.
+    /// @return true if parser pointer was successfully cast to specialized
+    /// parser type provided as Y.
+    template<typename T, typename Y>
+    bool buildParser(const ParserPtr& parser, Y& storage, const ConstElementPtr& subnet) {
+        // We need to cast to T in order to set storage for the parser.
+        boost::shared_ptr<T> cast_parser = boost::dynamic_pointer_cast<T>(parser);
+        // It is common that this cast is not successful because we try to cast to all
+        // supported parser types as we don't know the type of a parser in advance.
+        if (cast_parser) {
+            // Cast, successful so we go ahead with setting storage and actual parse.
+            cast_parser->setStorage(&storage);
+            parser->build(subnet);
+            parsers_.push_back(parser);
+            // We indicate that cast was successful so as the calling function
+            // may skip attempts to cast to other parser types and proceed to
+            // next element.
+            return (true);
+        }
+        // It was not successful. Indicate that another parser type
+        // should be tried.
+        return (false);
+    }
+
+    /// @brief Create a new subnet using a data from child parsers.
+    ///
+    /// @throw isc::dhcp::Dhcp4ConfigError if subnet configuration parsing failed.
+    void createSubnet() {
         StringStorage::const_iterator it = string_values_.find("subnet");
         if (it == string_values_.end()) {
             isc_throw(Dhcp4ConfigError,
@@ -888,13 +1165,13 @@ public:
 
         LOG_INFO(dhcp4_logger, DHCP4_CONFIG_NEW_SUBNET).arg(tmp.str());
 
-        Subnet4Ptr subnet(new Subnet4(addr, len, t1, t2, valid));
+        subnet_.reset(new Subnet4(addr, len, t1, t2, valid));
 
         for (PoolStorage::iterator it = pools_.begin(); it != pools_.end(); ++it) {
-            subnet->addPool4(*it);
+            subnet_->addPool4(*it);
         }
 
-        const Subnet::OptionContainer& options = subnet->getOptions();
+        const Subnet::OptionContainer& options = subnet_->getOptions();
         const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
 
         // Add subnet specific options.
@@ -904,7 +1181,7 @@ public:
                 LOG_WARN(dhcp4_logger, DHCP4_CONFIG_OPTION_DUPLICATE)
                     .arg(desc.option->getType()).arg(addr.toText());
             }
-            subnet->addOption(desc.option);
+            subnet_->addOption(desc.option);
         }
 
         // Check all global options and add them to the subnet object if
@@ -924,47 +1201,9 @@ public:
             // want to issue a warning about dropping the configuration of
             // a global option if one already exsists.
             if (std::distance(range.first, range.second) == 0) {
-                subnet->addOption(desc.option);
+                subnet_->addOption(desc.option);
             }
         }
-
-        CfgMgr::instance().addSubnet4(subnet);
-    }
-
-private:
-
-    /// @brief Set storage for a parser and invoke build.
-    ///
-    /// This helper method casts the provided parser pointer to the specified
-    /// type. If the cast is successful it sets the corresponding storage for
-    /// this parser, invokes build on it and saves the parser.
-    ///
-    /// @tparam T parser type to which parser argument should be cast.
-    /// @tparam Y storage type for the specified parser type.
-    /// @param parser parser on which build must be invoked.
-    /// @param storage reference to a storage that will be set for a parser.
-    /// @param subnet subnet element read from the configuration and being parsed.
-    /// @return true if parser pointer was successfully cast to specialized
-    /// parser type provided as Y.
-    template<typename T, typename Y>
-    bool buildParser(const ParserPtr& parser, Y& storage, const ConstElementPtr& subnet) {
-        // We need to cast to T in order to set storage for the parser.
-        boost::shared_ptr<T> cast_parser = boost::dynamic_pointer_cast<T>(parser);
-        // It is common that this cast is not successful because we try to cast to all
-        // supported parser types as we don't know the type of a parser in advance.
-        if (cast_parser) {
-            // Cast, successful so we go ahead with setting storage and actual parse.
-            cast_parser->setStorage(&storage);
-            parser->build(subnet);
-            parsers_.push_back(parser);
-            // We indicate that cast was successful so as the calling function
-            // may skip attempts to cast to other parser types and proceed to
-            // next element.
-            return (true);
-        }
-        // It was not successful. Indicate that another parser type
-        // should be tried.
-        return (false);
     }
 
     /// @brief creates parsers for entries in subnet definition
@@ -1043,6 +1282,9 @@ private:
 
     /// parsers are stored here
     ParserCollection parsers_;
+
+    /// @brief Pointer to the created subnet object.
+    isc::dhcp::Subnet4Ptr subnet_;
 };
 
 /// @brief this class parses list of subnets
@@ -1104,8 +1346,14 @@ public:
 
     /// @brief collection of subnet parsers.
     ParserCollection subnets_;
+
 };
 
+} // anonymous namespace
+
+namespace isc {
+namespace dhcp {
+
 /// @brief creates global parsers
 ///
 /// This method creates global parsers that parse global parameters, i.e.
@@ -1150,44 +1398,117 @@ configureDhcp4Server(Dhcpv4Srv& , ConstElementPtr config_set) {
 
     LOG_DEBUG(dhcp4_logger, DBG_DHCP4_COMMAND, DHCP4_CONFIG_START).arg(config_set->str());
 
-    ParserCollection parsers;
+    // Some of the values specified in the configuration depend on
+    // other values. Typically, the values in the subnet6 structure
+    // depend on the global values. Thus we need to make sure that
+    // the global values are processed first and that they can be
+    // accessed by the subnet6 parsers. We separate parsers that
+    // should process data first (independent_parsers) from those
+    // that must process data when the independent data is already
+    // processed (dependent_parsers).
+    ParserCollection independent_parsers;
+    ParserCollection dependent_parsers;
+
+    // The subnet parsers implement data inheritance by directly
+    // accesing global storages. For this reason the global data
+    // parsers must store the parsed data into global storages
+    // immediatelly. This may cause data inconsistency if the
+    // parsing operation fails after the global storage have been
+    // already modified. We need to preserve the original global
+    // data here so as we can rollback changes when an error occurs.
+    Uint32Storage uint32_local(uint32_defaults);
+    StringStorage string_local(string_defaults);
+    OptionStorage option_local(option_defaults);
+
+    // answer will hold the result.
+    ConstElementPtr answer;
+    // rollback informs whether error occured and original data
+    // have to be restored to global storages.
+    bool rollback = false;
+
     try {
+
+        // Iterate over all independent parsers first (all but subnet6)
+        // and try to parse the data.
         BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) {
+            ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first));
+            if (config_pair.first != "subnet4") {
+                independent_parsers.push_back(parser);
+                parser->build(config_pair.second);
+                // The commit operation here may modify the global storage
+                // but we need it so as the subnet6 parser can access the
+                // parsed data.
+                parser->commit();
+            }
+        }
 
+        // Process dependent configuration data.
+        BOOST_FOREACH(ConfigPair config_pair, config_set->mapValue()) {
             ParserPtr parser(createGlobalDhcp4ConfigParser(config_pair.first));
-            parser->build(config_pair.second);
-            parsers.push_back(parser);
+            if (config_pair.first == "subnet4") {
+                dependent_parsers.push_back(parser);
+                parser->build(config_pair.second);
+            }
         }
+
     } catch (const isc::Exception& ex) {
-        ConstElementPtr answer = isc::config::createAnswer(1,
-                                 string("Configuration parsing failed: ") + ex.what());
-        return (answer);
+        answer =
+            isc::config::createAnswer(1, string("Configuration parsing failed: ") + ex.what());
+
+        // An error occured, so make sure that we restore original data.
+        rollback = true;
+
     } catch (...) {
         // for things like bad_cast in boost::lexical_cast
-        ConstElementPtr answer = isc::config::createAnswer(1,
-                                 string("Configuration parsing failed"));
+        answer =
+            isc::config::createAnswer(1, string("Configuration parsing failed"));
+
+        // An error occured, so make sure that we restore original data.
+        rollback = true;
     }
 
-    try {
-        BOOST_FOREACH(ParserPtr parser, parsers) {
-            parser->commit();
+    // So far so good, there was no parsing error so let's commit the
+    // configuration. This will add created subnets and option values into
+    // the server's configuration.
+    // This operation should be exception safe but let's make sure.
+    if (!rollback) {
+        try {
+            BOOST_FOREACH(ParserPtr parser, dependent_parsers) {
+                parser->commit();
+            }
+        }
+        catch (const isc::Exception& ex) {
+            answer =
+                isc::config::createAnswer(2, string("Configuration commit failed: ") + ex.what());
+            rollback = true;
+
+        } catch (...) {
+            // for things like bad_cast in boost::lexical_cast
+            answer =
+                isc::config::createAnswer(2, string("Configuration commit failed"));
+            rollback = true;
+
         }
     }
-    catch (const isc::Exception& ex) {
-        ConstElementPtr answer = isc::config::createAnswer(2,
-                                 string("Configuration commit failed: ") + ex.what());
+
+    // Rollback changes as the configuration parsing failed.
+    if (rollback) {
+        std::swap(uint32_defaults, uint32_local);
+        std::swap(string_defaults, string_local);
+        std::swap(option_defaults, option_local);
         return (answer);
-    } catch (...) {
-        // for things like bad_cast in boost::lexical_cast
-        ConstElementPtr answer = isc::config::createAnswer(2,
-                                 string("Configuration commit failed"));
     }
 
     LOG_INFO(dhcp4_logger, DHCP4_CONFIG_COMPLETE).arg(config_details);
 
-    ConstElementPtr answer = isc::config::createAnswer(0, "Configuration commited.");
+    // Everything was fine. Configuration is successful.
+    answer = isc::config::createAnswer(0, "Configuration commited.");
     return (answer);
 }
 
+const std::map<std::string, uint32_t>& getUint32Defaults() {
+    return (uint32_defaults);
+}
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
diff --git a/src/bin/dhcp4/config_parser.h b/src/bin/dhcp4/config_parser.h
index cc4c690..0adfb36 100644
--- a/src/bin/dhcp4/config_parser.h
+++ b/src/bin/dhcp4/config_parser.h
@@ -35,7 +35,7 @@ typedef std::map<std::string, uint32_t> Uint32Storage;
 typedef std::map<std::string, std::string> StringStorage;
 
 /// An exception that is thrown if an error occurs while configuring an
-/// \c Dhcpv4Srv object.
+/// @c Dhcpv4Srv object.
 class Dhcp4ConfigError : public isc::Exception {
 public:
 
@@ -48,97 +48,12 @@ public:
         : isc::Exception(file, line, what) {}
 };
 
-/// @brief Base abstract class for all DHCPv4 parsers
+/// @brief Configure DHCPv4 server (@c Dhcpv4Srv) with a set of configuration values.
 ///
-/// Each instance of a class derived from this class parses one specific config
-/// element. Sometimes elements are simple (e.g. a string) and sometimes quite
-/// complex (e.g. a subnet). In such case, it is likely that a parser will
-/// spawn child parsers to parse child elements in the configuration.
-/// @todo: Merge this class with DhcpConfigParser in src/bin/dhcp6
-class Dhcp4ConfigParser {
-    ///
-    /// \name Constructors and Destructor
-    ///
-    /// Note: The copy constructor and the assignment operator are
-    /// intentionally defined as private to make it explicit that this is a
-    /// pure base class.
-    //@{
-private:
-
-    // Private construtor and assignment operator assures that nobody
-    // will be able to copy or assign a parser. There are no defined
-    // bodies for them.
-    Dhcp4ConfigParser(const Dhcp4ConfigParser& source);
-    Dhcp4ConfigParser& operator=(const Dhcp4ConfigParser& source);
-protected:
-    /// \brief The default constructor.
-    ///
-    /// This is intentionally defined as \c protected as this base class should
-    /// never be instantiated (except as part of a derived class).
-    Dhcp4ConfigParser() {}
-public:
-    /// The destructor.
-    virtual ~Dhcp4ConfigParser() {}
-    //@}
-
-    /// \brief Prepare configuration value.
-    ///
-    /// This method parses the "value part" of the configuration identifier
-    /// that corresponds to this derived class and prepares a new value to
-    /// apply to the server.
-    ///
-    /// This method must validate the given value both in terms of syntax
-    /// and semantics of the configuration, so that the server will be
-    /// validly configured at the time of \c commit().  Note: the given
-    /// configuration value is normally syntactically validated, but the
-    /// \c build() implementation must also expect invalid input.  If it
-    /// detects an error it may throw an exception of a derived class
-    /// of \c isc::Exception.
-    ///
-    /// Preparing a configuration value will often require resource
-    /// allocation.  If it fails, it may throw a corresponding standard
-    /// exception.
-    ///
-    /// This method is not expected to be called more than once in the
-    /// life of the object. Although multiple calls are not prohibited
-    /// by the interface, the behavior is undefined.
-    ///
-    /// \param config_value The configuration value for the identifier
-    /// corresponding to the derived class.
-    virtual void build(isc::data::ConstElementPtr config_value) = 0;
-
-    /// \brief Apply the prepared configuration value to the server.
-    ///
-    /// This method is expected to be exception free, and, as a consequence,
-    /// it should normally not involve resource allocation.
-    /// Typically it would simply perform exception free assignment or swap
-    /// operation on the value prepared in \c build().
-    /// In some cases, however, it may be very difficult to meet this
-    /// condition in a realistic way, while the failure case should really
-    /// be very rare.  In such a case it may throw, and, if the parser is
-    /// called via \c configureDhcp4Server(), the caller will convert the
-    /// exception as a fatal error.
-    ///
-    /// This method is expected to be called after \c build(), and only once.
-    /// The result is undefined otherwise.
-    virtual void commit() = 0;
-};
-
-/// @brief a pointer to configuration parser
-typedef boost::shared_ptr<Dhcp4ConfigParser> ParserPtr;
-
-/// @brief a collection of parsers
-///
-/// This container is used to store pointer to parsers for a given scope.
-typedef std::vector<ParserPtr> ParserCollection;
-
-
-/// \brief Configure DHCPv4 server (\c Dhcpv4Srv) with a set of configuration values.
-///
-/// This function parses configuration information stored in \c config_set
-/// and configures the \c server by applying the configuration to it.
+/// This function parses configuration information stored in @c config_set
+/// and configures the @c server by applying the configuration to it.
 /// It provides the strong exception guarantee as long as the underlying
-/// derived class implementations of \c DhcpConfigParser meet the assumption,
+/// derived class implementations of @c DhcpConfigParser meet the assumption,
 /// that is, it ensures that either configuration is fully applied or the
 /// state of the server is intact.
 ///
@@ -162,6 +77,16 @@ isc::data::ConstElementPtr
 configureDhcp4Server(Dhcpv4Srv&,
                      isc::data::ConstElementPtr config_set);
 
+
+/// @brief Returns the global uint32_t values storage.
+///
+/// This function must be only used by unit tests that need
+/// to access uint32_t global storage to verify that the
+/// Uint32Parser works as expected.
+///
+/// @return a reference to a global uint32 values storage.
+const std::map<std::string, uint32_t>& getUint32Defaults();
+
 }; // end of isc::dhcp namespace
 }; // end of isc namespace
 
diff --git a/src/bin/dhcp4/dhcp4.spec b/src/bin/dhcp4/dhcp4.spec
index 46192de..7b4fc63 100644
--- a/src/bin/dhcp4/dhcp4.spec
+++ b/src/bin/dhcp4/dhcp4.spec
@@ -61,6 +61,11 @@
             "item_type": "string",
             "item_optional": false,
             "item_default": ""
+          },
+          { "item_name": "csv-format",
+            "item_type": "boolean",
+            "item_optional": false,
+            "item_default": False
           } ]
         }
       },
@@ -141,6 +146,11 @@
                       "item_type": "string",
                       "item_optional": false,
                       "item_default": ""
+                    },
+                    { "item_name": "csv-format",
+                      "item_type": "bool",
+                      "item_optional": false,
+                      "item_default": False
                     } ]
                   }
                 } ]
diff --git a/src/bin/dhcp4/tests/config_parser_unittest.cc b/src/bin/dhcp4/tests/config_parser_unittest.cc
index 3dd75d7..baf550d 100644
--- a/src/bin/dhcp4/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp4/tests/config_parser_unittest.cc
@@ -17,9 +17,10 @@
 #include <arpa/inet.h>
 #include <gtest/gtest.h>
 
+#include <config/ccsession.h>
 #include <dhcp4/dhcp4_srv.h>
 #include <dhcp4/config_parser.h>
-#include <config/ccsession.h>
+#include <dhcp/option4_addrlst.h>
 #include <dhcpsrv/subnet.h>
 #include <dhcpsrv/cfgmgr.h>
 #include <boost/foreach.hpp>
@@ -35,12 +36,6 @@ using namespace isc::asiolink;
 using namespace isc::data;
 using namespace isc::config;
 
-namespace isc {
-namespace dhcp {
-extern Uint32Storage uint32_defaults;
-}
-}
-
 namespace {
 
 class Dhcp4ParserTest : public ::testing::Test {
@@ -55,7 +50,9 @@ public:
 
     // Checks if global parameter of name have expected_value
     void checkGlobalUint32(string name, uint32_t expected_value) {
-        Uint32Storage::const_iterator it = uint32_defaults.find(name);
+        const std::map<std::string, uint32_t>& uint32_defaults = getUint32Defaults();
+        std::map<std::string, uint32_t>::const_iterator it =
+            uint32_defaults.find(name);
         if (it == uint32_defaults.end()) {
             ADD_FAILURE() << "Expected uint32 with name " << name
                           << " not found";
@@ -96,14 +93,22 @@ public:
             params["name"] = param_value;
             params["code"] = "56";
             params["data"] = "AB CDEF0105";
+            params["csv-format"] = "False";
         } else if (parameter == "code") {
             params["name"] = "option_foo";
             params["code"] = param_value;
             params["data"] = "AB CDEF0105";
+            params["csv-format"] = "False";
         } else if (parameter == "data") {
             params["name"] = "option_foo";
             params["code"] = "56";
             params["data"] = param_value;
+            params["csv-format"] = "False";
+        } else if (parameter == "csv-format") {
+            params["name"] = "option_foo";
+            params["code"] = "56";
+            params["data"] = "AB CDEF0105";
+            params["csv-format"] = param_value;
         }
         return (createConfigWithOption(params));
     }
@@ -140,6 +145,8 @@ public:
                 stream << "\"code\": " << param.second << "";
             } else if (param.first == "data") {
                 stream << "\"data\": \"" << param.second << "\"";
+            } else if (param.first == "csv-format") {
+                stream << "\"csv-format\": " << param.second;
             }
         }
         stream <<
@@ -393,9 +400,9 @@ TEST_F(Dhcp4ParserTest, poolOutOfSubnet) {
 
     EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
 
-    // returned value must be 2 (values error)
+    // returned value must be 1 (values error)
     // as the pool does not belong to that subnet
-    checkResult(status, 2);
+    checkResult(status, 1);
 }
 
 // Goal of this test is to verify if pools can be defined
@@ -439,12 +446,14 @@ TEST_F(Dhcp4ParserTest, optionDataDefaults) {
         "\"option-data\": [ {"
         "    \"name\": \"option_foo\","
         "    \"code\": 56,"
-        "    \"data\": \"AB CDEF0105\""
+        "    \"data\": \"AB CDEF0105\","
+        "    \"csv-format\": False"
         " },"
         " {"
         "    \"name\": \"option_foo2\","
         "    \"code\": 23,"
-        "    \"data\": \"01\""
+        "    \"data\": \"01\","
+        "    \"csv-format\": False"
         " } ],"
         "\"subnet4\": [ { "
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
@@ -502,7 +511,8 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
         "\"option-data\": [ {"
         "      \"name\": \"option_foo\","
         "      \"code\": 56,"
-        "      \"data\": \"AB\""
+        "      \"data\": \"AB\","
+        "      \"csv-format\": False"
         " } ],"
         "\"subnet4\": [ { "
         "    \"pool\": [ \"192.0.2.1 - 192.0.2.100\" ],"
@@ -510,12 +520,14 @@ TEST_F(Dhcp4ParserTest, optionDataInSingleSubnet) {
         "    \"option-data\": [ {"
         "          \"name\": \"option_foo\","
         "          \"code\": 56,"
-        "          \"data\": \"AB CDEF0105\""
+        "          \"data\": \"AB CDEF0105\","
+        "          \"csv-format\": False"
         "        },"
         "        {"
         "          \"name\": \"option_foo2\","
         "          \"code\": 23,"
-        "          \"data\": \"01\""
+        "          \"data\": \"01\","
+        "          \"csv-format\": False"
         "        } ]"
         " } ],"
         "\"valid-lifetime\": 4000 }";
@@ -571,7 +583,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
         "    \"option-data\": [ {"
         "          \"name\": \"option_foo\","
         "          \"code\": 56,"
-        "          \"data\": \"0102030405060708090A\""
+        "          \"data\": \"0102030405060708090A\","
+        "          \"csv-format\": False"
         "        } ]"
         " },"
         " {"
@@ -580,7 +593,8 @@ TEST_F(Dhcp4ParserTest, optionDataInMultipleSubnets) {
         "    \"option-data\": [ {"
         "          \"name\": \"option_foo2\","
         "          \"code\": 23,"
-        "          \"data\": \"FF\""
+        "          \"data\": \"FF\","
+        "          \"csv-format\": False"
         "        } ]"
         " } ],"
         "\"valid-lifetime\": 4000 }";
@@ -724,10 +738,70 @@ TEST_F(Dhcp4ParserTest, optionDataLowerCase) {
     testOption(*range.first, 56, foo_expected, sizeof(foo_expected));
 }
 
+// Verify that specific option object is returned for standard
+// option which has dedicated option class derived from Option.
+TEST_F(Dhcp4ParserTest, stdOptionData) {
+    ConstElementPtr x;
+    std::map<std::string, std::string> params;
+    params["name"] = "nis-servers";
+    // Option code 41 means nis-servers.
+    params["code"] = "41";
+    // Specify option values in a CSV (user friendly) format.
+    params["data"] = "192.0.2.10, 192.0.2.1, 192.0.2.3";
+    params["csv-format"] = "True";
+
+    std::string config = createConfigWithOption(params);
+    ElementPtr json = Element::fromJSON(config);
+
+    EXPECT_NO_THROW(x = configureDhcp4Server(*srv_, json));
+    ASSERT_TRUE(x);
+    comment_ = parseAnswer(rcode_, x);
+    ASSERT_EQ(0, rcode_);
+
+    Subnet4Ptr subnet = CfgMgr::instance().getSubnet4(IOAddress("192.0.2.5"));
+    ASSERT_TRUE(subnet);
+    const Subnet::OptionContainer& options = subnet->getOptions();
+    ASSERT_EQ(1, options.size());
+
+    // Get the search index. Index #1 is to search using option code.
+    const Subnet::OptionContainerTypeIndex& idx = options.get<1>();
+
+    // Get the options for specified index. Expecting one option to be
+    // returned but in theory we may have multiple options with the same
+    // code so we get the range.
+    std::pair<Subnet::OptionContainerTypeIndex::const_iterator,
+              Subnet::OptionContainerTypeIndex::const_iterator> range =
+        idx.equal_range(DHO_NIS_SERVERS);
+    // Expect single option with the code equal to NIS_SERVERS option code.
+    ASSERT_EQ(1, std::distance(range.first, range.second));
+    // The actual pointer to the option is held in the option field
+    // in the structure returned.
+    OptionPtr option = range.first->option;
+    ASSERT_TRUE(option);
+    // Option object returned for here is expected to be Option6IA
+    // which is derived from Option. This class is dedicated to
+    // represent standard option IA_NA.
+    boost::shared_ptr<Option4AddrLst> option_addrs =
+        boost::dynamic_pointer_cast<Option4AddrLst>(option);
+    // If cast is unsuccessful than option returned was of a
+    // differnt type than Option6IA. This is wrong.
+    ASSERT_TRUE(option_addrs);
+
+    // Get addresses from the option.
+    Option4AddrLst::AddressContainer addrs = option_addrs->getAddresses();
+    // Verify that the addresses have been configured correctly.
+    ASSERT_EQ(3, addrs.size());
+    EXPECT_EQ("192.0.2.10", addrs[0].toText());
+    EXPECT_EQ("192.0.2.1", addrs[1].toText());
+    EXPECT_EQ("192.0.2.3", addrs[2].toText());
+}
+
 /// This test checks if Uint32Parser can really parse the whole range
 /// and properly err of out of range values. As we can't call Uint32Parser
 /// directly, we are exploiting the fact that it is used to parse global
 /// parameter renew-timer and the results are stored in uint32_defaults.
+/// We get the uint32_defaults using a getUint32Defaults functions which
+/// is defined only to access the values from this test.
 TEST_F(Dhcp4ParserTest, DISABLED_Uint32Parser) {
 
     ConstElementPtr status;
diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc
index 07aa7ec..8e9e065 100644
--- a/src/bin/dhcp6/config_parser.cc
+++ b/src/bin/dhcp6/config_parser.cc
@@ -858,9 +858,14 @@ private:
         std::vector<uint8_t> binary;
         std::vector<std::string> data_tokens;
         if (csv_format) {
+            // If the option data is specified as a string of comma
+            // separated values then we need to split this string into
+            // individual values - each value will be used to initialize
+            // one data field of an option.
             data_tokens = isc::util::str::tokens(option_data, ",");
         } else {
-            // Transform string of hexadecimal digits into binary format.
+            // Otherwise, the option data is specified as a string of
+            // hexadecimal digits that we have to turn into binary format.
             try {
                 isc::util::encode::decodeHex(option_data, binary);
             } catch (...) {
diff --git a/src/bin/dhcp6/tests/config_parser_unittest.cc b/src/bin/dhcp6/tests/config_parser_unittest.cc
index 3aa25f6..9774270 100644
--- a/src/bin/dhcp6/tests/config_parser_unittest.cc
+++ b/src/bin/dhcp6/tests/config_parser_unittest.cc
@@ -763,7 +763,6 @@ TEST_F(Dhcp6ParserTest, stdOptionData) {
     EXPECT_NO_THROW(x = configureDhcp6Server(srv_, json));
     ASSERT_TRUE(x);
     comment_ = parseAnswer(rcode_, x);
-    std::cout << comment_->str() << std::endl;
     ASSERT_EQ(0, rcode_);
 
     Subnet6Ptr subnet = CfgMgr::instance().getSubnet6(IOAddress("2001:db8:1::5"));
diff --git a/src/lib/dhcpsrv/subnet.cc b/src/lib/dhcpsrv/subnet.cc
index c2e5ed1..c23ecda 100644
--- a/src/lib/dhcpsrv/subnet.cc
+++ b/src/lib/dhcpsrv/subnet.cc
@@ -77,8 +77,8 @@ void Subnet4::addPool4(const Pool4Ptr& pool) {
 
     if (!inRange(first_addr) || !inRange(last_addr)) {
         isc_throw(BadValue, "Pool4 (" << first_addr.toText() << "-" << last_addr.toText()
-                  << " does not belong in this (" << prefix_ << "/" << prefix_len_
-                  << ") subnet4");
+                  << " does not belong in this (" << prefix_.toText() << "/"
+                  << static_cast<int>(prefix_len_) << ") subnet4");
     }
 
     /// @todo: Check that pools do not overlap



More information about the bind10-changes mailing list