[2410] Merge branch 'master' into trac2410

ChangeLog

@@ -1,3 +1,16 @@
+510.	[func]		marcin
+	DHCP option instances can be created using a collection of strings.
+	Each string represents a value of a particular data field within
+	an option. The data field values, given as strings, are validated
+	against the actual types of option fields specified in the options
+	definitions.
+	(Trac #2490, git 56cfd6612fcaeae9acec4a94e1e5f1a88142c44d)
+
+509.	[func]		muks
+	Log messages now include the pid of the process that logged the
+	message.
+	(Trac #1745, git fc8bbf3d438e8154e7c2bdd322145a7f7854dc6a)
+
 508.	[bug]		stephen
 	Split the DHCP library into two directories, each with its own
 	Makefile.  This properly solves the problem whereby a "make"

src/bin/dhcp6/config_parser.cc

@@ -698,11 +698,8 @@ private:
             // We have exactly one option definition for the particular option code
             // use it to create the option instance.
             const OptionDefinitionPtr& def = *(range.first);
-            // getFactory should never return NULL pointer.
-            Option::Factory* factory = def->getFactory();
-            assert(factory != NULL);
             try {
-                OptionPtr option = factory(Option::V6, option_code, binary);
+                OptionPtr option = def->optionFactory(Option::V6, option_code, binary);
                 Subnet::OptionDescriptor desc(option, false);
                 option_descriptor_.option = option;
                 option_descriptor_.persistent = false;

src/bin/dhcp6/dhcp6_srv.cc

@@ -56,12 +56,6 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port, const char* dbconfig)
 
     // Initialize objects required for DHCP server operation.
     try {
-        // Initialize standard DHCPv6 option definitions. This function
-        // may throw bad_alloc if system goes out of memory during the
-        // creation if option definitions. It may also throw isc::Unexpected
-        // if definitions are wrong. This would mean error in implementation.
-        initStdOptionDefs();
-
         // Port 0 is used for testing purposes. It means that the server should
         // not open any sockets at all. Some tests, e.g. configuration parser,
         // require Dhcpv6Srv object, but they don't really need it to do
@@ -622,10 +616,5 @@ Dhcpv6Srv::serverReceivedPacketName(uint8_t type) {
     return (UNKNOWN);
 }
 
-void
-Dhcpv6Srv::initStdOptionDefs() {
-    LibDHCP::initStdOptionDefs(Option::V6);
-}
-
 };
 };

src/bin/dhcp6/dhcp6_srv.h

@@ -241,17 +241,6 @@ protected:
     ///         interfaces for new DUID generation are detected.
     void setServerID();
 
-    /// @brief Initializes option definitions for standard options.
-    ///
-    /// Each standard option's format is described by the
-    /// dhcp::OptionDefinition object. This function creates such objects
-    /// for each standard DHCPv6 option.
-    ///
-    /// @todo list thrown exceptions.
-    /// @todo extend this function to cover all standard options. Currently
-    /// it is limited to critical options only.
-    void initStdOptionDefs();
-
 private:
     /// @brief Allocation Engine.
     /// Pointer to the allocation engine that we are currently using

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -46,11 +46,6 @@ public:
         // srv_(0) means to not open any sockets. We don't want to
         // deal with sockets here, just check if configuration handling
         // is sane.
-
-        // Create instances of option definitions and put them into storage.
-        // This is normally initialized by the server when calling run()
-        // run() function.
-        LibDHCP::initStdOptionDefs(Option::V6);
     }
 
     ~Dhcp6ParserTest() {

src/lib/asiolink/io_address.cc

@@ -61,7 +61,7 @@ IOAddress::toText() const {
 }
 
 IOAddress
-IOAddress::from_bytes(short family, const uint8_t* data) {
+IOAddress::fromBytes(short family, const uint8_t* data) {
     if (data == NULL) {
         isc_throw(BadValue, "NULL pointer received.");
     } else

src/lib/asiolink/io_address.h

@@ -111,7 +111,7 @@ public:
     ///
     /// \return Created IOAddress object
     static IOAddress
-    from_bytes(short family, const uint8_t* data);
+    fromBytes(short family, const uint8_t* data);
 
     /// \brief Compare addresses for equality
     ///

src/lib/asiolink/tests/io_address_unittest.cc

@@ -64,7 +64,7 @@ TEST(IOAddressTest, Family) {
     EXPECT_EQ(AF_INET6, IOAddress("2001:0DB8:0:0::0012").getFamily());
 }
 
-TEST(IOAddressTest, from_bytes) {
+TEST(IOAddressTest, fromBytes) {
     // 2001:db8:1::dead:beef
     uint8_t v6[] = {
         0x20, 0x01, 0x0d, 0xb8, 0x00, 0x01, 0, 0,
@@ -74,12 +74,12 @@ TEST(IOAddressTest, from_bytes) {
 
     IOAddress addr("::");
     EXPECT_NO_THROW({
-        addr = IOAddress::from_bytes(AF_INET6, v6);
+        addr = IOAddress::fromBytes(AF_INET6, v6);
     });
     EXPECT_EQ("2001:db8:1::dead:beef", addr.toText());
 
     EXPECT_NO_THROW({
-        addr = IOAddress::from_bytes(AF_INET, v4);
+        addr = IOAddress::fromBytes(AF_INET, v4);
     });
     EXPECT_EQ(addr.toText(), IOAddress("192.0.2.3").toText());
 }

src/lib/datasrc/Makefile.am

@@ -36,6 +36,8 @@ libb10_datasrc_la_SOURCES += database.h database.cc
 libb10_datasrc_la_SOURCES += factory.h factory.cc
 libb10_datasrc_la_SOURCES += client_list.h client_list.cc
 libb10_datasrc_la_SOURCES += memory_datasrc.h memory_datasrc.cc
+libb10_datasrc_la_SOURCES += master_loader_callbacks.h
+libb10_datasrc_la_SOURCES += master_loader_callbacks.cc
 nodist_libb10_datasrc_la_SOURCES = datasrc_messages.h datasrc_messages.cc
 libb10_datasrc_la_LDFLAGS = -no-undefined -version-info 1:0:1
 

src/lib/datasrc/datasrc_messages.mes

@@ -315,6 +315,18 @@ An error was found in the zone data when it was being loaded from
 another data source. The zone was not loaded. The specific error is
 shown in the message, and should be addressed.
 
+% DATASRC_MASTER_LOAD_ERROR %1:%2: Zone '%3/%4' contains error: %5
+There's an error in the given master file. The zone won't be loaded for
+this reason. Parsing might follow, so you might get further errors and
+warnings to fix everything at once. But in case the error is serious enough,
+the parser might just give up or get confused and generate false errors
+afterwards.
+
+% DATASRC_MASTER_LOAD_WARN %1:%2: Zone '%3/%4' has a potential problem: %5
+There's something suspicious in the master file. This is a warning only.
+It may be a problem or it may be harmless, but it should be checked.
+This problem does not stop the zone from being loaded.
+
 % DATASRC_MEM_ADD_RRSET adding RRset '%1/%2' into zone '%3'
 Debug information. An RRset is being added to the in-memory data source.
 

src/lib/datasrc/master_loader_callbacks.cc

@@ -0,0 +1,73 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/master_loader_callbacks.h>
+#include <datasrc/zone.h>
+#include <datasrc/logger.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+
+#include <string>
+#include <boost/bind.hpp>
+
+namespace isc {
+namespace datasrc {
+
+namespace {
+
+void
+logError(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
+         bool* ok, const std::string& source, size_t line,
+         const std::string& reason)
+{
+    LOG_ERROR(logger, DATASRC_MASTER_LOAD_ERROR).arg(source).arg(line).
+        arg(name).arg(rrclass).arg(reason);
+    if (ok != NULL) {
+        *ok = false;
+    }
+}
+
+void
+logWarning(const isc::dns::Name& name, const isc::dns::RRClass& rrclass,
+         const std::string& source, size_t line, const std::string& reason)
+{
+    LOG_WARN(logger, DATASRC_MASTER_LOAD_WARN).arg(source).arg(line).
+        arg(name).arg(rrclass).arg(reason);
+}
+
+}
+
+isc::dns::MasterLoaderCallbacks
+createMasterLoaderCallbacks(const isc::dns::Name& name,
+                            const isc::dns::RRClass& rrclass, bool* ok)
+{
+    return (isc::dns::MasterLoaderCallbacks(boost::bind(&logError, name,
+                                                        rrclass, ok, _1, _2,
+                                                        _3),
+                                            boost::bind(&logWarning, name,
+                                                        rrclass, _1, _2, _3)));
+}
+
+isc::dns::AddRRsetCallback
+createMasterLoaderAddCallback(ZoneUpdater& updater) {
+    return (boost::bind(&ZoneUpdater::addRRset, &updater,
+                        // The callback provides a shared pointer, we
+                        // need the object. This bind unpacks the object.
+                        boost::bind(&isc::dns::RRsetPtr::operator*, _1)));
+}
+
+}
+}

src/lib/datasrc/master_loader_callbacks.h

@@ -0,0 +1,67 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DATASRC_MASTER_LOADER_CALLBACKS_H
+#define DATASRC_MASTER_LOADER_CALLBACKS_H
+
+#include <dns/master_loader_callbacks.h>
+
+namespace isc {
+namespace dns {
+class Name;
+class RRClass;
+}
+namespace datasrc {
+
+class ZoneUpdater;
+
+/// \brief Create issue callbacks for MasterLoader
+///
+/// This will create set of callbacks for the MasterLoader that
+/// will be used to report any issues found in the zone data.
+///
+/// \param name Name of the zone. Used in logging.
+/// \param rrclass The class of the zone. Used in logging.
+/// \param ok If this is non-NULL and there are any errors during
+///     the loading, it is set to false. Otherwise, it is untouched.
+/// \return Set of callbacks to be passed to the master loader.
+/// \throw std::bad_alloc when allocation fails.
+isc::dns::MasterLoaderCallbacks
+createMasterLoaderCallbacks(const isc::dns::Name& name,
+                            const isc::dns::RRClass& rrclass, bool* ok);
+
+/// \brief Create a callback for MasterLoader to add RRsets.
+///
+/// This creates a callback that can be used by the MasterLoader to add
+/// loaded RRsets into a zone updater.
+///
+/// The zone updater should be opened in the replace mode no changes should
+/// have been done to it yet (but it is not checked). It is not commited
+/// automatically and it is up to the caller to commit the changes (or not).
+/// It must not be destroyed for the whole time of loading.
+///
+/// The function is mostly straightforward packing of the updater.addRRset
+/// into a boost::function, it is defined explicitly due to small technical
+/// annoyences around boost::bind application, so it can be reused.
+///
+/// \param updater The zone updater to use.
+/// \return The callback to be passed to MasterLoader.
+/// \throw std::bad_alloc when allocation fails.
+isc::dns::AddRRsetCallback
+createMasterLoaderAddCallback(ZoneUpdater& updater);
+
+}
+}
+
+#endif

src/lib/datasrc/tests/Makefile.am

@@ -59,6 +59,7 @@ run_unittests_SOURCES += zonetable_unittest.cc
 run_unittests_SOURCES += zone_finder_context_unittest.cc
 run_unittests_SOURCES += faked_nsec3.h faked_nsec3.cc
 run_unittests_SOURCES += client_list_unittest.cc
+run_unittests_SOURCES += master_loader_callbacks_test.cc
 
 # We need the actual module implementation in the tests (they are not part
 # of libdatasrc)

src/lib/datasrc/tests/master_loader_callbacks_test.cc

@@ -0,0 +1,124 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <datasrc/master_loader_callbacks.h>
+#include <datasrc/zone.h>
+
+#include <dns/rrset.h>
+#include <dns/rrclass.h>
+#include <dns/rrttl.h>
+
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/lexical_cast.hpp>
+
+#include <list>
+#include <string>
+
+using namespace isc::datasrc;
+
+namespace {
+
+// An updater for the tests. Most of the virtual methods throw
+// NotImplemented, as they are not used in the tests.
+class MockUpdater : public ZoneUpdater {
+public:
+    // We do the adding in this test. We currently only check these are
+    // the correct ones, according to a predefined set in a list.
+    virtual void addRRset(const isc::dns::AbstractRRset& rrset) {
+        ASSERT_FALSE(expected_rrsets_.empty());
+        // In our tests, pointer equality is enough.
+        EXPECT_EQ(expected_rrsets_.front().get(), &rrset);
+        // And remove this RRset, as it has been used.
+        expected_rrsets_.pop_front();
+    }
+    // The unused but required methods
+    virtual ZoneFinder& getFinder() {
+        isc_throw(isc::NotImplemented, "Not to be called in this test");
+    }
+    virtual void deleteRRset(const isc::dns::AbstractRRset&) {
+        isc_throw(isc::NotImplemented, "Not to be called in this test");
+    }
+    virtual void commit() {
+        isc_throw(isc::NotImplemented, "Not to be called in this test");
+    }
+    // The RRsets that are expected to appear through addRRset.
+    std::list<isc::dns::RRsetPtr> expected_rrsets_;
+};
+
+class MasterLoaderCallbackTest : public ::testing::Test {
+protected:
+    MasterLoaderCallbackTest() :
+        ok_(true),
+        callbacks_(createMasterLoaderCallbacks(isc::dns::Name("example.org"),
+                                               isc::dns::RRClass::IN(), &ok_))
+    {}
+    // Generate a new RRset, put it to the updater and return it.
+    isc::dns::RRsetPtr generateRRset() {
+        const isc::dns::RRsetPtr
+            result(new isc::dns::RRset(isc::dns::Name("example.org"),
+                                       isc::dns::RRClass::IN(),
+                                       isc::dns::RRType::A(),
+                                       isc::dns::RRTTL(3600)));
+        updater_.expected_rrsets_.push_back(result);
+        return (result);
+    }
+    // An updater to be passed to the context
+    MockUpdater updater_;
+    // Is the loading OK?
+    bool ok_;
+    // The tested context
+    isc::dns::MasterLoaderCallbacks callbacks_;
+};
+
+// Check it doesn't crash if we don't provide the OK
+TEST_F(MasterLoaderCallbackTest, noOkProvided) {
+    createMasterLoaderCallbacks(isc::dns::Name("example.org"),
+                                isc::dns::RRClass::IN(), NULL).
+        error("No source", 1, "No reason");
+}
+
+// Check the callbacks can be called, don't crash and the error one switches
+// to non-OK mode. This, however, does not stop anybody from calling more
+// callbacks.
+TEST_F(MasterLoaderCallbackTest, callbacks) {
+    EXPECT_NO_THROW(callbacks_.warning("No source", 1, "Just for fun"));
+    // The warning does not hurt the OK mode.
+    EXPECT_TRUE(ok_);
+    // Now the error
+    EXPECT_NO_THROW(callbacks_.error("No source", 2, "Some error"));
+    // The OK is turned off once there's at least one error
+    EXPECT_FALSE(ok_);
+
+    // Not being OK does not hurt that much, we can still call the callbacks
+    EXPECT_NO_THROW(callbacks_.warning("No source", 3, "Just for fun"));
+    // The OK is not reset back to true
+    EXPECT_FALSE(ok_);
+    EXPECT_NO_THROW(callbacks_.error("No source", 4, "Some error"));
+}
+
+// Try adding some RRsets.
+TEST_F(MasterLoaderCallbackTest, addRRset) {
+    isc::dns::AddRRsetCallback
+        callback(createMasterLoaderAddCallback(updater_));
+    // Put some of them in.
+    EXPECT_NO_THROW(callback(generateRRset()));
+    EXPECT_NO_THROW(callback(generateRRset()));
+    // They all get pushed there right away, so there are none in the queue
+    EXPECT_TRUE(updater_.expected_rrsets_.empty());
+}
+
+}

src/lib/dhcp/iface_mgr.cc

@@ -1098,9 +1098,9 @@ Pkt6Ptr IfaceMgr::receive6(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
 
     pkt->updateTimestamp();
 
-    pkt->setLocalAddr(IOAddress::from_bytes(AF_INET6,
+    pkt->setLocalAddr(IOAddress::fromBytes(AF_INET6,
                       reinterpret_cast<const uint8_t*>(&to_addr)));
-    pkt->setRemoteAddr(IOAddress::from_bytes(AF_INET6,
+    pkt->setRemoteAddr(IOAddress::fromBytes(AF_INET6,
                        reinterpret_cast<const uint8_t*>(&from.sin6_addr)));
     pkt->setRemotePort(ntohs(from.sin6_port));
     pkt->setIndex(ifindex);

src/lib/dhcp/iface_mgr_linux.cc

@@ -302,7 +302,7 @@ void Netlink::ipaddrs_get(IfaceMgr::Iface& iface, NetlinkMessages& addr_info) {
 
             memcpy(addr, RTA_DATA(rta_tb[IFLA_ADDRESS]),
                    ifa->ifa_family==AF_INET?V4ADDRESS_LEN:V6ADDRESS_LEN);
-            IOAddress a = IOAddress::from_bytes(ifa->ifa_family, addr);
+            IOAddress a = IOAddress::fromBytes(ifa->ifa_family, addr);
             iface.addAddress(a);
 
             /// TODO: Read lifetimes of configured IPv6 addresses

src/lib/dhcp/libdhcp++.cc

@@ -48,8 +48,12 @@ const OptionDefContainer&
 LibDHCP::getOptionDefs(Option::Universe u) {
     switch (u) {
     case Option::V4:
+        initStdOptionDefs4();
         return (v4option_defs_);
     case Option::V6:
+        if (v6option_defs_.size() == 0) {
+            initStdOptionDefs6();
+        }
         return (v6option_defs_);
     default:
         isc_throw(isc::BadValue, "invalid universe " << u << " specified");
@@ -96,30 +100,43 @@ size_t LibDHCP::unpackOptions6(const OptionBuffer& buf,
             // @todo: consider throwing exception here.
             return (offset);
         }
+
+        // Get the list of stdandard option definitions.
+        OptionDefContainer option_defs = LibDHCP::getOptionDefs(Option::V6);
+        // Get the search index #1. It allows to search for option definitions
+        // using option code.
+        const OptionDefContainerTypeIndex& idx = option_defs.get<1>();
+        // Get all options with the particular option code. Note that option code
+        // is non-unique within this container however at this point we expect
+        // to get one option definition with the particular code. If more are
+        // returned we report an error.
+        const OptionDefContainerTypeRange& range = idx.equal_range(opt_type);
+        // Get the number of returned option definitions for the option code.
+        size_t num_defs = distance(range.first, range.second);
         OptionPtr opt;
-        switch (opt_type) {
-        case D6O_IA_NA:
-        case D6O_IA_PD:
-            opt = OptionPtr(new Option6IA(opt_type,
-                                          buf.begin() + offset,
-                                          buf.begin() + offset + opt_len));
-            break;
-        case D6O_IAADDR:
-            opt = OptionPtr(new Option6IAAddr(opt_type,
-                                              buf.begin() + offset,
-                                              buf.begin() + offset + opt_len));
-            break;
-        case D6O_ORO:
-            opt = OptionPtr(new Option6IntArray<uint16_t>(opt_type,
-                                                          buf.begin() + offset,
-                                                          buf.begin() + offset + opt_len));
-            break;
-        default:
-            opt = OptionPtr(new Option(Option::V6,
-                                       opt_type,
+        if (num_defs > 1) {
+            // Multiple options of the same code are not supported right now!
+            isc_throw(isc::Unexpected, "Internal error: multiple option definitions"
+                      " for option type " << opt_type << " returned. Currently it is not"
+                      " supported to initialize multiple option definitions"
+                      " for the same option code. This will be supported once"
+                      " support for option spaces is implemented");
+        } else if (num_defs == 0) {
+            // @todo Don't crash if definition does not exist because only a few
+            // option definitions are initialized right now. In the future
+            // we will initialize definitions for all options and we will
+            // remove this elseif. For now, return generic option.
+            opt = OptionPtr(new Option(Option::V6, opt_type,
                                        buf.begin() + offset,
                                        buf.begin() + offset + opt_len));
-            break;
+        } else {
+            // The option definition has been found. Use it to create
+            // the option instance from the provided buffer chunk.
+            const OptionDefinitionPtr& def = *(range.first);
+            assert(def);
+            opt = def->optionFactory(Option::V6, opt_type,
+                                     buf.begin() + offset,
+                                     buf.begin() + offset + opt_len);
         }
         // add option to options
         options.insert(std::make_pair(opt_type, opt));
@@ -229,20 +246,6 @@ void LibDHCP::OptionFactoryRegister(Option::Universe u,
 }
 
 void
-LibDHCP::initStdOptionDefs(Option::Universe u) {
-    switch (u) {
-    case Option::V4:
-        initStdOptionDefs4();
-        break;
-    case Option::V6:
-        initStdOptionDefs6();
-        break;
-    default:
-        isc_throw(isc::BadValue, "invalid universe " << u << " specified");
-    }
-}
-
-void
 LibDHCP::initStdOptionDefs4() {
     isc_throw(isc::NotImplemented, "initStdOptionDefs4 is not implemented");
 }
@@ -254,19 +257,20 @@ LibDHCP::initStdOptionDefs6() {
     struct OptionParams {
         std::string name;
         uint16_t code;
-        OptionDefinition::DataType type;
+        OptionDataType type;
         bool array;
     };
     OptionParams params[] = {
-        { "CLIENTID", D6O_CLIENTID, OptionDefinition::BINARY_TYPE, false },
-        { "SERVERID", D6O_SERVERID, OptionDefinition::BINARY_TYPE, false },
-        { "IA_NA", D6O_IA_NA, OptionDefinition::RECORD_TYPE, false },
-        { "IAADDR", D6O_IAADDR, OptionDefinition::RECORD_TYPE, false },
-        { "ORO", D6O_ORO, OptionDefinition::UINT16_TYPE, true },
-        { "ELAPSED_TIME", D6O_ELAPSED_TIME, OptionDefinition::UINT16_TYPE, false },
-        { "STATUS_CODE", D6O_STATUS_CODE, OptionDefinition::RECORD_TYPE, false },
-        { "RAPID_COMMIT", D6O_RAPID_COMMIT, OptionDefinition::EMPTY_TYPE, false },
-        { "DNS_SERVERS", D6O_NAME_SERVERS, OptionDefinition::IPV6_ADDRESS_TYPE, true }
+        { "CLIENTID", D6O_CLIENTID, OPT_BINARY_TYPE, false },
+        { "SERVERID", D6O_SERVERID, OPT_BINARY_TYPE, false },
+        { "IA_NA", D6O_IA_NA, OPT_RECORD_TYPE, false },
+        { "IAADDR", D6O_IAADDR, OPT_RECORD_TYPE, false },
+        { "ORO", D6O_ORO, OPT_UINT16_TYPE, true },
+        { "ELAPSED_TIME", D6O_ELAPSED_TIME, OPT_UINT16_TYPE, false },
+        { "STATUS_CODE", D6O_STATUS_CODE, OPT_RECORD_TYPE, false },
+        { "RAPID_COMMIT", D6O_RAPID_COMMIT, OPT_EMPTY_TYPE, false },
+        { "DNS_SERVERS", D6O_NAME_SERVERS, OPT_IPV6_ADDRESS_TYPE, true },
+        { "IA_PD", D6O_IA_PD, OPT_RECORD_TYPE, false }
     };
     const int params_size = sizeof(params) / sizeof(params[0]);
 
@@ -277,18 +281,19 @@ LibDHCP::initStdOptionDefs6() {
                                                             params[i].array));
         switch(params[i].code) {
         case D6O_IA_NA:
+        case D6O_IA_PD:
             for (int j = 0; j < 3; ++j) {
-                definition->addRecordField(OptionDefinition::UINT32_TYPE);
+                definition->addRecordField(OPT_UINT32_TYPE);
             }
             break;
         case D6O_IAADDR:
-            definition->addRecordField(OptionDefinition::IPV6_ADDRESS_TYPE);
-            definition->addRecordField(OptionDefinition::UINT32_TYPE);
-            definition->addRecordField(OptionDefinition::UINT32_TYPE);
+            definition->addRecordField(OPT_IPV6_ADDRESS_TYPE);
+            definition->addRecordField(OPT_UINT32_TYPE);
+            definition->addRecordField(OPT_UINT32_TYPE);
             break;
         case D6O_STATUS_CODE:
-            definition->addRecordField(OptionDefinition::UINT16_TYPE);
-            definition->addRecordField(OptionDefinition::STRING_TYPE);
+            definition->addRecordField(OPT_UINT16_TYPE);
+            definition->addRecordField(OPT_STRING_TYPE);
             break;
         default:
             // The default case is intentionally left empty
@@ -298,9 +303,12 @@ LibDHCP::initStdOptionDefs6() {
         try {
             definition->validate();
         } catch (const Exception& ex) {
-            isc_throw(isc::Unexpected, "internal server error: invalid definition of standard"
-                      << " DHCPv6 option (with code " << params[i].code << "): "
-                      << ex.what());
+            // This is unlikely event that validation fails and may
+            // be only caused by programming error. To guarantee the
+            // data consistency we clear all option definitions that
+            // have been added so far and pass the exception forward.
+            v6option_defs_.clear();
+            throw;
         }
         v6option_defs_.push_back(definition);
     }

src/lib/dhcp/libdhcp++.h

@@ -33,7 +33,14 @@ public:
 
     /// @brief Return collection of option definitions.
     ///
+    /// Method returns the collection of DHCP standard DHCP
+    /// option definitions.
+    /// @todo DHCPv4 option definitions are not implemented. For now
+    /// this function will throw isc::NotImplemented in case of attempt
+    /// to get option definitions for V4 universe.
+    ///
     /// @param u universe of the options (V4 or V6).
+    ///
     /// @return collection of option definitions.
     static const OptionDefContainer& getOptionDefs(Option::Universe u);
 
@@ -113,21 +120,6 @@ public:
                                       uint16_t type,
                                       Option::Factory * factory);
 
-    /// Initialize standard DHCP options (V4 or V6).
-    ///
-    /// The method creates option definitions for all options
-    /// (DHCPv4 or DHCPv6 depending on universe specified).
-    /// Currently DHCPv4 option definitions initialization is not
-    /// implemented thus this function will throw isc::NotImplemented
-    /// if V4 universe is specified.
-    ///
-    /// @param u universe
-    /// @throw isc::Unexpected if internal error occured during option
-    /// definitions creation.
-    /// @throw std::bad_alloc if system went out of memory.
-    /// @throw isc::NotImplemented when V4 universe specified.
-    static void initStdOptionDefs(Option::Universe u);
-
 private:
 
     /// Initialize standard DHCPv4 option definitions.
@@ -144,9 +136,9 @@ private:
     ///
     /// The method creates option definitions for all DHCPv6 options.
     ///
-    /// @throw isc::Unexpected if internal error occured during option
-    /// definitions creation.
     /// @throw std::bad_alloc if system went out of memory.
+    /// @throw MalformedOptionDefinition if any of the definitions
+    /// is incorect. This is a programming error.
     static void initStdOptionDefs6();
 
     /// pointers to factories that produce DHCPv6 options

src/lib/dhcp/option6_addrlst.cc

@@ -84,7 +84,7 @@ void Option6AddrLst::unpack(OptionBufferConstIter begin,
                   << " is not divisible by 16.");
     }
     while (begin != end) {
-        addrs_.push_back(IOAddress::from_bytes(AF_INET6, &(*begin)));
+        addrs_.push_back(IOAddress::fromBytes(AF_INET6, &(*begin)));
         begin += V6ADDRESS_LEN;
     }
 }

src/lib/dhcp/option6_iaaddr.cc

@@ -69,7 +69,7 @@ void Option6IAAddr::unpack(OptionBuffer::const_iterator begin,
     }
 
     // 16 bytes: IPv6 address
-    addr_ = IOAddress::from_bytes(AF_INET6, &(*begin));
+    addr_ = IOAddress::fromBytes(AF_INET6, &(*begin));
     begin += V6ADDRESS_LEN;
 
     preferred_ = readUint32( &(*begin) );

src/lib/dhcp/option6_int.h

@@ -48,7 +48,7 @@ public:
     /// as template parameter is not a supported integer type.
     Option6Int(uint16_t type, T value)
         : Option(Option::V6, type), value_(value) {
-        if (!OptionDataTypes<T>::valid) {
+        if (!OptionDataTypeTraits<T>::integer_type) {
             isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
     }
@@ -69,7 +69,7 @@ public:
     Option6Int(uint16_t type, OptionBufferConstIter begin,
                OptionBufferConstIter end)
         : Option(Option::V6, type) {
-        if (!OptionDataTypes<T>::valid) {
+        if (!OptionDataTypeTraits<T>::integer_type) {
             isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         unpack(begin, end);
@@ -91,7 +91,7 @@ public:
         // order to the provided buffer. The same functions can be safely used
         // for either unsiged or signed integers so there is not need to create
         // special cases for intX_t types.
-        switch (OptionDataTypes<T>::len) {
+        switch (OptionDataTypeTraits<T>::len) {
         case 1:
             buf.writeUint8(value_);
             break;
@@ -130,7 +130,7 @@ public:
         // order from the provided buffer. The same functions can be safely used
         // for either unsiged or signed integers so there is not need to create
         // special cases for intX_t types.
-        int data_size_len = OptionDataTypes<T>::len;
+        int data_size_len = OptionDataTypeTraits<T>::len;
         switch (data_size_len) {
         case 1:
             value_ = *begin;
@@ -145,9 +145,9 @@ public:
             isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         // Use local variable to set a new value for this iterator.
-        // When using OptionDataTypes<T>::len directly some versions
+        // When using OptionDataTypeTraits<T>::len directly some versions
         // of clang complain about unresolved reference to
-        // OptionDataTypes structure during linking.
+        // OptionDataTypeTraits structure during linking.
         begin += data_size_len;
         LibDHCP::unpackOptions6(OptionBuffer(begin, end), options_);
     }

src/lib/dhcp/option6_int_array.h

@@ -58,7 +58,7 @@ public:
     Option6IntArray(uint16_t type)
         : Option(Option::V6, type),
           values_(0) {
-        if (!OptionDataTypes<T>::valid) {
+        if (!OptionDataTypeTraits<T>::integer_type) {
             isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
     }
@@ -74,7 +74,7 @@ public:
     /// as template parameter is not a supported integer type.
     Option6IntArray(uint16_t type, const OptionBuffer& buf)
         : Option(Option::V6, type) {
-        if (!OptionDataTypes<T>::valid) {
+        if (!OptionDataTypeTraits<T>::integer_type) {
             isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         unpack(buf.begin(), buf.end());
@@ -97,7 +97,7 @@ public:
     Option6IntArray(uint16_t type, OptionBufferConstIter begin,
                     OptionBufferConstIter end)
         : Option(Option::V6, type) {
-        if (!OptionDataTypes<T>::valid) {
+        if (!OptionDataTypeTraits<T>::integer_type) {
             isc_throw(dhcp::InvalidDataType, "non-integer type");
         }
         unpack(begin, end);
@@ -120,7 +120,7 @@ public:
             // order to the provided buffer. The same functions can be safely used
             // for either unsiged or signed integers so there is not need to create
             // special cases for intX_t types.
-            switch (OptionDataTypes<T>::len) {
+            switch (OptionDataTypeTraits<T>::len) {
             case 1:
                 buf.writeUint8(values_[i]);
                 break;
@@ -166,7 +166,7 @@ public:
             // order from the provided buffer. The same functions can be safely used
             // for either unsiged or signed integers so there is not need to create
             // special cases for intX_t types.
-            int data_size_len = OptionDataTypes<T>::len;
+            int data_size_len = OptionDataTypeTraits<T>::len;
             switch (data_size_len) {
             case 1:
                 values_.push_back(*begin);
@@ -181,9 +181,9 @@ public:
                 isc_throw(dhcp::InvalidDataType, "non-integer type");
             }
             // Use local variable to set a new value for this iterator.
-            // When using OptionDataTypes<T>::len directly some versions
+            // When using OptionDataTypeTraits<T>::len directly some versions
             // of clang complain about unresolved reference to
-            // OptionDataTypes structure during linking.
+            // OptionDataTypeTraits structure during linking.
             begin += data_size_len;
         }
         // We do not unpack sub-options here because we have array-type option.

src/lib/dhcp/option_data_types.h

@@ -15,6 +15,7 @@
 #ifndef OPTION_DATA_TYPES_H
 #define OPTION_DATA_TYPES_H
 
+#include <asiolink/io_address.h>
 #include <exceptions/exceptions.h>
 
 #include <stdint.h>
@@ -29,59 +30,147 @@ public:
         isc::Exception(file, line, what) { };
 };
 
-/// @brief Trait class for integer data types supported in DHCP option definitions.
+/// @brief Exception to be thrown when cast to the data type was unsuccessful.
+class BadDataTypeCast : public Exception {
+public:
+    BadDataTypeCast(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+
+/// @brief Data types of DHCP option fields.
+enum OptionDataType {
+    OPT_EMPTY_TYPE,
+    OPT_BINARY_TYPE,
+    OPT_BOOLEAN_TYPE,
+    OPT_INT8_TYPE,
+    OPT_INT16_TYPE,
+    OPT_INT32_TYPE,
+    OPT_UINT8_TYPE,
+    OPT_UINT16_TYPE,
+    OPT_UINT32_TYPE,
+    OPT_ANY_ADDRESS_TYPE,
+    OPT_IPV4_ADDRESS_TYPE,
+    OPT_IPV6_ADDRESS_TYPE,
+    OPT_STRING_TYPE,
+    OPT_FQDN_TYPE,
+    OPT_RECORD_TYPE,
+    OPT_UNKNOWN_TYPE
+};
+
+/// @brief Trait class for 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 {
+struct OptionDataTypeTraits {
     static const bool valid = false;
     static const int len = 0;
+    static const bool integer_type = false;
+    static const OptionDataType type = OPT_UNKNOWN_TYPE;
+};
+
+/// binary type is supported
+template<>
+struct OptionDataTypeTraits<OptionBuffer> {
+    static const bool valid = true;
+    static const int len = sizeof(OptionBuffer);
+    static const bool integer_type = false;
+    static const OptionDataType type = OPT_BINARY_TYPE;
+};
+
+/// bool type is supported
+template<>
+struct OptionDataTypeTraits<bool> {
+    static const bool valid = true;
+    static const int len = sizeof(bool);
+    static const bool integer_type = false;
+    static const OptionDataType type = OPT_BOOLEAN_TYPE;
 };
 
 /// int8_t type is supported.
 template<>
-struct OptionDataTypes<int8_t> {
+struct OptionDataTypeTraits<int8_t> {
     static const bool valid = true;
     static const int len = 1;
+    static const bool integer_type = true;
+    static const OptionDataType type = OPT_INT8_TYPE;
 };
 
 /// int16_t type is supported.
 template<>
-struct OptionDataTypes<int16_t> {
+struct OptionDataTypeTraits<int16_t> {
     static const bool valid = true;
     static const int len = 2;
+    static const bool integer_type = true;
+    static const OptionDataType type = OPT_INT16_TYPE;
 };
 
 /// int32_t type is supported.
 template<>
-struct OptionDataTypes<int32_t> {
+struct OptionDataTypeTraits<int32_t> {
     static const bool valid = true;
     static const int len = 4;
+    static const bool integer_type = true;
+    static const OptionDataType type = OPT_INT32_TYPE;
 };
 
 /// uint8_t type is supported.
 template<>
-struct OptionDataTypes<uint8_t> {
+struct OptionDataTypeTraits<uint8_t> {
     static const bool valid = true;
     static const int len = 1;
+    static const bool integer_type = true;
+    static const OptionDataType type = OPT_UINT8_TYPE;
 };
 
 /// uint16_t type is supported.
 template<>
-struct OptionDataTypes<uint16_t> {
+struct OptionDataTypeTraits<uint16_t> {
     static const bool valid = true;
     static const int len = 2;
+    static const bool integer_type = true;
+    static const OptionDataType type = OPT_UINT16_TYPE;
 };
 
 /// uint32_t type is supported.
 template<>
-struct OptionDataTypes<uint32_t> {
+struct OptionDataTypeTraits<uint32_t> {
     static const bool valid = true;
     static const int len = 4;
+    static const bool integer_type = true;
+    static const OptionDataType type = OPT_UINT32_TYPE;
+};
+
+/// IPv4 and IPv6 address type is supported
+template<>
+struct OptionDataTypeTraits<asiolink::IOAddress> {
+    static const bool valid = true;
+    // The len value is used to determine the size of the data
+    // to be written to an option buffer. IOAddress object may
+    // either represent an IPv4 or IPv6 addresses which have
+    // different lengths. Thus we can't put fixed value here.
+    // The length of a data to be written into an option buffer
+    // have to be determined in the runtime for a particular
+    // IOAddress object. Thus setting len to zero.
+    static const int len = 0;
+    static const bool integer_type = false;
+    static const OptionDataType type = OPT_ANY_ADDRESS_TYPE;
 };
 
+/// string type is supported
+template<>
+struct OptionDataTypeTraits<std::string> {
+    static const bool valid = true;
+    // The len value is used to determine the size of the data
+    // to be written to an option buffer. For strings this
+    // size is unknown until we actually deal with the particular
+    // string to be written. Thus setting it to zero.
+    static const int len = 0;
+    static const bool integer_type = false;
+    static const OptionDataType type = OPT_STRING_TYPE;
+};
 
 } // isc::dhcp namespace
 } // isc namespace

src/lib/dhcp/option_definition.cc

@@ -20,6 +20,7 @@
 #include <dhcp/option6_int.h>
 #include <dhcp/option6_int_array.h>
 #include <dhcp/option_definition.h>
+#include <util/encode/hex.h>
 
 using namespace std;
 using namespace isc::util;
@@ -28,30 +29,212 @@ namespace isc {
 namespace dhcp {
 
 OptionDefinition::DataTypeUtil::DataTypeUtil() {
-    data_types_["empty"] = EMPTY_TYPE;
-    data_types_["binary"] = BINARY_TYPE;
-    data_types_["boolean"] = BOOLEAN_TYPE;
-    data_types_["int8"] = INT8_TYPE;
-    data_types_["int16"] = INT16_TYPE;
-    data_types_["int32"] = INT32_TYPE;
-    data_types_["uint8"] = UINT8_TYPE;
-    data_types_["uint16"] = UINT16_TYPE;
-    data_types_["uint32"] = UINT32_TYPE;
-    data_types_["ipv4-address"] = IPV4_ADDRESS_TYPE;
-    data_types_["ipv6-address"] = IPV6_ADDRESS_TYPE;
-    data_types_["string"] = STRING_TYPE;
-    data_types_["fqdn"] = FQDN_TYPE;
-    data_types_["record"] = RECORD_TYPE;
+    data_types_["empty"] = OPT_EMPTY_TYPE;
+    data_types_["binary"] = OPT_BINARY_TYPE;
+    data_types_["boolean"] = OPT_BOOLEAN_TYPE;
+    data_types_["int8"] = OPT_INT8_TYPE;
+    data_types_["int16"] = OPT_INT16_TYPE;
+    data_types_["int32"] = OPT_INT32_TYPE;
+    data_types_["uint8"] = OPT_UINT8_TYPE;
+    data_types_["uint16"] = OPT_UINT16_TYPE;
+    data_types_["uint32"] = OPT_UINT32_TYPE;
+    data_types_["ipv4-address"] = OPT_IPV4_ADDRESS_TYPE;
+    data_types_["ipv6-address"] = OPT_IPV6_ADDRESS_TYPE;
+    data_types_["string"] = OPT_STRING_TYPE;
+    data_types_["fqdn"] = OPT_FQDN_TYPE;
+    data_types_["record"] = OPT_RECORD_TYPE;
 }
 
-OptionDefinition::DataType
-OptionDefinition::DataTypeUtil::getDataType(const std::string& data_type) {
-    std::map<std::string, DataType>::const_iterator data_type_it =
+OptionDataType
+OptionDefinition::DataTypeUtil::getOptionDataType(const std::string& data_type) {
+    std::map<std::string, OptionDataType>::const_iterator data_type_it =
         data_types_.find(data_type);
     if (data_type_it != data_types_.end()) {
         return (data_type_it->second);
     }
-    return UNKNOWN_TYPE;
+    return (OPT_UNKNOWN_TYPE);
+}
+
+template<typename T>
+T OptionDefinition::DataTypeUtil::lexicalCastWithRangeCheck(const std::string& value_str) const {
+    // Lexical cast in case of our data types make sense only
+    // for uintX_t, intX_t and bool type.
+    if (!OptionDataTypeTraits<T>::integer_type &&
+        OptionDataTypeTraits<T>::type != OPT_BOOLEAN_TYPE) {
+        isc_throw(BadDataTypeCast, "unable to do lexical cast to non-integer and"
+                  << " non-boolean data type");
+    }
+    // We use the 64-bit value here because it has wider range than
+    // any other type we use here and it allows to detect out of
+    // bounds conditions e.g. negative value specified for uintX_t
+    // data type. Obviously if the value exceeds the limits of int64
+    // this function will not handle that properly.
+    int64_t result = 0;
+    try {
+        result = boost::lexical_cast<int64_t>(value_str);
+    } catch (const boost::bad_lexical_cast& ex) {
+        // Prepare error message here.
+        std::string data_type_str = "boolean";
+        if (OptionDataTypeTraits<T>::integer_type) {
+            data_type_str = "integer";
+        }
+        isc_throw(BadDataTypeCast, "unable to do lexical cast to " << data_type_str
+                  << " data type for value " << value_str << ": " << ex.what());
+    }
+    // Perform range checks for integer values only (exclude bool values).
+    if (OptionDataTypeTraits<T>::integer_type) {
+        if (result > numeric_limits<T>::max() ||
+            result < numeric_limits<T>::min()) {
+            isc_throw(BadDataTypeCast, "unable to do lexical cast for value "
+                      << value_str << ". This value is expected to be in the range of "
+                      << numeric_limits<T>::min() << ".." << numeric_limits<T>::max());
+        }
+    }
+    return (static_cast<T>(result));
+}
+
+void
+OptionDefinition::DataTypeUtil::writeToBuffer(const std::string& value,
+                                              const OptionDataType type,
+                                              OptionBuffer& buf) {
+    // We are going to write value given by value argument to the buffer.
+    // The actual type of the value is given by second argument. Check
+    // this argument to determine how to write this value to the buffer.
+    switch (type) {
+    case OPT_BINARY_TYPE:
+        {
+            // Binary value means that the value is encoded as a string
+            // of hexadecimal deigits. We need to decode this string
+            // to the binary format here.
+            OptionBuffer binary;
+            try {
+                util::encode::decodeHex(value, binary);
+            } catch (const Exception& ex) {
+                isc_throw(BadDataTypeCast, "unable to cast " << value
+                          << " to binary data type: " << ex.what());
+            }
+            // Decode was successful so append decoded binary value
+            // to the buffer.
+            buf.insert(buf.end(), binary.begin(), binary.end());
+            return;
+        }
+    case OPT_BOOLEAN_TYPE:
+        {
+            // We encode the true value as 1 and false as 0 on 8 bits.
+            // That way we actually waist 7 bits but it seems to be the
+            // simpler way to encode boolean.
+            // @todo Consider if any other encode methods can be used.
+            bool bool_value = lexicalCastWithRangeCheck<bool>(value);
+            if (bool_value) {
+                buf.push_back(static_cast<uint8_t>(1));
+            } else {
+                buf.push_back(static_cast<uint8_t>(0));
+            }
+            return;
+        }
+    case OPT_INT8_TYPE:
+        {
+            // Buffer holds the uin8_t values so we need to cast the signed
+            // value to unsigned but the bits values remain untouched.
+            buf.push_back(static_cast<uint8_t>(lexicalCastWithRangeCheck<int8_t>(value)));
+            return;
+        }
+    case OPT_INT16_TYPE:
+        {
+            // Write the int16 value as uint16 value is ok because the bit values
+            // remain untouched.
+            int16_t int_value = lexicalCastWithRangeCheck<int16_t>(value);
+            buf.resize(buf.size() + 2);
+            writeUint16(static_cast<uint16_t>(int_value), &buf[buf.size() - 2]);
+            return;
+        }
+    case OPT_INT32_TYPE:
+        {
+            int32_t int_value = lexicalCastWithRangeCheck<int32_t>(value);
+            buf.resize(buf.size() + 4);
+            writeUint32(static_cast<uint32_t>(int_value), &buf[buf.size() - 4]);
+            return;
+        }
+    case OPT_UINT8_TYPE:
+        {
+            buf.push_back(lexicalCastWithRangeCheck<uint8_t>(value));
+            return;
+        }
+    case OPT_UINT16_TYPE:
+        {
+            uint16_t uint_value = lexicalCastWithRangeCheck<uint16_t>(value);
+            buf.resize(buf.size() + 2);
+            writeUint16(uint_value, &buf[buf.size() - 2]);
+            return;
+        }
+    case OPT_UINT32_TYPE:
+        {
+            uint32_t uint_value = lexicalCastWithRangeCheck<uint32_t>(value);
+            buf.resize(buf.size() + 4);
+            writeUint32(uint_value, &buf[buf.size() - 4]);
+            return;
+        }
+    case OPT_IPV4_ADDRESS_TYPE:
+        {
+            // The easiest way to get the binary form of IPv4 address is
+            // to create IOAddress object from string and use its accessors
+            // to retrieve the binary form.
+            asiolink::IOAddress address(value);
+            if (!address.getAddress().is_v4()) {
+                isc_throw(BadDataTypeCast, "provided address " << address.toText()
+                          << " is not a valid IPV4 address");
+            }
+            asio::ip::address_v4::bytes_type addr_bytes =
+                address.getAddress().to_v4().to_bytes();
+            // Increase the buffer size by the size of IPv4 address.
+            buf.resize(buf.size() + addr_bytes.size());
+            std::copy_backward(addr_bytes.begin(), addr_bytes.end(),
+                               buf.end());
+            return;
+        }
+    case OPT_IPV6_ADDRESS_TYPE:
+        {
+            asiolink::IOAddress address(value);
+            if (!address.getAddress().is_v6()) {
+                isc_throw(BadDataTypeCast, "provided address " << address.toText()
+                          << " is not a valid IPV6 address");
+            }
+            asio::ip::address_v6::bytes_type addr_bytes =
+                address.getAddress().to_v6().to_bytes();
+            // Incresase the buffer size by the size of IPv6 address.
+            buf.resize(buf.size() + addr_bytes.size());
+            std::copy_backward(addr_bytes.begin(), addr_bytes.end(),
+                               buf.end());
+            return;
+        }
+    case OPT_STRING_TYPE:
+        if (value.size() > 0) {
+            // Increase the size of the storage by the size of the string.
+            buf.resize(buf.size() + value.size());
+            // Assuming that the string is already UTF8 encoded.
+            std::copy_backward(value.c_str(), value.c_str() + value.size(),
+                               buf.end());
+            return;
+        }
+    case OPT_FQDN_TYPE:
+        {
+            // FQDN implementation is not terribly complicated but will require
+            // creation of some additional logic (maybe object) that will parse
+            // the fqdn into labels.
+            isc_throw(isc::NotImplemented, "write of FQDN record into option buffer"
+                      " is not supported yet");
+            return;
+        }
+    default:
+        // We hit this point because invalid option data type has been specified
+        // This may be the case because 'empty' or 'record' data type has been
+        // specified. We don't throw exception here because it will be thrown
+        // at the exit point from this function.
+        ;
+    }
+    isc_throw(isc::BadValue, "attempt to write invalid option data field type"
+              " into the option buffer: " << type);
+
 }
 
 OptionDefinition::OptionDefinition(const std::string& name,
@@ -60,17 +243,17 @@ OptionDefinition::OptionDefinition(const std::string& name,
                                  const bool array_type /* = false */)
     : name_(name),
       code_(code),
-      type_(UNKNOWN_TYPE),
+      type_(OPT_UNKNOWN_TYPE),
       array_type_(array_type) {
     // Data type is held as enum value by this class.
     // Use the provided option type string to get the
     // corresponding enum value.
-    type_ = DataTypeUtil::instance().getDataType(type);
+    type_ = DataTypeUtil::instance().getOptionDataType(type);
 }
 
 OptionDefinition::OptionDefinition(const std::string& name,
                                    const uint16_t code,
-                                   const DataType type,
+                                   const OptionDataType type,
                                    const bool array_type /* = false */)
     : name_(name),
       code_(code),
@@ -80,67 +263,113 @@ OptionDefinition::OptionDefinition(const std::string& name,
 
 void
 OptionDefinition::addRecordField(const std::string& data_type_name) {
-    DataType data_type = DataTypeUtil::instance().getDataType(data_type_name);
+    OptionDataType data_type = DataTypeUtil::instance().getOptionDataType(data_type_name);
     addRecordField(data_type);
 }
 
 void
-OptionDefinition::addRecordField(const DataType data_type) {
-    if (type_ != RECORD_TYPE) {
+OptionDefinition::addRecordField(const OptionDataType data_type) {
+    if (type_ != OPT_RECORD_TYPE) {
         isc_throw(isc::InvalidOperation, "'record' option type must be used"
                   " to add data fields to the record");
     }
-    if (data_type >= UNKNOWN_TYPE) {
+    if (data_type >= OPT_UNKNOWN_TYPE) {
         isc_throw(isc::BadValue, "attempted to add invalid data type to the record");
     }
     record_fields_.push_back(data_type);
 }
 
-Option::Factory*
-OptionDefinition::getFactory() const {
+OptionPtr
+OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
+                                OptionBufferConstIter begin,
+                                OptionBufferConstIter end) const {
     validate();
+    
+    try {
+        if (type_ == OPT_BINARY_TYPE) {
+            return (factoryGeneric(u, type, begin, end));
+
+        } else if (type_ == OPT_IPV6_ADDRESS_TYPE && array_type_) {
+            return (factoryAddrList6(type, begin, end));
+
+        } else if (type_ == OPT_IPV4_ADDRESS_TYPE && array_type_) {
+            return (factoryAddrList4(type, begin, end));
+
+        } else if (type_ == OPT_EMPTY_TYPE) {
+            return (factoryEmpty(u, type));
+
+        } else if (u == Option::V6 &&
+                   code_ == D6O_IA_NA &&
+                   haveIA6Format()) {
+            return (factoryIA6(type, begin, end));
+
+        } else if (u == Option::V6 &&
+                   code_ == D6O_IAADDR &&
+                   haveIAAddr6Format()) {
+            return (factoryIAAddr6(type, begin, end));
+
+        } else if (type_ == OPT_UINT8_TYPE) {
+            if (array_type_) {
+                return (factoryGeneric(u, type, begin, end));
+            } else {
+                return (factoryInteger<uint8_t>(u, type, begin, end));
+            }
+
+        } else if (type_ == OPT_UINT16_TYPE) {
+            if (array_type_) {
+                return (factoryIntegerArray<uint16_t>(type, begin, end));
+            } else {
+                return (factoryInteger<uint16_t>(u, type, begin, end));
+            }
+
+        } else if (type_ == OPT_UINT32_TYPE) {
+            if (array_type_) {
+                return (factoryIntegerArray<uint32_t>(type, begin, end));
+            } else {
+                return (factoryInteger<uint32_t>(u, type, begin, end));
+            }
 
-    // @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_ == BINARY_TYPE) {
-        return (factoryGeneric);
-    } else if (type_ == IPV6_ADDRESS_TYPE && array_type_) {
-        return (factoryAddrList6);
-    } else if (type_ == IPV4_ADDRESS_TYPE && array_type_) {
-        return (factoryAddrList4);
-    } else if (type_ == EMPTY_TYPE) {
-        return (factoryEmpty);
-    } else if (code_ == D6O_IA_NA && haveIA6Format()) {
-        return (factoryIA6);
-    } else if (code_ == D6O_IAADDR && haveIAAddr6Format()) {
-        return (factoryIAAddr6);
-    } else if (type_ == UINT8_TYPE) {
-        if (array_type_) {
-            return (factoryGeneric);
-        } else {
-            return (factoryInteger<uint8_t>);
         }
-    } else if (type_ == UINT16_TYPE) {
-        if (array_type_) {
-            return (factoryIntegerArray<uint16_t>);
-        } else {
-            return (factoryInteger<uint16_t>);
+        return (factoryGeneric(u, type, begin, end));
+
+    } catch (const Exception& ex) {
+        isc_throw(InvalidOptionValue, ex.what());
+    }
+}
+
+OptionPtr
+OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
+                                const OptionBuffer& buf) const {
+    return (optionFactory(u, type, buf.begin(), buf.end()));
+}
+
+OptionPtr
+OptionDefinition::optionFactory(Option::Universe u, uint16_t type,
+                                const std::vector<std::string>& values) const {
+    validate();
+
+    OptionBuffer buf;
+    if (!array_type_ && type_ != OPT_RECORD_TYPE) {
+        if (values.size() == 0) {
+            isc_throw(InvalidOptionValue, "no option value specified");
         }
-    } else if (type_ == UINT32_TYPE) {
-        if (array_type_) {
-            return (factoryIntegerArray<uint32_t>);
-        } else {
-            return (factoryInteger<uint32_t>);
+        DataTypeUtil::instance().writeToBuffer(values[0], type_, buf);
+    } else if (array_type_ && type_ != OPT_RECORD_TYPE) {
+        for (size_t i = 0; i < values.size(); ++i) {
+            DataTypeUtil::instance().writeToBuffer(values[i], type_, buf);
+        }
+    } else if (type_ == OPT_RECORD_TYPE) {
+        const RecordFieldsCollection& records = getRecordFields();
+        if (records.size() > values.size()) {
+            isc_throw(InvalidOptionValue, "number of data fields for the option"
+                      << " type " << type_ << " is greater than number of values"
+                      << " provided.");
+        }
+        for (size_t i = 0; i < records.size(); ++i) {
+            DataTypeUtil::instance().writeToBuffer(values[i], records[i], buf);
         }
     }
-    // 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);
+    return (optionFactory(u, type, buf.begin(), buf.end()));
 }
 
 void
@@ -153,28 +382,61 @@ OptionDefinition::sanityCheckUniverse(const Option::Universe expected_universe,
 
 void
 OptionDefinition::validate() const {
-    // Option name must not be empty.
+    std::ostringstream err_str;
     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");
+        // Option name must not be empty.
+        err_str << "option name must not be empty";
+    } else if (name_.find(" ") != string::npos) {
+        // Option name must not contain spaces.
+        err_str << "option name must not contain spaces";
+    } else if (type_ >= OPT_UNKNOWN_TYPE) {
+        // Option definition must be of a known type.
+        err_str << "option type value " << type_ << " is out of range";
+    } else if (type_ == OPT_STRING_TYPE && array_type_) {
+        // Array of strings is not allowed because there is no way
+        // to determine the size of a particular string and thus there
+        // it no way to tell when other data fields begin.
+        err_str << "array of strings is not a valid option definition";
+    } else if (type_ == OPT_RECORD_TYPE) {
+        // At least two data fields should be added to the record. Otherwise
+        // non-record option definition could be used.
+        if (getRecordFields().size() < 2) {
+            err_str << "invalid number of data fields: " << getRecordFields().size()
+                    << " specified for the option of type 'record'. Expected at"
+                    << " least 2 fields.";
+        } else {
+            // If the number of fields is valid we have to check if their order
+            // is valid too. We check that string data fields are not laid before
+            // other fields. But we allow that they are laid at the end of
+            // an option.
+            const RecordFieldsCollection& fields = getRecordFields();
+            for (RecordFieldsConstIter it = fields.begin();
+                 it != fields.end(); ++it) {
+                if (*it == OPT_STRING_TYPE &&
+                    it < fields.end() - 1) {
+                    err_str << "string data field can't be laid before data fields"
+                            << " of other types.";
+                    break;
+                }
+            }
+        }
+
     }
-    // Unsupported option types are not allowed.
-    if (type_ >= UNKNOWN_TYPE) {
-        isc_throw(isc::OutOfRange, "option type value " << type_
-                  << " is out of range");
+
+    // Non-empty error string means that we have hit the error. We throw
+    // exception and include error string.
+    if (!err_str.str().empty()) {
+        isc_throw(MalformedOptionDefinition, err_str.str());
     }
 }
 
 bool
-OptionDefinition::haveIAx6Format(OptionDefinition::DataType first_type) const {
-   return (haveType(RECORD_TYPE) &&
+OptionDefinition::haveIAx6Format(OptionDataType first_type) const {
+   return (haveType(OPT_RECORD_TYPE) &&
            record_fields_.size() == 3 &&
            record_fields_[0] == first_type &&
-           record_fields_[1] == UINT32_TYPE &&
-           record_fields_[2] == UINT32_TYPE);
+           record_fields_[1] == OPT_UINT32_TYPE &&
+           record_fields_[2] == OPT_UINT32_TYPE);
 }
 
 bool
@@ -185,70 +447,66 @@ OptionDefinition::haveIA6Format() const {
     // arrays do not impose limitations on number of elements in
     // the array while this limitation is needed for IA_NA - need
     // exactly 3 elements.
-    return (haveIAx6Format(UINT32_TYPE));
+    return (haveIAx6Format(OPT_UINT32_TYPE));
 }
 
 bool
 OptionDefinition::haveIAAddr6Format() const {
-    return (haveIAx6Format(IPV6_ADDRESS_TYPE));
+    return (haveIAx6Format(OPT_IPV6_ADDRESS_TYPE));
 }
 
 OptionPtr
-OptionDefinition::factoryAddrList4(Option::Universe u, uint16_t type,
-                                   const OptionBuffer& buf) {
-    sanityCheckUniverse(u, Option::V4);
-    boost::shared_ptr<Option4AddrLst> option(new Option4AddrLst(type, buf.begin(),
-                                                                buf.begin() + buf.size()));
+OptionDefinition::factoryAddrList4(uint16_t type,
+                                  OptionBufferConstIter begin,
+                                  OptionBufferConstIter end) {
+    boost::shared_ptr<Option4AddrLst> option(new Option4AddrLst(type, begin, end));
     return (option);
 }
 
 OptionPtr
-OptionDefinition::factoryAddrList6(Option::Universe u, uint16_t type,
-                                   const OptionBuffer& buf) {
-    sanityCheckUniverse(u, Option::V6);
-    boost::shared_ptr<Option6AddrLst> option(new Option6AddrLst(type, buf.begin(),
-                                                                buf.begin() + buf.size()));
+OptionDefinition::factoryAddrList6(uint16_t type,
+                                   OptionBufferConstIter begin,
+                                   OptionBufferConstIter end) {
+    boost::shared_ptr<Option6AddrLst> option(new Option6AddrLst(type, begin, end));
     return (option);
 }
 
 
 OptionPtr
-OptionDefinition::factoryEmpty(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
-    if (buf.size() > 0) {
-        isc_throw(isc::BadValue, "input option buffer must be empty"
-                  " when creating empty option instance");
-    }
+OptionDefinition::factoryEmpty(Option::Universe u, uint16_t type) {
     OptionPtr option(new Option(u, type));
     return (option);
 }
 
 OptionPtr
-OptionDefinition::factoryGeneric(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
-    OptionPtr option(new Option(u, type, buf));
+OptionDefinition::factoryGeneric(Option::Universe u, uint16_t type,
+                                 OptionBufferConstIter begin,
+                                 OptionBufferConstIter end) {
+    OptionPtr option(new Option(u, type, begin, end));
     return (option);
 }
 
 OptionPtr
-OptionDefinition::factoryIA6(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
-    sanityCheckUniverse(u, Option::V6);
-    if (buf.size() != Option6IA::OPTION6_IA_LEN) {
-        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expeted "
-                  << Option6IA::OPTION6_IA_LEN << " bytes");
+OptionDefinition::factoryIA6(uint16_t type,
+                             OptionBufferConstIter begin,
+                             OptionBufferConstIter end) {
+    if (std::distance(begin, end) < Option6IA::OPTION6_IA_LEN) {
+        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expected "
+                  "at least " << Option6IA::OPTION6_IA_LEN << " bytes");
     }
-    boost::shared_ptr<Option6IA> option(new Option6IA(type, buf.begin(),
-                                                      buf.begin() + buf.size()));
+    boost::shared_ptr<Option6IA> option(new Option6IA(type, begin, end));
     return (option);
 }
 
 OptionPtr
-OptionDefinition::factoryIAAddr6(Option::Universe u, uint16_t type, const OptionBuffer& buf) {
-    sanityCheckUniverse(u, Option::V6);
-    if (buf.size() != Option6IAAddr::OPTION6_IAADDR_LEN) {
-        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expeted "
-                  << Option6IAAddr::OPTION6_IAADDR_LEN << " bytes");
+OptionDefinition::factoryIAAddr6(uint16_t type,
+                                 OptionBufferConstIter begin,
+                                 OptionBufferConstIter end) {
+    if (std::distance(begin, end) < Option6IAAddr::OPTION6_IAADDR_LEN) {
+        isc_throw(isc::OutOfRange, "input option buffer has invalid size, expected "
+                  " at least " << Option6IAAddr::OPTION6_IAADDR_LEN << " bytes");
     }
-    boost::shared_ptr<Option6IAAddr> option(new Option6IAAddr(type, buf.begin(),
-                                                      buf.begin() + buf.size()));
+    boost::shared_ptr<Option6IAAddr> option(new Option6IAAddr(type, begin, end));
     return (option);
 }
 

src/lib/dhcp/option_definition.h

@@ -27,6 +27,21 @@
 namespace isc {
 namespace dhcp {
 
+/// @brief Exception to be thrown when invalid option value has been
+/// specified for a particular option definition.
+class InvalidOptionValue : public Exception {
+public:
+    InvalidOptionValue(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
+/// @brief Exception to be thrown when option definition is invalid.
+class MalformedOptionDefinition : public Exception {
+public:
+    MalformedOptionDefinition(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) { };
+};
+
 /// @brief Forward declaration to OptionDefinition.
 class OptionDefinition;
 
@@ -82,7 +97,7 @@ class Option6IntArray;
 ///
 /// Should the 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 OptioDefinition::addRecordField.
+/// are specified using \ref OptionDefinition::addRecordField.
 ///
 /// When the OptionDefinition object has been sucessfully created, it can be
 /// queried to return the appropriate option factory function for the specified
@@ -111,33 +126,14 @@ class Option6IntArray;
 class OptionDefinition {
 public:
 
-    /// Data types of DHCP option fields.
-    enum DataType {
-        EMPTY_TYPE,
-        BINARY_TYPE,
-        BOOLEAN_TYPE,
-        INT8_TYPE,
-        INT16_TYPE,
-        INT32_TYPE,
-        UINT8_TYPE,
-        UINT16_TYPE,
-        UINT32_TYPE,
-        IPV4_ADDRESS_TYPE,
-        IPV6_ADDRESS_TYPE,
-        STRING_TYPE,
-        FQDN_TYPE,
-        RECORD_TYPE,
-        UNKNOWN_TYPE
-    };
-
     /// List of fields within the record.
-    typedef std::vector<DataType> RecordFieldsCollection;
+    typedef std::vector<OptionDataType> RecordFieldsCollection;
     /// Const iterator for record data fields.
-    typedef std::vector<DataType>::const_iterator RecordFieldsConstIter;
+    typedef std::vector<OptionDataType>::const_iterator RecordFieldsConstIter;
 
 private:
 
-    /// @brief Utility class for operations on DataTypes.
+    /// @brief Utility class for operations on OptionDataTypes.
     ///
     /// This class is implemented as the singleton because the list of
     /// supported data types need only be loaded only once into memory as it
@@ -161,7 +157,41 @@ private:
         /// @param data_type_name data type string.
         ///
         /// @return option data type.
-        DataType getDataType(const std::string& data_type_name);
+        OptionDataType getOptionDataType(const std::string& data_type_name);
+
+        /// @brief Perform lexical cast of the value and validate its range.
+        ///
+        /// This function performs lexical cast of a string value to integer
+        /// or boolean value and checks if the resulting value is within a
+        /// range of a target type. Note that range checks are not performed
+        /// on boolean values. The target type should be one of the supported
+        /// integer types or bool.
+        ///
+        /// @param value_str input value given as string.
+        /// @tparam T target type for lexical cast.
+        ///
+        /// @return cast value.
+        /// @throw BadDataTypeCast if cast was not successful.
+        template<typename T>
+        T lexicalCastWithRangeCheck(const std::string& value_str) const;
+
+        /// @brief Write the string value into the provided buffer.
+        ///
+        /// This method writes the given value to the specified buffer.
+        /// The provided string value may represent data of different types.
+        /// The actual data type is specified with the second argument.
+        /// Based on a value of this argument, this function will first
+        /// try to cast the string value to the particular data type and
+        /// if it is successful it will store the data in the buffer
+        /// in a binary format.
+        ///
+        /// @param value string representation of the value to be written.
+        /// @param type the actual data type to be stored.
+        /// @param [in, out] buf buffer where the value is to be stored.
+        ///
+        /// @throw BadDataTypeCast if data write was unsuccessful.
+        void writeToBuffer(const std::string& value, const OptionDataType type,
+                           OptionBuffer& buf);
 
     private:
         /// @brief Private constructor.
@@ -174,7 +204,7 @@ private:
         DataTypeUtil();
 
         /// Map of data types, maps name of the type to enum value.
-        std::map<std::string, DataType> data_types_;
+        std::map<std::string, OptionDataType> data_types_;
     };
 
 public:
@@ -199,7 +229,7 @@ public:
     /// option fields are the array.
     OptionDefinition(const std::string& name,
                      const uint16_t code,
-                     const DataType type,
+                     const OptionDataType type,
                      const bool array_type = false);
 
     /// @brief Adds data field to the record.
@@ -216,7 +246,7 @@ public:
     ///
     /// @throw isc::InvalidOperation if option type is not set to RECORD_TYPE.
     /// @throw isc::BadValue if specified invalid data type.
-    void addRecordField(const DataType data_type);
+    void addRecordField(const OptionDataType data_type);
 
     /// @brief Return array type indicator.
     ///
@@ -231,13 +261,6 @@ public:
     /// @return option code.
     uint16_t getCode() const { return (code_); }
 
-    /// @brief Return factory function for the given definition.
-    ///
-    /// @throw isc::OutOfRange if \ref validate returns it.
-    /// @throw isc::BadValue if \ref validate returns it.
-    /// @return pointer to a factory function.
-    Option::Factory* getFactory() const;
-
     /// @brief Return option name.
     ///
     /// @return option name.
@@ -251,13 +274,11 @@ public:
     /// @brief Return option data type.
     ///
     /// @return option data type.
-    DataType getType() const { return (type_); };
+    OptionDataType 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.
+    /// @throw MalformedOptionDefinition option definition is invalid.
     void validate() const;
 
     /// @brief Check if specified format is IA_NA option format.
@@ -270,101 +291,163 @@ public:
     /// @return true if specified format is IAADDR option format.
     bool haveIAAddr6Format() const;
 
+    /// @brief Option factory.
+    ///
+    /// This function creates an instance of DHCP option using
+    /// provided chunk of buffer. This function may be used to
+    /// create option which is to be sent in the outgoing packet.
+    ///
+    /// @param u option universe (V4 or V6).
+    /// @param type option type.
+    /// @param begin beginning of the option buffer.
+    /// @param end end of the option buffer.
+    ///
+    /// @return instance of the DHCP option.
+    /// @throw MalformedOptionDefinition if option definition is invalid.
+    /// @throw InvalidOptionValue if data for the option is invalid.
+    OptionPtr optionFactory(Option::Universe u, uint16_t type,
+                            OptionBufferConstIter begin,
+                            OptionBufferConstIter end) const;
+
+    /// @brief Option factory.
+    ///
+    /// This function creates an instance of DHCP option using
+    /// whole provided buffer. This function may be used to
+    /// create option which is to be sent in the outgoing packet.
+    ///
+    /// @param u option universe (V4 or V6).
+    /// @param type option type.
+    /// @param buf option buffer.
+    ///
+    /// @return instance of the DHCP option.
+    /// @throw MalformedOptionDefinition if option definition is invalid.
+    /// @throw InvalidOptionValue if data for the option is invalid.
+    OptionPtr optionFactory(Option::Universe u, uint16_t type,
+                            const OptionBuffer& buf) const;
+
+    /// @brief Option factory.
+    ///
+    /// This function creates an instance of DHCP option using the vector
+    /// of strings which carry data values for option data fields.
+    /// The order of values in the vector corresponds to the order of data
+    /// fields in the option. The supplied string values are cast to
+    /// their actual data types which are determined based on the
+    /// option definition. If cast fails due to type mismatch, an exception
+    /// is thrown. This factory function can be used to create option
+    /// instance when user specified option value in the <b>comma separated
+    /// values</b> format in the configuration database. Provided string
+    /// must be tokenized into the vector of string values and this vector
+    /// can be supplied to this function.
+    ///
+    /// @param u option universe (V4 or V6).
+    /// @param type option type.
+    /// @param values a vector of values to be used to set data for an option.
+    ///
+    /// @return instance of the DHCP option.
+    /// @throw MalformedOptionDefinition if option definition is invalid.
+    /// @throw InvalidOptionValue if data for the option is invalid.
+    OptionPtr optionFactory(Option::Universe u, uint16_t type,
+                            const std::vector<std::string>& values) const;
+
     /// @brief Factory to create option with address list.
     ///
-    /// @param u universe (must be V4).
     /// @param type option type.
-    /// @param buf option buffer with a list of IPv4 addresses.
+    /// @param begin iterator pointing to the beginning of the buffer
+    /// with a list of IPv4 addresses.
+    /// @param end iterator pointing to the end of the 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);
+    static OptionPtr factoryAddrList4(uint16_t type,
+                                      OptionBufferConstIter begin,
+                                      OptionBufferConstIter end);
 
     /// @brief Factory to create option with address list.
     ///
-    /// @param u universe (must be V6).
     /// @param type option type.
-    /// @param buf option buffer with a list of IPv6 addresses.
+    /// @param begin iterator pointing to the beginning of the buffer
+    /// with a list of IPv6 addresses.
+    /// @param end iterator pointing to the end of the 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);
+    static OptionPtr factoryAddrList6(uint16_t type,
+                                      OptionBufferConstIter begin,
+                                      OptionBufferConstIter end);
 
     /// @brief Empty option factory.
     ///
     /// @param u universe (V6 or V4).
     /// @param type option type.
-    /// @param buf option buffer (must be empty).
-    static OptionPtr factoryEmpty(Option::Universe u, uint16_t type,
-                                  const OptionBuffer& buf);
+    static OptionPtr factoryEmpty(Option::Universe u, uint16_t type);
 
     /// @brief Factory to create generic option.
     ///
     /// @param u universe (V6 or V4).
     /// @param type option type.
-    /// @param buf option buffer.
+    /// @param begin iterator pointing to the beginning of the buffer.
+    /// @param end iterator pointing to the end of the buffer.
     static OptionPtr factoryGeneric(Option::Universe u, uint16_t type,
-                                    const OptionBuffer& buf);
+                                    OptionBufferConstIter begin,
+                                    OptionBufferConstIter end);
 
     /// @brief Factory for IA-type of option.
     ///
-    /// @param u universe (must be V6).
     /// @param type option type.
-    /// @param buf option buffer.
+    /// @param begin iterator pointing to the beginning of the buffer.
+    /// @param end iterator pointing to the end of the buffer.
     ///
     /// @throw isc::OutOfRange if provided option buffer is too short or
     /// too long. Expected size is 12 bytes.
     /// @throw isc::BadValue if specified universe value is not V6.
-    static OptionPtr factoryIA6(Option::Universe u, uint16_t type,
-                                const OptionBuffer& buf);
+    static OptionPtr factoryIA6(uint16_t type,
+                                OptionBufferConstIter begin,
+                                OptionBufferConstIter end);
 
     /// @brief Factory for IAADDR-type of option.
     ///
-    /// @param u universe (must be V6).
     /// @param type option type.
-    /// @param buf option buffer.
+    /// @param begin iterator pointing to the beginning of the buffer.
+    /// @param end iterator pointing to the end of the buffer.
     ///
     /// @throw isc::OutOfRange if provided option buffer is too short or
     /// too long. Expected size is 24 bytes.
     /// @throw isc::BadValue if specified universe value is not V6.
-    static OptionPtr factoryIAAddr6(Option::Universe u, uint16_t type,
-                                const OptionBuffer& buf);
+    static OptionPtr factoryIAAddr6(uint16_t type,
+                                    OptionBufferConstIter begin,
+                                    OptionBufferConstIter end);
 
     /// @brief Factory function to create option with integer value.
     ///
     /// @param type option type.
-    /// @param buf option buffer.
+    /// @param begin iterator pointing to the beginning of the buffer.
+    /// @param end iterator pointing to the end of the buffer.
     /// @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)) {
-            isc_throw(isc::OutOfRange, "provided option buffer is too large, expected: "
-                      << sizeof(T) << " bytes");
-        }
-        OptionPtr option(new Option6Int<T>(type, buf.begin(), buf.end()));
+    static OptionPtr factoryInteger(Option::Universe, uint16_t type,
+                                    OptionBufferConstIter begin,
+                                    OptionBufferConstIter end) {
+        OptionPtr option(new Option6Int<T>(type, begin, end));
         return (option);
     }
 
     /// @brief Factory function to create option with array of integer values.
     ///
     /// @param type option type.
-    /// @param buf option buffer.
+    /// @param begin iterator pointing to the beginning of the buffer.
+    /// @param end iterator pointing to the end of the buffer.
     /// @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) {
-            isc_throw(isc::OutOfRange, "option buffer length must be greater than zero");
-        } else if (buf.size() % OptionDataTypes<T>::len != 0) {
-            isc_throw(isc::OutOfRange, "option buffer length must be multiple of "
-                      << OptionDataTypes<T>::len << " bytes");
-        }
-        OptionPtr option(new Option6IntArray<T>(type, buf.begin(), buf.end()));
+    static OptionPtr factoryIntegerArray(uint16_t type,
+                                         OptionBufferConstIter begin,
+                                         OptionBufferConstIter end) {
+        OptionPtr option(new Option6IntArray<T>(type, begin, end));
         return (option);
     }
 
@@ -379,12 +462,12 @@ private:
     /// @param first_type type of the first data field.
     ///
     /// @return true if actual option format matches expected format.
-    bool haveIAx6Format(const OptionDefinition::DataType first_type) const;
+    bool haveIAx6Format(const OptionDataType first_type) const;
 
     /// @brief Check if specified type matches option definition type.
     ///
     /// @return true if specified type matches option definition type.
-    inline bool haveType(const DataType type) const {
+    inline bool haveType(const OptionDataType type) const {
         return (type == type_);
     }
 
@@ -395,14 +478,14 @@ private:
     ///
     /// @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); 
+                                          const Option::Universe actual_universe);
 
     /// Option name.
     std::string name_;
     /// Option code.
     uint16_t code_;
     /// Option data type.
-    DataType type_;
+    OptionDataType type_;
     /// Indicates wheter option is a single value or array.
     bool array_type_;
     /// Collection of data fields within the record.
@@ -421,7 +504,7 @@ private:
 /// Note that this container can hold multiple options with the
 /// same code. For this reason, the latter index can be used to
 /// obtain a range of options for a particular option code.
-/// 
+///
 /// @todo: need an index to search options using option space name
 /// once option spaces are implemented.
 typedef boost::multi_index_container<

src/lib/dhcp/tests/libdhcp++_unittest.cc

@@ -39,8 +39,7 @@ using namespace isc::util;
 namespace {
 class LibDhcpTest : public ::testing::Test {
 public:
-    LibDhcpTest() {
-    }
+    LibDhcpTest() { }
 
     /// @brief Generic factory function to create any option.
     ///
@@ -64,14 +63,13 @@ public:
     /// @param bug buffer to be used to create option instance.
     /// @param expected_type type of the option created by the
     /// factory function returned by the option definition.
-    static void testInitOptionDefs6(const uint16_t code,
+    static void testStdOptionDefs6(const uint16_t code,
                              const OptionBuffer& buf,
                              const std::type_info& expected_type) {
-        // Initialize stdandard options definitions. They are held
-        // in the static container throughout the program.
-        LibDHCP::initStdOptionDefs(Option::V6);
         // Get all option definitions, we will use them to extract
         // the definition for a particular option code.
+        // We don't have to initialize option deinitions here because they
+        // are initialized in the class'es constructor.
         OptionDefContainer options = LibDHCP::getOptionDefs(Option::V6);
         // Get the container index #1. This one allows for searching
         // option definitions using option code.
@@ -90,14 +88,9 @@ public:
         ASSERT_TRUE(def);
         // Check that option definition is valid.
         ASSERT_NO_THROW(def->validate());
-        // Get the factory function for the particular option
-        // definition. We will use this factory function to
-        // create option instance.
-        Option::Factory* factory = NULL;
-        ASSERT_NO_THROW(factory = def->getFactory());
         OptionPtr option;
         // Create the option.
-        ASSERT_NO_THROW(option = factory(Option::V6, code, buf));
+        ASSERT_NO_THROW(option = def->optionFactory(Option::V6, code, buf));
         // Make sure it is not NULL.
         ASSERT_TRUE(option);
         // And the actual object type is the one that we expect.
@@ -108,14 +101,14 @@ public:
 };
 
 static const uint8_t packed[] = {
-    0, 12, 0, 5, 100, 101, 102, 103, 104, // opt1 (9 bytes)
-    0, 13, 0, 3, 105, 106, 107, // opt2 (7 bytes)
-    0, 14, 0, 2, 108, 109, // opt3 (6 bytes)
-    1,  0, 0, 4, 110, 111, 112, 113, // opt4 (8 bytes)
-    1,  1, 0, 1, 114 // opt5 (5 bytes)
+    0, 1, 0, 5, 100, 101, 102, 103, 104, // CLIENT_ID (9 bytes)
+    0, 2, 0, 3, 105, 106, 107, // SERVER_ID (7 bytes)
+    0, 14, 0, 0, // RAPID_COMMIT (0 bytes)
+    0,  6, 0, 4, 108, 109, 110, 111, // ORO (8 bytes)
+    0,  8, 0, 2, 112, 113 // ELAPSED_TIME (6 bytes)
 };
 
-TEST(LibDhcpTest, optionFactory) {
+TEST_F(LibDhcpTest, optionFactory) {
     OptionBuffer buf;
     // Factory functions for specific options must be registered before
     // they can be used to create options instances. Otherwise exception
@@ -187,7 +180,7 @@ TEST(LibDhcpTest, optionFactory) {
                            opt_clientid->getData().begin()));
 }
 
-TEST(LibDhcpTest, packOptions6) {
+TEST_F(LibDhcpTest, packOptions6) {
     OptionBuffer buf(512);
     isc::dhcp::Option::OptionCollection opts; // list of options
 
@@ -196,11 +189,11 @@ TEST(LibDhcpTest, packOptions6) {
         buf[i]=i+100;
     }
 
-    OptionPtr opt1(new Option(Option::V6, 12, buf.begin() + 0, buf.begin() + 5));
-    OptionPtr opt2(new Option(Option::V6, 13, buf.begin() + 5, buf.begin() + 8));
-    OptionPtr opt3(new Option(Option::V6, 14, buf.begin() + 8, buf.begin() + 10));
-    OptionPtr opt4(new Option(Option::V6,256, buf.begin() + 10,buf.begin() + 14));
-    OptionPtr opt5(new Option(Option::V6,257, buf.begin() + 14,buf.begin() + 15));
+    OptionPtr opt1(new Option(Option::V6, 1, buf.begin() + 0, buf.begin() + 5));
+    OptionPtr opt2(new Option(Option::V6, 2, buf.begin() + 5, buf.begin() + 8));
+    OptionPtr opt3(new Option(Option::V6, 14, buf.begin() + 8, buf.begin() + 8));
+    OptionPtr opt4(new Option(Option::V6, 6, buf.begin() + 8, buf.begin() + 12));
+    OptionPtr opt5(new Option(Option::V6, 8, buf.begin() + 12, buf.begin() + 14));
 
     opts.insert(pair<int, OptionPtr >(opt1->getType(), opt1));
     opts.insert(pair<int, OptionPtr >(opt1->getType(), opt2));
@@ -211,11 +204,11 @@ TEST(LibDhcpTest, packOptions6) {
     OutputBuffer assembled(512);
 
     EXPECT_NO_THROW(LibDHCP::packOptions6(assembled, opts));
-    EXPECT_EQ(35, assembled.getLength()); // options should take 35 bytes
-    EXPECT_EQ(0, memcmp(assembled.getData(), packed, 35) );
+    EXPECT_EQ(sizeof(packed), assembled.getLength());
+    EXPECT_EQ(0, memcmp(assembled.getData(), packed, sizeof(packed)));
 }
 
-TEST(LibDhcpTest, unpackOptions6) {
+TEST_F(LibDhcpTest, unpackOptions6) {
 
     // just couple of random options
     // Option is used as a simple option implementation
@@ -224,55 +217,85 @@ TEST(LibDhcpTest, unpackOptions6) {
     isc::dhcp::Option::OptionCollection options; // list of options
 
     OptionBuffer buf(512);
-    memcpy(&buf[0], packed, 35);
+    memcpy(&buf[0], packed, sizeof(packed));
 
     EXPECT_NO_THROW ({
-        LibDHCP::unpackOptions6(OptionBuffer(buf.begin(), buf.begin()+35), options);
+            LibDHCP::unpackOptions6(OptionBuffer(buf.begin(), buf.begin() + sizeof(packed)),
+                                    options);
     });
 
     EXPECT_EQ(options.size(), 5); // there should be 5 options
 
-    isc::dhcp::Option::OptionCollection::const_iterator x = options.find(12);
+    isc::dhcp::Option::OptionCollection::const_iterator x = options.find(1);
     ASSERT_FALSE(x == options.end()); // option 1 should exist
-    EXPECT_EQ(12, x->second->getType());  // this should be option 12
+    EXPECT_EQ(1, x->second->getType());  // this should be option 1
     ASSERT_EQ(9, x->second->len()); // it should be of length 9
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+4, 5)); // data len=5
+    ASSERT_EQ(5, x->second->getData().size());
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed + 4, 5)); // data len=5
 
-    x = options.find(13);
-    ASSERT_FALSE(x == options.end()); // option 13 should exist
-    EXPECT_EQ(13, x->second->getType());  // this should be option 13
+        x = options.find(2);
+    ASSERT_FALSE(x == options.end()); // option 2 should exist
+    EXPECT_EQ(2, x->second->getType());  // this should be option 2
     ASSERT_EQ(7, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+13, 3)); // data len=3
+    ASSERT_EQ(3, x->second->getData().size());
+    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed + 13, 3)); // data len=3
 
     x = options.find(14);
-    ASSERT_FALSE(x == options.end()); // option 3 should exist
+    ASSERT_FALSE(x == options.end()); // option 14 should exist
     EXPECT_EQ(14, x->second->getType());  // this should be option 14
-    ASSERT_EQ(6, x->second->len()); // it should be of length 6
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+20, 2)); // data len=2
-
-    x = options.find(256);
-    ASSERT_FALSE(x == options.end()); // option 256 should exist
-    EXPECT_EQ(256, x->second->getType());  // this should be option 256
-    ASSERT_EQ(8, x->second->len()); // it should be of length 7
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+26, 4)); // data len=4
-
-    x = options.find(257);
-    ASSERT_FALSE(x == options.end()); // option 257 should exist
-    EXPECT_EQ(257, x->second->getType());  // this should be option 257
-    ASSERT_EQ(5, x->second->len()); // it should be of length 5
-    EXPECT_EQ(0, memcmp(&x->second->getData()[0], packed+34, 1)); // data len=1
+    ASSERT_EQ(4, x->second->len()); // it should be of length 4
+    EXPECT_EQ(0, x->second->getData().size()); // data len = 0
+
+    x = options.find(6);
+    ASSERT_FALSE(x == options.end()); // option 6 should exist
+    EXPECT_EQ(6, x->second->getType());  // this should be option 6
+    ASSERT_EQ(8, x->second->len()); // it should be of length 8
+    // Option with code 6 is the OPTION_ORO. This option is
+    // represented by the Option6IntArray<uint16_t> class which
+    // comprises the set of uint16_t values. We need to cast the
+    // returned pointer to this type to get values stored in it.
+    boost::shared_ptr<Option6IntArray<uint16_t> > opt_oro =
+        boost::dynamic_pointer_cast<Option6IntArray<uint16_t> >(x->second);
+    // This value will be NULL if cast was unsuccessful. This is the case
+    // when returned option has different type than expected.
+    ASSERT_TRUE(opt_oro);
+    // Get set of uint16_t values.
+    std::vector<uint16_t> opts = opt_oro->getValues();
+    // Prepare the refrence data.
+    std::vector<uint16_t> expected_opts;
+    expected_opts.push_back(0x6C6D); // equivalent to: 108, 109
+    expected_opts.push_back(0x6E6F); // equivalent to 110, 111
+    ASSERT_EQ(expected_opts.size(), opts.size());
+    // Validated if option has been unpacked correctly.
+    EXPECT_TRUE(std::equal(expected_opts.begin(), expected_opts.end(),
+                           opts.begin()));
+
+    x = options.find(8);
+    ASSERT_FALSE(x == options.end()); // option 8 should exist
+    EXPECT_EQ(8, x->second->getType());  // this should be option 8
+    ASSERT_EQ(6, x->second->len()); // it should be of length 9
+    // Option with code 8 is OPTION_ELAPSED_TIME. This option is
+    // represented by Option6Int<uint16_t> value that holds single
+    // uint16_t value.
+    boost::shared_ptr<Option6Int<uint16_t> > opt_elapsed_time =
+        boost::dynamic_pointer_cast<Option6Int<uint16_t> >(x->second);
+    // This value will be NULL if cast was unsuccessful. This is the case
+    // when returned option has different type than expected.
+    ASSERT_TRUE(opt_elapsed_time);
+    // Returned value should be equivalent to two byte values: 112, 113
+    EXPECT_EQ(0x7071, opt_elapsed_time->getValue());
 
     x = options.find(0);
     EXPECT_TRUE(x == options.end()); // option 0 not found
 
-    x = options.find(1); // 1 is htons(256) on little endians. Worth checking
+    x = options.find(256); // 256 is htons(1) on little endians. Worth checking
     EXPECT_TRUE(x == options.end()); // option 1 not found
 
-    x = options.find(2);
+    x = options.find(7);
     EXPECT_TRUE(x == options.end()); // option 2 not found
 
     x = options.find(32000);
-    EXPECT_TRUE(x == options.end()); // option 32000 not found
+    EXPECT_TRUE(x == options.end()); // option 32000 not found */
 }
 
 
@@ -284,7 +307,7 @@ static uint8_t v4Opts[] = {
     128, 3, 40, 41, 42
 };
 
-TEST(LibDhcpTest, packOptions4) {
+TEST_F(LibDhcpTest, packOptions4) {
 
     vector<uint8_t> payload[5];
     for (int i = 0; i < 5; i++) {
@@ -316,7 +339,7 @@ TEST(LibDhcpTest, packOptions4) {
 
 }
 
-TEST(LibDhcpTest, unpackOptions4) {
+TEST_F(LibDhcpTest, unpackOptions4) {
 
     vector<uint8_t> packed(v4Opts, v4Opts + sizeof(v4Opts));
     isc::dhcp::Option::OptionCollection options; // list of options
@@ -375,24 +398,24 @@ TEST(LibDhcpTest, unpackOptions4) {
 // @todo Only limited number of option definitions are now created
 // This test have to be extended once all option definitions are
 // created.
-TEST(LibDhcpTest, initStdOptionDefs) {
-    LibDhcpTest::testInitOptionDefs6(D6O_CLIENTID, OptionBuffer(14, 1),
+TEST_F(LibDhcpTest, stdOptionDefs6) {
+    LibDhcpTest::testStdOptionDefs6(D6O_CLIENTID, OptionBuffer(14, 1),
                                      typeid(Option));
-    LibDhcpTest::testInitOptionDefs6(D6O_SERVERID, OptionBuffer(14, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_SERVERID, OptionBuffer(14, 1),
                                      typeid(Option));
-    LibDhcpTest::testInitOptionDefs6(D6O_IA_NA, OptionBuffer(12, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_IA_NA, OptionBuffer(12, 1),
                                      typeid(Option6IA));
-    LibDhcpTest::testInitOptionDefs6(D6O_IAADDR, OptionBuffer(24, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_IAADDR, OptionBuffer(24, 1),
                                      typeid(Option6IAAddr));
-    LibDhcpTest::testInitOptionDefs6(D6O_ORO, OptionBuffer(10, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_ORO, OptionBuffer(10, 1),
                                      typeid(Option6IntArray<uint16_t>));
-    LibDhcpTest::testInitOptionDefs6(D6O_ELAPSED_TIME, OptionBuffer(2, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_ELAPSED_TIME, OptionBuffer(2, 1),
                                      typeid(Option6Int<uint16_t>));
-    LibDhcpTest::testInitOptionDefs6(D6O_STATUS_CODE, OptionBuffer(10, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_STATUS_CODE, OptionBuffer(10, 1),
                                      typeid(Option));
-    LibDhcpTest::testInitOptionDefs6(D6O_RAPID_COMMIT, OptionBuffer(),
+    LibDhcpTest::testStdOptionDefs6(D6O_RAPID_COMMIT, OptionBuffer(),
                                      typeid(Option));
-    LibDhcpTest::testInitOptionDefs6(D6O_NAME_SERVERS, OptionBuffer(32, 1),
+    LibDhcpTest::testStdOptionDefs6(D6O_NAME_SERVERS, OptionBuffer(32, 1),
                                      typeid(Option6AddrLst));
 }
 

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -206,7 +206,7 @@ TEST_F(Option6IATest, suboptions_unpack) {
 
     Option6IA* ia = 0;
     EXPECT_NO_THROW({
-            ia = new Option6IA(D6O_IA_NA, buf_.begin() + 4, buf_.begin() + sizeof(expected));
+        ia = new Option6IA(D6O_IA_NA, buf_.begin() + 4, buf_.begin() + sizeof(expected));
     });
     ASSERT_TRUE(ia);
 

src/lib/dhcp/tests/option_definition_unittest.cc

@@ -47,32 +47,35 @@ public:
     OptionDefinitionTest() { }
 };
 
+// The purpose of this test is to verify that OptionDefinition
+// constructor initializes its members correctly.
 TEST_F(OptionDefinitionTest, constructor) {
     // Specify the option data type as string. This should get converted
     // to enum value returned by getType().
     OptionDefinition opt_def1("OPTION_CLIENTID", 1, "string");
     EXPECT_EQ("OPTION_CLIENTID", opt_def1.getName());
+
     EXPECT_EQ(1, opt_def1.getCode());
-    EXPECT_EQ(OptionDefinition::STRING_TYPE,  opt_def1.getType());
+    EXPECT_EQ(OPT_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,
-                              OptionDefinition::EMPTY_TYPE);
+                              OPT_EMPTY_TYPE);
     EXPECT_EQ("OPTION_RAPID_COMMIT", opt_def2.getName());
     EXPECT_EQ(14, opt_def2.getCode());
-    EXPECT_EQ(OptionDefinition::EMPTY_TYPE, opt_def2.getType());
+    EXPECT_EQ(OPT_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,
-                              OptionDefinition::IPV6_ADDRESS_TYPE,
+                              OPT_IPV6_ADDRESS_TYPE,
                               true);
     EXPECT_EQ("OPTION_NIS_SERVERS", opt_def3.getName());
     EXPECT_EQ(27, opt_def3.getCode());
-    EXPECT_EQ(OptionDefinition::IPV6_ADDRESS_TYPE, opt_def3.getType());
+    EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, opt_def3.getType());
     EXPECT_TRUE(opt_def3.getArrayType());
     EXPECT_NO_THROW(opt_def3.validate());
 
@@ -81,23 +84,27 @@ TEST_F(OptionDefinitionTest, constructor) {
     // it has been created.
     EXPECT_NO_THROW(
         OptionDefinition opt_def4("OPTION_SERVERID",
-                                  OptionDefinition::UNKNOWN_TYPE + 10,
-                                  OptionDefinition::STRING_TYPE);
+                                  OPT_UNKNOWN_TYPE + 10,
+                                  OPT_STRING_TYPE);
     );
 }
 
+// The purpose of this test is to verify that various data fields
+// can be specified for an option definition when this definition
+// is marked as 'record' and that fields can't be added if option
+// definition is not marked as 'record'.
 TEST_F(OptionDefinitionTest, addRecordField) {
     // We can only add fields to record if the option type has been
     // specified as 'record'. We try all other types but 'record'
     // here and expect exception to be thrown.
-    for (int i = 0; i < OptionDefinition::UNKNOWN_TYPE; ++i) {
+    for (int i = 0; i < OPT_UNKNOWN_TYPE; ++i) {
         // Do not try for 'record' type because this is the only
         // type for which adding record will succeed.
-        if (i == OptionDefinition::RECORD_TYPE) {
+        if (i == OPT_RECORD_TYPE) {
             continue;
         }
         OptionDefinition opt_def("OPTION_IAADDR", 5,
-                                 static_cast<OptionDefinition::DataType>(i));
+                                 static_cast<OptionDataType>(i));
         EXPECT_THROW(opt_def.addRecordField("uint8"), isc::InvalidOperation);
     }
 
@@ -106,54 +113,88 @@ TEST_F(OptionDefinitionTest, addRecordField) {
     EXPECT_NO_THROW(opt_def.addRecordField("ipv6-address"));
     EXPECT_NO_THROW(opt_def.addRecordField("uint32"));
     // It should not matter if we specify field type by its name or using enum.
-    EXPECT_NO_THROW(opt_def.addRecordField(OptionDefinition::UINT32_TYPE));
+    EXPECT_NO_THROW(opt_def.addRecordField(OPT_UINT32_TYPE));
 
     // Check what we have actually added.
     OptionDefinition::RecordFieldsCollection fields = opt_def.getRecordFields();
     ASSERT_EQ(3, fields.size());
-    EXPECT_EQ(OptionDefinition::IPV6_ADDRESS_TYPE, fields[0]);
-    EXPECT_EQ(OptionDefinition::UINT32_TYPE, fields[1]);
-    EXPECT_EQ(OptionDefinition::UINT32_TYPE, fields[2]);
+    EXPECT_EQ(OPT_IPV6_ADDRESS_TYPE, fields[0]);
+    EXPECT_EQ(OPT_UINT32_TYPE, fields[1]);
+    EXPECT_EQ(OPT_UINT32_TYPE, fields[2]);
 
     // Let's try some more negative scenarios: use invalid data types.
     EXPECT_THROW(opt_def.addRecordField("unknown_type_xyz"), isc::BadValue);
-    OptionDefinition::DataType invalid_type =
-        static_cast<OptionDefinition::DataType>(OptionDefinition::UNKNOWN_TYPE + 10);
+    OptionDataType invalid_type =
+        static_cast<OptionDataType>(OPT_UNKNOWN_TYPE + 10);
     EXPECT_THROW(opt_def.addRecordField(invalid_type), isc::BadValue);
+
+    // It is bad if we use 'record' option type but don't specify
+    // at least two fields.
+    OptionDefinition opt_def2("OPTION_EMPTY_RECORD", 100, "record");
+    EXPECT_THROW(opt_def2.validate(), MalformedOptionDefinition);
+    opt_def2.addRecordField("uint8");
+    EXPECT_THROW(opt_def2.validate(), MalformedOptionDefinition);
+    opt_def2.addRecordField("uint32");
+    EXPECT_NO_THROW(opt_def2.validate());
 }
 
+// The purpose of this test is to check that validate() function
+// reports errors for invalid option definitions.
 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);
+    EXPECT_THROW(opt_def1.validate(), MalformedOptionDefinition);
 
     // 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_def2("OPTION_CLIENTID", D6O_CLIENTID, OPT_UNKNOWN_TYPE);
+    EXPECT_THROW(opt_def2.validate(), MalformedOptionDefinition);
 
     OptionDefinition opt_def3("OPTION_CLIENTID", D6O_CLIENTID,
-                              static_cast<OptionDefinition::DataType>(OptionDefinition::UNKNOWN_TYPE
+                              static_cast<OptionDataType>(OPT_UNKNOWN_TYPE
                                                                       + 2));
-    EXPECT_THROW(opt_def3.validate(), isc::OutOfRange);
-    
+    EXPECT_THROW(opt_def3.validate(), MalformedOptionDefinition);
+
     // Empty option name is not allowed.
     OptionDefinition opt_def4("", D6O_CLIENTID, "string");
-    EXPECT_THROW(opt_def4.validate(), isc::BadValue);
+    EXPECT_THROW(opt_def4.validate(), MalformedOptionDefinition);
 
     // Option name must not contain spaces.
     OptionDefinition opt_def5(" OPTION_CLIENTID", D6O_CLIENTID, "string");
-    EXPECT_THROW(opt_def5.validate(), isc::BadValue);
+    EXPECT_THROW(opt_def5.validate(), MalformedOptionDefinition);
 
-    OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string");
-    EXPECT_THROW(opt_def6.validate(), isc::BadValue);
+    // Option name must not contain spaces.
+    OptionDefinition opt_def6("OPTION CLIENTID", D6O_CLIENTID, "string", true);
+    EXPECT_THROW(opt_def6.validate(), MalformedOptionDefinition);
+
+    // Having array of strings does not make sense because there is no way
+    // to determine string's length.
+    OptionDefinition opt_def7("OPTION_CLIENTID", D6O_CLIENTID, "string", true);
+    EXPECT_THROW(opt_def7.validate(), MalformedOptionDefinition);
+
+    // It does not make sense to have string field within the record before
+    // other fields because there is no way to determine the length of this
+    // string and thus there is no way to determine where the other field
+    // begins.
+    OptionDefinition opt_def8("OPTION_STATUS_CODE", D6O_STATUS_CODE,
+                              "record");
+    opt_def8.addRecordField("string");
+    opt_def8.addRecordField("uint16");
+    EXPECT_THROW(opt_def8.validate(), MalformedOptionDefinition);
+
+    // ... but it is ok if the string value is the last one.
+    OptionDefinition opt_def9("OPTION_STATUS_CODE", D6O_STATUS_CODE,
+                              "record");
+    opt_def9.addRecordField("uint8");
+    opt_def9.addRecordField("string");
 }
 
-TEST_F(OptionDefinitionTest, factoryAddrList6) {
+
+// The purpose of this test is to verify that option definition
+// that comprises array of IPv6 addresses will return an instance
+// of option with a list of IPv6 addresses.
+TEST_F(OptionDefinitionTest, ipv6AddressArray) {
     OptionDefinition opt_def("OPTION_NIS_SERVERS", D6O_NIS_SERVERS,
                              "ipv6-address", true);
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     // Create a list of some V6 addresses.
     std::vector<asiolink::IOAddress> addrs;
@@ -176,7 +217,7 @@ TEST_F(OptionDefinitionTest, factoryAddrList6) {
     // the provided buffer.
     OptionPtr option_v6;
     ASSERT_NO_THROW(
-        option_v6 = factory(Option::V6, D6O_NIS_SERVERS, buf);
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_NIS_SERVERS, buf);
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6AddrLst));
     boost::shared_ptr<Option6AddrLst> option_cast_v6 =
@@ -195,17 +236,64 @@ TEST_F(OptionDefinitionTest, factoryAddrList6) {
     buf.insert(buf.end(), 1, 1);
     // It should throw exception then.
     EXPECT_THROW(
-        factory(Option::V6, D6O_NIS_SERVERS, buf),
-        isc::OutOfRange
+        opt_def.optionFactory(Option::V6, D6O_NIS_SERVERS, buf),
+        InvalidOptionValue
+    );
+}
+
+// The purpose of this test is to verify that option definition
+// that comprises array of IPv6 addresses will return an instance
+// of option with a list of IPv6 addresses. Array of IPv6 addresses
+// is specified as a vector of strings (each string represents single
+// IPv6 address).
+TEST_F(OptionDefinitionTest, ipv6AddressArrayTokenized) {
+    OptionDefinition opt_def("OPTION_NIS_SERVERS", D6O_NIS_SERVERS,
+                             "ipv6-address", true);
+
+    // Create a vector of some V6 addresses.
+    std::vector<asiolink::IOAddress> addrs;
+    addrs.push_back(asiolink::IOAddress("2001:0db8::ff00:0042:8329"));
+    addrs.push_back(asiolink::IOAddress("2001:0db8::ff00:0042:2319"));
+    addrs.push_back(asiolink::IOAddress("::1"));
+    addrs.push_back(asiolink::IOAddress("::2"));
+
+    // Create a vector of strings representing addresses given above.
+    std::vector<std::string> addrs_str;
+    for (std::vector<asiolink::IOAddress>::const_iterator it = addrs.begin();
+         it != addrs.end(); ++it) {
+        addrs_str.push_back(it->toText());
+    }
+
+    // Create DHCPv6 option using the list of IPv6 addresses given in the
+    // string form.
+    OptionPtr option_v6;
+    ASSERT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_NIS_SERVERS,
+                                          addrs_str);
     );
+    // Non-null pointer option is supposed to be returned and it
+    // should have Option6AddrLst type.
+    ASSERT_TRUE(option_v6);
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6AddrLst));
+    // Cast to the actual option type to get IPv6 addresses from it.
+    boost::shared_ptr<Option6AddrLst> option_cast_v6 =
+        boost::static_pointer_cast<Option6AddrLst>(option_v6);
+    // Check that cast was successful.
+    ASSERT_TRUE(option_cast_v6);
+    // Get the list of parsed addresses from the option object.
+    std::vector<asiolink::IOAddress> addrs_returned =
+        option_cast_v6->getAddresses();
+    // Returned addresses must match the addresses that have been used to create
+    // the option instance.
+    EXPECT_TRUE(std::equal(addrs.begin(), addrs.end(), addrs_returned.begin()));
 }
 
-TEST_F(OptionDefinitionTest, factoryAddrList4) {
+// The purpose of this test is to verify that option definition
+// that comprises array of IPv4 addresses will return an instance
+// of option with a list of IPv4 addresses.
+TEST_F(OptionDefinitionTest, ipv4AddressArray) {
     OptionDefinition opt_def("OPTION_NAME_SERVERS", D6O_NIS_SERVERS,
                              "ipv4-address", true);
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     // Create a list of some V6 addresses.
     std::vector<asiolink::IOAddress> addrs;
@@ -228,7 +316,7 @@ TEST_F(OptionDefinitionTest, factoryAddrList4) {
     // the provided buffer.
     OptionPtr option_v4;
     ASSERT_NO_THROW(
-        option_v4 = factory(Option::V4, DHO_NAME_SERVERS, buf)
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_NAME_SERVERS, buf)
     );
     ASSERT_TRUE(typeid(*option_v4) == typeid(Option4AddrLst));
     // Get the list of parsed addresses from the option object.
@@ -245,19 +333,66 @@ TEST_F(OptionDefinitionTest, factoryAddrList4) {
     // fulfilled anymore.
     buf.insert(buf.end(), 1, 1);
     // It should throw exception then.
-    EXPECT_THROW(factory(Option::V4, DHO_NIS_SERVERS, buf), isc::OutOfRange);
+    EXPECT_THROW(opt_def.optionFactory(Option::V4, DHO_NIS_SERVERS, buf),
+                 InvalidOptionValue);
+}
+
+// The purpose of this test is to verify that option definition
+// that comprises array of IPv4 addresses will return an instance
+// of option with a list of IPv4 addresses. The array of IPv4 addresses
+// is specified as a vector of strings (each string represents single
+// IPv4 address).
+TEST_F(OptionDefinitionTest, ipv4AddressArrayTokenized) {
+    OptionDefinition opt_def("OPTION_NIS_SERVERS", DHO_NIS_SERVERS,
+                             "ipv4-address", true);
+
+    // Create a vector of some V6 addresses.
+    std::vector<asiolink::IOAddress> addrs;
+    addrs.push_back(asiolink::IOAddress("192.168.0.1"));
+    addrs.push_back(asiolink::IOAddress("172.16.1.1"));
+    addrs.push_back(asiolink::IOAddress("127.0.0.1"));
+    addrs.push_back(asiolink::IOAddress("213.41.23.12"));
+
+    // Create a vector of strings representing addresses given above.
+    std::vector<std::string> addrs_str;
+    for (std::vector<asiolink::IOAddress>::const_iterator it = addrs.begin();
+         it != addrs.end(); ++it) {
+        addrs_str.push_back(it->toText());
+    }
+
+    // Create DHCPv4 option using the list of IPv4 addresses given in the
+    // string form.
+    OptionPtr option_v4;
+    ASSERT_NO_THROW(
+        option_v4 = opt_def.optionFactory(Option::V4, DHO_NIS_SERVERS,
+                                          addrs_str);
+    );
+    // Non-null pointer option is supposed to be returned and it
+    // should have Option6AddrLst type.
+    ASSERT_TRUE(option_v4);
+    ASSERT_TRUE(typeid(*option_v4) == typeid(Option4AddrLst));
+    // Cast to the actual option type to get IPv4 addresses from it.
+    boost::shared_ptr<Option4AddrLst> option_cast_v4 =
+        boost::static_pointer_cast<Option4AddrLst>(option_v4);
+    // Check that cast was successful.
+    ASSERT_TRUE(option_cast_v4);
+    // Get the list of parsed addresses from the option object.
+    std::vector<asiolink::IOAddress> addrs_returned =
+        option_cast_v4->getAddresses();
+    // Returned addresses must match the addresses that have been used to create
+    // the option instance.
+    EXPECT_TRUE(std::equal(addrs.begin(), addrs.end(), addrs_returned.begin()));
 }
 
-TEST_F(OptionDefinitionTest, factoryEmpty) {
+// The purpose of thie test is to verify that option definition for
+// 'empty' option can be created and that it returns 'empty' option.
+TEST_F(OptionDefinitionTest, empty) {
     OptionDefinition opt_def("OPTION_RAPID_COMMIT", D6O_RAPID_COMMIT, "empty");
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     // Create option instance and provide empty buffer as expected.
     OptionPtr option_v6;
     ASSERT_NO_THROW(
-        option_v6 = factory(Option::V6, D6O_RAPID_COMMIT, OptionBuffer())
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_RAPID_COMMIT, OptionBuffer())
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option));
     // Expect 'empty' DHCPv6 option.
@@ -266,31 +401,23 @@ TEST_F(OptionDefinitionTest, factoryEmpty) {
     EXPECT_EQ(0, option_v6->getData().size());
 
     // Repeat the same test scenario for DHCPv4 option.
-    EXPECT_THROW(factory(Option::V4, 214, OptionBuffer(2)),isc::BadValue);
-
     OptionPtr option_v4;
-    ASSERT_NO_THROW(option_v4 = factory(Option::V4, 214, OptionBuffer()));
+    ASSERT_NO_THROW(option_v4 = opt_def.optionFactory(Option::V4, 214, OptionBuffer()));
     // Expect 'empty' DHCPv4 option.
     EXPECT_EQ(Option::V4, option_v4->getUniverse());
     EXPECT_EQ(2, option_v4->getHeaderLen());
     EXPECT_EQ(0, option_v4->getData().size());
-
-    // This factory produces empty option (consisting of option type
-    // and length). Attempt to provide some data in the buffer should
-    // result in exception.
-    EXPECT_THROW(factory(Option::V6, D6O_RAPID_COMMIT,OptionBuffer(2)),isc::BadValue);
 }
 
-TEST_F(OptionDefinitionTest, factoryBinary) {
+// The purpose of this test is to verify that definition can be
+// creates for the option that holds binary data.
+TEST_F(OptionDefinitionTest, binary) {
     // Binary option is the one that is represented by the generic
     // Option class. In fact all options can be represented by this
     // class but for some of them it is just natural. The SERVERID
     // option consists of the option code, length and binary data so
     // this one was picked for this test.
     OptionDefinition opt_def("OPTION_SERVERID", D6O_SERVERID, "binary");
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     // Prepare some dummy data (serverid): 0, 1, 2 etc.
     OptionBuffer buf(14);
@@ -302,7 +429,7 @@ TEST_F(OptionDefinitionTest, factoryBinary) {
     // object of the type Option should be returned.
     OptionPtr option_v6;
     ASSERT_NO_THROW(
-        option_v6 = factory(Option::V6, D6O_SERVERID, buf);
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_SERVERID, buf);
     );
     // Expect base option type returned.
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option));
@@ -320,7 +447,7 @@ TEST_F(OptionDefinitionTest, factoryBinary) {
 
     // Repeat the same test scenario for DHCPv4 option.
     OptionPtr option_v4;
-    ASSERT_NO_THROW(option_v4 = factory(Option::V4, 214, buf));
+    ASSERT_NO_THROW(option_v4 = opt_def.optionFactory(Option::V4, 214, buf));
     // Expect 'empty' DHCPv4 option.
     EXPECT_EQ(Option::V4, option_v4->getUniverse());
     EXPECT_EQ(2, option_v4->getHeaderLen());
@@ -331,19 +458,69 @@ TEST_F(OptionDefinitionTest, factoryBinary) {
                            buf.begin()));
 }
 
-TEST_F(OptionDefinitionTest, factoryIA6) {
+// The purpose of this test is to verify that definition can be
+// creates for the option that holds binary data and that the
+// binary data can be specified in 'comma separated values'
+// format.
+TEST_F(OptionDefinitionTest, binaryTokenized) {
+    OptionDefinition opt_def("OPTION_FOO", 1000, "binary", true);
+
+    // Prepare some dummy data (serverid): 0, 1, 2 etc.
+    OptionBuffer buf(16);
+    for (int i = 0; i < buf.size(); ++i) {
+        buf[i] = i;
+    }
+    std::vector<std::string> hex_data;
+    hex_data.push_back("00010203");
+    hex_data.push_back("04050607");
+    hex_data.push_back("08090A0B0C0D0E0F");
+
+    // Create option instance with the factory function.
+    // If the OptionDefinition code works properly than
+    // object of the type Option should be returned.
+    OptionPtr option_v6;
+    ASSERT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, 1000, hex_data);
+    );
+    // Expect base option type returned.
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option));
+    // Sanity check on universe, length and size. These are
+    // the basic parameters identifying any option.
+    EXPECT_EQ(Option::V6, option_v6->getUniverse());
+    EXPECT_EQ(4, option_v6->getHeaderLen());
+    ASSERT_EQ(buf.size(), option_v6->getData().size());
+
+    // Get data from the option and compare against reference buffer.
+    // They are expected to match.
+    EXPECT_TRUE(std::equal(option_v6->getData().begin(),
+                           option_v6->getData().end(),
+                           buf.begin()));
+
+    // Repeat the same test scenario for DHCPv4 option.
+    OptionPtr option_v4;
+    ASSERT_NO_THROW(option_v4 = opt_def.optionFactory(Option::V4, 214, hex_data));
+    EXPECT_EQ(Option::V4, option_v4->getUniverse());
+    EXPECT_EQ(2, option_v4->getHeaderLen());
+    ASSERT_EQ(buf.size(), option_v4->getData().size());
+
+    EXPECT_TRUE(std::equal(option_v6->getData().begin(),
+                           option_v6->getData().end(),
+                           buf.begin()));
+}
+
+// The purpose of this test is to verify that definition can be created
+// for option that comprises record of data. In this particular test
+// the IA_NA option is used. This option comprises three uint32 fields.
+TEST_F(OptionDefinitionTest, recordIA6) {
     // This option consists of IAID, T1 and T2 fields (each 4 bytes long).
     const int option6_ia_len = 12;
 
     // Get the factory function pointer.
-    OptionDefinition opt_def("OPTION_IA_NA", D6O_IA_NA, "record", true);
+    OptionDefinition opt_def("OPTION_IA_NA", D6O_IA_NA, "record", false);
     // Each data field is uint32.
     for (int i = 0; i < 3; ++i) {
         EXPECT_NO_THROW(opt_def.addRecordField("uint32"));
     }
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     // Check the positive scenario.
     OptionBuffer buf(12);
@@ -351,7 +528,7 @@ TEST_F(OptionDefinitionTest, factoryIA6) {
         buf[i] = i;
     }
     OptionPtr option_v6;
-    ASSERT_NO_THROW(option_v6 = factory(Option::V6, D6O_IA_NA, buf));
+    ASSERT_NO_THROW(option_v6 = opt_def.optionFactory(Option::V6, D6O_IA_NA, buf));
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IA));
     boost::shared_ptr<Option6IA> option_cast_v6 =
         boost::static_pointer_cast<Option6IA>(option_v6);
@@ -359,25 +536,18 @@ TEST_F(OptionDefinitionTest, factoryIA6) {
     EXPECT_EQ(0x04050607, option_cast_v6->getT1());
     EXPECT_EQ(0x08090A0B, option_cast_v6->getT2());
 
-    // This should work for DHCPv6 only, try passing invalid universe value.
-    EXPECT_THROW(
-        factory(Option::V4, D6O_IA_NA, OptionBuffer(option6_ia_len)),
-        isc::BadValue
-    );
-    // The length of the buffer must be 12 bytes.
+    // The length of the buffer must be at least 12 bytes.
     // Check too short buffer.
     EXPECT_THROW(
-        factory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len - 1)),
-        isc::OutOfRange
+        opt_def.optionFactory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len - 1)),
+        InvalidOptionValue
      );
-    // Check too long buffer.
-    EXPECT_THROW(
-        factory(Option::V6, D6O_IA_NA, OptionBuffer(option6_ia_len + 1)),
-        isc::OutOfRange
-    );
 }
 
-TEST_F(OptionDefinitionTest, factoryIAAddr6) {
+// The purpose of this test is to verify that definition can be created
+// for option that comprises record of data. In this particular test
+// the IAADDR option is used.
+TEST_F(OptionDefinitionTest, recordIAAddr6) {
     // This option consists of IPV6 Address (16 bytes) and preferred-lifetime and
     // valid-lifetime fields (each 4 bytes long).
     const int option6_iaaddr_len = 24;
@@ -386,9 +556,6 @@ TEST_F(OptionDefinitionTest, factoryIAAddr6) {
     ASSERT_NO_THROW(opt_def.addRecordField("ipv6-address"));
     ASSERT_NO_THROW(opt_def.addRecordField("uint32"));
     ASSERT_NO_THROW(opt_def.addRecordField("uint32"));
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     // Check the positive scenario.
     OptionPtr option_v6;
@@ -403,7 +570,7 @@ TEST_F(OptionDefinitionTest, factoryIAAddr6) {
     for (int i = 0; i < option6_iaaddr_len - asiolink::V6ADDRESS_LEN; ++i) {
         buf.push_back(i);
     }
-    ASSERT_NO_THROW(option_v6 = factory(Option::V6, D6O_IAADDR, buf));
+    ASSERT_NO_THROW(option_v6 = opt_def.optionFactory(Option::V6, D6O_IAADDR, buf));
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IAAddr));
     boost::shared_ptr<Option6IAAddr> option_cast_v6 =
         boost::static_pointer_cast<Option6IAAddr>(option_v6);
@@ -411,44 +578,54 @@ TEST_F(OptionDefinitionTest, factoryIAAddr6) {
     EXPECT_EQ(0x00010203, option_cast_v6->getPreferred());
     EXPECT_EQ(0x04050607, option_cast_v6->getValid());
 
-    // This should work for DHCPv6 only, try passing invalid universe value.
-    EXPECT_THROW(
-        factory(Option::V4, D6O_IAADDR, OptionBuffer(option6_iaaddr_len)),
-        isc::BadValue
-    );
-    // The length of the buffer must be 12 bytes.
+    // The length of the buffer must be at least 12 bytes.
     // Check too short buffer.
     EXPECT_THROW(
-        factory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len - 1)),
-        isc::OutOfRange
+        opt_def.optionFactory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len - 1)),
+        InvalidOptionValue
      );
-    // Check too long buffer.
-    EXPECT_THROW(
-        factory(Option::V6, D6O_IAADDR, OptionBuffer(option6_iaaddr_len + 1)),
-        isc::OutOfRange
-    );
 }
 
-TEST_F(OptionDefinitionTest, factoryIntegerInvalidType) {
-    // The template function factoryInteger<> accepts integer values only
-    // as template typename. Here we try passing different type and
-    // see if it rejects it.
-    EXPECT_THROW(
-        OptionDefinition::factoryInteger<bool>(Option::V6, D6O_PREFERENCE, OptionBuffer(1)),
-        isc::dhcp::InvalidDataType
-    );
+// The purpose of this test is to verify that definition can be created
+// for option that comprises record of data. In this particular test
+// the IAADDR option is used. The data for the option is speicifed as
+// a vector of strings. Each string carries the data for the corresponding
+// data field.
+TEST_F(OptionDefinitionTest, recordIAAddr6Tokenized) {
+    // This option consists of IPV6 Address (16 bytes) and preferred-lifetime and
+    // valid-lifetime fields (each 4 bytes long).
+    OptionDefinition opt_def("OPTION_IAADDR", D6O_IAADDR, "record");
+    ASSERT_NO_THROW(opt_def.addRecordField("ipv6-address"));
+    ASSERT_NO_THROW(opt_def.addRecordField("uint32"));
+    ASSERT_NO_THROW(opt_def.addRecordField("uint32"));
+
+    // Check the positive scenario.
+    std::vector<std::string> data_field_values;
+    data_field_values.push_back("2001:0db8::ff00:0042:8329");
+    data_field_values.push_back("1234");
+    data_field_values.push_back("5678");
+
+    OptionPtr option_v6;
+    ASSERT_NO_THROW(option_v6 = opt_def.optionFactory(Option::V6, D6O_IAADDR,
+                                                      data_field_values));
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IAAddr));
+    boost::shared_ptr<Option6IAAddr> option_cast_v6 =
+        boost::static_pointer_cast<Option6IAAddr>(option_v6);
+    EXPECT_EQ("2001:db8::ff00:42:8329", option_cast_v6->getAddress().toText());
+    EXPECT_EQ(1234, option_cast_v6->getPreferred());
+    EXPECT_EQ(5678, option_cast_v6->getValid());
 }
 
-TEST_F(OptionDefinitionTest, factoryUint8) {
+// The purpose of this test is to verify that definition for option that
+// comprises single uint8 value can be created and that this definition
+// can be used to create an option with single uint8 value.
+TEST_F(OptionDefinitionTest, uint8) {
     OptionDefinition opt_def("OPTION_PREFERENCE", D6O_PREFERENCE, "uint8");
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     OptionPtr option_v6;
     // Try to use correct buffer length = 1 byte.
     ASSERT_NO_THROW(
-        option_v6 = factory(Option::V6, D6O_PREFERENCE, OptionBuffer(1, 1));
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, OptionBuffer(1, 1));
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint8_t>));
     // Validate the value.
@@ -456,26 +633,47 @@ TEST_F(OptionDefinitionTest, factoryUint8) {
         boost::static_pointer_cast<Option6Int<uint8_t> >(option_v6);
     EXPECT_EQ(1, option_cast_v6->getValue());
 
-    // Try to provide too large buffer. Expect exception.
+    // Try to provide zero-length buffer. Expect exception.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, D6O_PREFERENCE, OptionBuffer(3)),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, OptionBuffer()),
+        InvalidOptionValue
     );
 
-    // Try to provide zero-length buffer. Expect exception.
-    EXPECT_THROW(
-        option_v6 = factory(Option::V6, D6O_PREFERENCE, OptionBuffer()),
-        isc::OutOfRange
+    // @todo Add more cases for DHCPv4
+}
+
+// The purpose of this test is to verify that definition for option that
+// comprises single uint8 value can be created and that this definition
+// can be used to create an option with single uint8 value.
+TEST_F(OptionDefinitionTest, uint8Tokenized) {
+    OptionDefinition opt_def("OPTION_PREFERENCE", D6O_PREFERENCE, "uint8");
+
+    OptionPtr option_v6;
+    std::vector<std::string> values;
+    values.push_back("123");
+    values.push_back("456");
+    try {
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, values);
+    } catch (std::exception& ex) {
+        std::cout << ex.what() << std::endl;
+    }
+    ASSERT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_PREFERENCE, values);
     );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint8_t>));
+    // Validate the value.
+    boost::shared_ptr<Option6Int<uint8_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6Int<uint8_t> >(option_v6);
+    EXPECT_EQ(123, option_cast_v6->getValue());
 
     // @todo Add more cases for DHCPv4
 }
 
-TEST_F(OptionDefinitionTest, factoryUint16) {
+// The purpose of this test is to verify that definition for option that
+// comprises single uint16 value can be created and that this definition
+// can be used to create an option with single uint16 value.
+TEST_F(OptionDefinitionTest, uint16) {
     OptionDefinition opt_def("OPTION_ELAPSED_TIME", D6O_ELAPSED_TIME, "uint16");
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     OptionPtr option_v6;
     // Try to use correct buffer length = 2 bytes.
@@ -483,7 +681,7 @@ TEST_F(OptionDefinitionTest, factoryUint16) {
     buf.push_back(1);
     buf.push_back(2);
     ASSERT_NO_THROW(
-        option_v6 = factory(Option::V6, D6O_ELAPSED_TIME, buf);
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_ELAPSED_TIME, buf);
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint16_t>));
     // Validate the value.
@@ -491,25 +689,44 @@ TEST_F(OptionDefinitionTest, factoryUint16) {
         boost::static_pointer_cast<Option6Int<uint16_t> >(option_v6);
     EXPECT_EQ(0x0102, option_cast_v6->getValue());
 
-    // Try to provide too large buffer. Expect exception.
-    EXPECT_THROW(
-        option_v6 = factory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(3)),
-        isc::OutOfRange
-    );
     // Try to provide zero-length buffer. Expect exception.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(1)),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_ELAPSED_TIME, OptionBuffer(1)),
+        InvalidOptionValue
+    );
+
+    // @todo Add more cases for DHCPv4
+}
+
+// The purpose of this test is to verify that definition for option that
+// comprises single uint16 value can be created and that this definition
+// can be used to create an option with single uint16 value.
+TEST_F(OptionDefinitionTest, uint16Tokenized) {
+    OptionDefinition opt_def("OPTION_ELAPSED_TIME", D6O_ELAPSED_TIME, "uint16");
+
+    OptionPtr option_v6;
+
+    std::vector<std::string> values;
+    values.push_back("1234");
+    values.push_back("5678");
+    ASSERT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_ELAPSED_TIME, values);
     );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint16_t>));
+    // Validate the value.
+    boost::shared_ptr<Option6Int<uint16_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6Int<uint16_t> >(option_v6);
+    EXPECT_EQ(1234, option_cast_v6->getValue());
 
     // @todo Add more cases for DHCPv4
+
 }
 
-TEST_F(OptionDefinitionTest, factoryUint32) {
+// The purpose of this test is to verify that definition for option that
+// comprises single uint32 value can be created and that this definition
+// can be used to create an option with single uint32 value.
+TEST_F(OptionDefinitionTest, uint32) {
     OptionDefinition opt_def("OPTION_CLT_TIME", D6O_CLT_TIME, "uint32");
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     OptionPtr option_v6;
     OptionBuffer buf;
@@ -518,7 +735,7 @@ TEST_F(OptionDefinitionTest, factoryUint32) {
     buf.push_back(3);
     buf.push_back(4);
     ASSERT_NO_THROW(
-        option_v6 = factory(Option::V6, D6O_CLT_TIME, buf);
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_CLT_TIME, buf);
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint32_t>));
     // Validate the value.
@@ -526,27 +743,44 @@ TEST_F(OptionDefinitionTest, factoryUint32) {
         boost::static_pointer_cast<Option6Int<uint32_t> >(option_v6);
     EXPECT_EQ(0x01020304, option_cast_v6->getValue());
 
-    // Try to provide too large buffer. Expect exception.
+    // Try to provide too short buffer. Expect exception.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, D6O_CLT_TIME, OptionBuffer(5)),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_CLT_TIME, OptionBuffer(2)),
+        InvalidOptionValue
     );
-    // Try to provide zero-length buffer. Expect exception.
-    EXPECT_THROW(
-        option_v6 = factory(Option::V6, D6O_CLT_TIME, OptionBuffer(2)),
-        isc::OutOfRange
+
+    // @todo Add more cases for DHCPv4
+}
+
+// The purpose of this test is to verify that definition for option that
+// comprises single uint32 value can be created and that this definition
+// can be used to create an option with single uint32 value.
+TEST_F(OptionDefinitionTest, uint32Tokenized) {
+    OptionDefinition opt_def("OPTION_CLT_TIME", D6O_CLT_TIME, "uint32");
+
+    OptionPtr option_v6;
+    std::vector<std::string> values;
+    values.push_back("123456");
+    values.push_back("789");
+    ASSERT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, D6O_CLT_TIME, values);
     );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6Int<uint32_t>));
+    // Validate the value.
+    boost::shared_ptr<Option6Int<uint32_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6Int<uint32_t> >(option_v6);
+    EXPECT_EQ(123456, option_cast_v6->getValue());
 
     // @todo Add more cases for DHCPv4
 }
 
-TEST_F(OptionDefinitionTest, factoryUint16Array) {
+// The purpose of this test is to verify that definition for option that
+// comprises array of uint16 values can be created and that this definition
+// can be used to create option with an array of uint16 values.
+TEST_F(OptionDefinitionTest, uint16Array) {
     // Let's define some dummy option.
     const uint16_t opt_code = 79;
     OptionDefinition opt_def("OPTION_UINT16_ARRAY", opt_code, "uint16", true);
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     OptionPtr option_v6;
     // Positive scenario, initiate the buffer with length being
@@ -558,7 +792,7 @@ TEST_F(OptionDefinitionTest, factoryUint16Array) {
     }
     // Constructor should succeed because buffer has correct size.
     EXPECT_NO_THROW(
-        option_v6 = factory(Option::V6, opt_code, buf);
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, buf);
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IntArray<uint16_t>));
     boost::shared_ptr<Option6IntArray<uint16_t> > option_cast_v6 =
@@ -576,24 +810,50 @@ TEST_F(OptionDefinitionTest, factoryUint16Array) {
     // Provided buffer size must be greater than zero. Check if we
     // get exception if we provide zero-length buffer.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, opt_code, OptionBuffer()),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, OptionBuffer()),
+        InvalidOptionValue
     );
     // Buffer length must be multiple of data type size.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, opt_code, OptionBuffer(5)),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, OptionBuffer(5)),
+        InvalidOptionValue
+    );
+}
+
+// The purpose of this test is to verify that definition for option that
+// comprises array of uint16 values can be created and that this definition
+// can be used to create option with an array of uint16 values.
+TEST_F(OptionDefinitionTest, uint16ArrayTokenized) {
+    // Let's define some dummy option.
+    const uint16_t opt_code = 79;
+    OptionDefinition opt_def("OPTION_UINT16_ARRAY", opt_code, "uint16", true);
+
+    OptionPtr option_v6;
+    std::vector<std::string> str_values;
+    str_values.push_back("12345");
+    str_values.push_back("5679");
+    str_values.push_back("12");
+    EXPECT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, str_values);
     );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IntArray<uint16_t>));
+    boost::shared_ptr<Option6IntArray<uint16_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6IntArray<uint16_t> >(option_v6);
+    // Get the values from the initiated options and validate.
+    std::vector<uint16_t> values = option_cast_v6->getValues();
+    EXPECT_EQ(12345, values[0]);
+    EXPECT_EQ(5679, values[1]);
+    EXPECT_EQ(12, values[2]);
 }
 
-TEST_F(OptionDefinitionTest, factoryUint32Array) {
+// The purpose of this test is to verify that definition for option that
+// comprises array of uint32 values can be created and that this definition
+// can be used to create option with an array of uint32 values.
+TEST_F(OptionDefinitionTest, uint32Array) {
     // Let's define some dummy option.
     const uint16_t opt_code = 80;
 
     OptionDefinition opt_def("OPTION_UINT32_ARRAY", opt_code, "uint32", true);
-    Option::Factory* factory(NULL);
-    EXPECT_NO_THROW(factory = opt_def.getFactory());
-    ASSERT_TRUE(factory != NULL);
 
     OptionPtr option_v6;
     // Positive scenario, initiate the buffer with length being
@@ -605,7 +865,7 @@ TEST_F(OptionDefinitionTest, factoryUint32Array) {
     }
     // Constructor should succeed because buffer has correct size.
     EXPECT_NO_THROW(
-        option_v6 = factory(Option::V6, opt_code, buf);
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, buf);
     );
     ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IntArray<uint32_t>));
     boost::shared_ptr<Option6IntArray<uint32_t> > option_cast_v6 =
@@ -623,16 +883,85 @@ TEST_F(OptionDefinitionTest, factoryUint32Array) {
     // Provided buffer size must be greater than zero. Check if we
     // get exception if we provide zero-length buffer.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, opt_code, OptionBuffer()),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, OptionBuffer()),
+        InvalidOptionValue
     );
     // Buffer length must be multiple of data type size.
     EXPECT_THROW(
-        option_v6 = factory(Option::V6, opt_code, OptionBuffer(5)),
-        isc::OutOfRange
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, OptionBuffer(5)),
+        InvalidOptionValue
+    );
+}
+
+// The purpose of this test is to verify that definition for option that
+// comprises array of uint32 values can be created and that this definition
+// can be used to create option with an array of uint32 values.
+TEST_F(OptionDefinitionTest, uint32ArrayTokenized) {
+    // Let's define some dummy option.
+    const uint16_t opt_code = 80;
+
+    OptionDefinition opt_def("OPTION_UINT32_ARRAY", opt_code, "uint32", true);
+
+    OptionPtr option_v6;
+    std::vector<std::string> str_values;
+    str_values.push_back("123456");
+    str_values.push_back("7");
+    str_values.push_back("256");
+    str_values.push_back("1111");
+
+    EXPECT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, str_values);
+    );
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option6IntArray<uint32_t>));
+    boost::shared_ptr<Option6IntArray<uint32_t> > option_cast_v6 =
+        boost::static_pointer_cast<Option6IntArray<uint32_t> >(option_v6);
+    // Get the values from the initiated options and validate.
+    std::vector<uint32_t> values = option_cast_v6->getValues();
+    EXPECT_EQ(123456, values[0]);
+    EXPECT_EQ(7, values[1]);
+    EXPECT_EQ(256, values[2]);
+    EXPECT_EQ(1111, values[3]);
+}
+
+// The purpose of this test is to verify that the definition can be created
+// for the option that comprises string value in the UTF8 format.
+TEST_F(OptionDefinitionTest, utf8StringTokenized) {
+    // Let's create some dummy option.
+    const uint16_t opt_code = 80;
+    OptionDefinition opt_def("OPTION_WITH_STRING", opt_code, "string");
+    
+    std::vector<std::string> values;
+    values.push_back("Hello World");
+    values.push_back("this string should not be included in the option");
+    OptionPtr option_v6;
+    EXPECT_NO_THROW(
+        option_v6 = opt_def.optionFactory(Option::V6, opt_code, values);
+    );
+    ASSERT_TRUE(option_v6);
+    ASSERT_TRUE(typeid(*option_v6) == typeid(Option));
+    std::vector<uint8_t> data = option_v6->getData();
+    std::vector<uint8_t> ref_data(values[0].c_str(), values[0].c_str()
+                                  + values[0].length());
+    EXPECT_TRUE(std::equal(ref_data.begin(), ref_data.end(), data.begin()));
+}
+
+// The purpose of this test is to check that non-integer data type can't
+// be used for factoryInteger function.
+TEST_F(OptionDefinitionTest, integerInvalidType) {
+    // The template function factoryInteger<> accepts integer values only
+    // as template typename. Here we try passing different type and
+    // see if it rejects it.
+    OptionBuffer buf(1);
+    EXPECT_THROW(
+        OptionDefinition::factoryInteger<bool>(Option::V6, D6O_PREFERENCE,
+                                               buf.begin(), buf.end()),
+        isc::dhcp::InvalidDataType
     );
 }
 
+// The purpose of this test is to verify that helper methods
+// haveIA6Format and haveIAAddr6Format can be used to determine
+// IA_NA  and IAADDR option formats.
 TEST_F(OptionDefinitionTest, recognizeFormat) {
     // IA_NA option format.
     OptionDefinition opt_def1("OPTION_IA_NA", D6O_IA_NA, "record");

src/lib/dhcpsrv/addr_utilities.cc

@@ -79,7 +79,7 @@ isc::asiolink::IOAddress firstAddrInPrefix6(const isc::asiolink::IOAddress& pref
     }
 
     // Finally, let's wrap this into nice and easy IOAddress object.
-    return (isc::asiolink::IOAddress::from_bytes(AF_INET6, packed));
+    return (isc::asiolink::IOAddress::fromBytes(AF_INET6, packed));
 }
 
 /// @brief calculates the first IPv4 address in a IPv4 prefix
@@ -159,7 +159,7 @@ isc::asiolink::IOAddress lastAddrInPrefix6(const isc::asiolink::IOAddress& prefi
     }
 
     // Finally, let's wrap this into nice and easy IOAddress object.
-    return (isc::asiolink::IOAddress::from_bytes(AF_INET6, packed));
+    return (isc::asiolink::IOAddress::fromBytes(AF_INET6, packed));
 }
 
 }; // end of anonymous namespace

src/lib/dhcpsrv/alloc_engine.cc

@@ -51,7 +51,7 @@ AllocEngine::IterativeAllocator::increaseAddress(const isc::asiolink::IOAddress&
         }
     }
 
-    return (IOAddress::from_bytes(addr.getFamily(), packed));
+    return (IOAddress::fromBytes(addr.getFamily(), packed));
 }
 
 

src/lib/dns/Makefile.am

@@ -119,6 +119,7 @@ libb10_dns___la_SOURCES += tsigerror.h tsigerror.cc
 libb10_dns___la_SOURCES += tsigkey.h tsigkey.cc
 libb10_dns___la_SOURCES += tsigrecord.h tsigrecord.cc
 libb10_dns___la_SOURCES += character_string.h character_string.cc
+libb10_dns___la_SOURCES += master_loader_callbacks.h
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.cc
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec3param_common.cc

src/lib/dns/master_lexer.cc

@@ -33,9 +33,13 @@ typedef boost::shared_ptr<master_lexer_internal::InputSource> InputSourcePtr;
 }
 using namespace master_lexer_internal;
 
+
 struct MasterLexer::MasterLexerImpl {
     MasterLexerImpl() : source_(NULL), token_(Token::NOT_STARTED),
-                        paren_count_(0), last_was_eol_(false)
+                        paren_count_(0), last_was_eol_(false),
+                        has_previous_(false),
+                        previous_paren_count_(0),
+                        previous_was_eol_(false)
     {
         separators_.set('\r');
         separators_.set('\n');
@@ -91,6 +95,11 @@ struct MasterLexer::MasterLexerImpl {
     // if escaped by a backslash.  See isTokenEnd() for the bitmap size.
     std::bitset<128> separators_;
     std::bitset<128> esc_separators_;
+
+    // These are to allow restoring state before previous token.
+    bool has_previous_;
+    size_t previous_paren_count_;
+    bool previous_was_eol_;
 };
 
 MasterLexer::MasterLexer() : impl_(new MasterLexerImpl) {
@@ -116,6 +125,7 @@ MasterLexer::pushSource(const char* filename, std::string* error) {
     }
 
     impl_->source_ = impl_->sources_.back().get();
+    impl_->has_previous_ = false;
     return (true);
 }
 
@@ -123,6 +133,7 @@ void
 MasterLexer::pushSource(std::istream& input) {
     impl_->sources_.push_back(InputSourcePtr(new InputSource(input)));
     impl_->source_ = impl_->sources_.back().get();
+    impl_->has_previous_ = false;
 }
 
 void
@@ -134,6 +145,7 @@ MasterLexer::popSource() {
     impl_->sources_.pop_back();
     impl_->source_ = impl_->sources_.empty() ? NULL :
         impl_->sources_.back().get();
+    impl_->has_previous_ = false;
 }
 
 std::string
@@ -152,12 +164,53 @@ MasterLexer::getSourceLine() const {
     return (impl_->sources_.back()->getCurrentLine());
 }
 
+const MasterLexer::Token&
+MasterLexer::getNextToken(Options options) {
+    // If the source is not available
+    if (impl_->source_ == NULL) {
+        isc_throw(isc::InvalidOperation, "No source to read tokens from");
+    }
+    // Store the current state so we can restore it in ungetToken
+    impl_->previous_paren_count_ = impl_->paren_count_;
+    impl_->previous_was_eol_ = impl_->last_was_eol_;
+    impl_->source_->mark();
+    impl_->has_previous_ = true;
+    // Reset the token now. This is to check a token was actually produced.
+    // This is debugging aid.
+    impl_->token_ = Token(Token::NO_TOKEN_PRODUCED);
+    // And get the token
+
+    // This actually handles EOF internally too.
+    const State* state = State::start(*this, options);
+    if (state != NULL) {
+        state->handle(*this);
+    }
+    // Make sure a token was produced. Since this Can Not Happen, we assert
+    // here instead of throwing.
+    assert(impl_->token_.getType() != Token::ERROR ||
+           impl_->token_.getErrorCode() != Token::NO_TOKEN_PRODUCED);
+    return (impl_->token_);
+}
+
+void
+MasterLexer::ungetToken() {
+    if (impl_->has_previous_) {
+        impl_->has_previous_ = false;
+        impl_->source_->ungetAll();
+        impl_->last_was_eol_ = impl_->previous_was_eol_;
+        impl_->paren_count_ = impl_->previous_paren_count_;
+    } else {
+        isc_throw(isc::InvalidOperation, "No token to unget ready");
+    }
+}
+
 namespace {
 const char* const error_text[] = {
     "lexer not started",        // NOT_STARTED
     "unbalanced parentheses",   // UNBALANCED_PAREN
     "unexpected end of input",  // UNEXPECTED_END
-    "unbalanced quotes"         // UNBALANCED_QUOTES
+    "unbalanced quotes",        // UNBALANCED_QUOTES
+    "no token produced"         // NO_TOKEN_PRODUCED
 };
 const size_t error_text_max_count = sizeof(error_text) / sizeof(error_text[0]);
 }
@@ -201,7 +254,7 @@ class CRLF : public State {
 public:
     CRLF() {}
     virtual ~CRLF() {}          // see the base class for the destructor
-    virtual const State* handle(MasterLexer& lexer) const {
+    virtual void handle(MasterLexer& lexer) const {
         // We've just seen '\r'.  If this is part of a sequence of '\r\n',
         // we combine them as a single END-OF-LINE.  Otherwise we treat the
         // single '\r' as an EOL and continue tokeniziation from the character
@@ -218,7 +271,6 @@ public:
         }
         getLexerImpl(lexer)->token_ = Token(Token::END_OF_LINE);
         getLexerImpl(lexer)->last_was_eol_ = true;
-        return (NULL);
     }
 };
 
@@ -226,14 +278,14 @@ class String : public State {
 public:
     String() {}
     virtual ~String() {}      // see the base class for the destructor
-    virtual const State* handle(MasterLexer& lexer) const;
+    virtual void handle(MasterLexer& lexer) const;
 };
 
 class QString : public State {
 public:
     QString() {}
     virtual ~QString() {}      // see the base class for the destructor
-    virtual const State* handle(MasterLexer& lexer) const;
+    virtual void handle(MasterLexer& lexer) const;
 };
 
 // We use a common instance of a each state in a singleton-like way to save
@@ -325,7 +377,7 @@ State::start(MasterLexer& lexer, MasterLexer::Options options) {
     }
 }
 
-const State*
+void
 String::handle(MasterLexer& lexer) const {
     std::vector<char>& data = getLexerImpl(lexer)->data_;
     data.clear();
@@ -339,14 +391,14 @@ String::handle(MasterLexer& lexer) const {
             getLexerImpl(lexer)->source_->ungetChar();
             getLexerImpl(lexer)->token_ =
                 MasterLexer::Token(&data.at(0), data.size());
-            return (NULL);
+            return;
         }
         escaped = (c == '\\' && !escaped);
         data.push_back(c);
     }
 }
 
-const State*
+void
 QString::handle(MasterLexer& lexer) const {
     MasterLexer::Token& token = getLexerImpl(lexer)->token_;
     std::vector<char>& data = getLexerImpl(lexer)->data_;
@@ -357,7 +409,7 @@ QString::handle(MasterLexer& lexer) const {
         const int c = getLexerImpl(lexer)->source_->getChar();
         if (c == InputSource::END_OF_STREAM) {
             token = Token(Token::UNEXPECTED_END);
-            return (NULL);
+            return;
         } else if (c == '"') {
             if (escaped) {
                 // found escaped '"'. overwrite the preceding backslash.
@@ -366,12 +418,12 @@ QString::handle(MasterLexer& lexer) const {
                 data.back() = '"';
             } else {
                 token = MasterLexer::Token(&data.at(0), data.size(), true);
-                return (NULL);
+                return;
             }
         } else if (c == '\n' && !escaped) {
             getLexerImpl(lexer)->source_->ungetChar();
             token = Token(Token::UNBALANCED_QUOTES);
-            return (NULL);
+            return;
         } else {
             escaped = (c == '\\' && !escaped);
             data.push_back(c);

src/lib/dns/master_lexer.h

@@ -69,6 +69,14 @@ class State;
 class MasterLexer {
     friend class master_lexer_internal::State;
 public:
+    /// \brief Exception thrown when we fail to read from the input
+    /// stream or file.
+    struct ReadError : public Unexpected {
+        ReadError(const char* file, size_t line, const char* what) :
+            Unexpected(file, line, what)
+        {}
+    };
+
     class Token;       // we define it separately for better readability
 
     /// \brief Options for getNextToken.
@@ -178,6 +186,52 @@ public:
     /// \return The current line number of the source (see the description)
     size_t getSourceLine() const;
 
+    /// \brief Parse and return another token from the input.
+    ///
+    /// It reads a bit of the last opened source and produces another token
+    /// found in it.
+    ///
+    /// This method does not provide the strong exception guarantee. Generally,
+    /// if it throws, the object should not be used any more and should be
+    /// discarded. It was decided all the exceptions thrown from here are
+    /// serious enough that aborting the loading process is the only reasonable
+    /// recovery anyway, so the strong exception guarantee is not needed.
+    ///
+    /// \param options The options can be used to modify the tokenization.
+    ///     The method can be made reporting things which are usually ignored
+    ///     by this parameter. Multiple options can be passed at once by
+    ///     bitwise or (eg. option1 | option 2). See description of available
+    ///     options.
+    /// \return Next token found in the input. Note that the token refers to
+    ///     some internal data in the lexer. It is valid only until
+    ///     getNextToken or ungetToken is called. Also, the token becomes
+    ///     invalid when the lexer is destroyed.
+    /// \throw isc::InvalidOperation in case the source is not available. This
+    ///     may mean the pushSource() has not been called yet, or that the
+    ///     current source has been read past the end.
+    /// \throw ReadError in case there's problem reading from the underlying
+    ///     source (eg. I/O error in the file on the disk).
+    /// \throw std::bad_alloc in case allocation of some internal resources
+    ///     or the token fail.
+    const Token& getNextToken(Options options = NONE);
+
+    /// \brief Return the last token back to the lexer.
+    ///
+    /// The method undoes the lasts call to getNextToken(). If you call the
+    /// getNextToken() again with the same options, it'll return the same
+    /// token. If the options are different, it may return a different token,
+    /// but it acts as if the previous getNextToken() was never called.
+    ///
+    /// It is possible to return only one token back in time (you can't call
+    /// ungetToken() twice in a row without calling getNextToken() in between
+    /// successfully).
+    ///
+    /// It does not work after change of source (by pushSource or popSource).
+    ///
+    /// \throw isc::InvalidOperation If called second time in a row or if
+    ///     getNextToken() was not called since the last change of the source.
+    void ungetToken();
+
 private:
     struct MasterLexerImpl;
     MasterLexerImpl* impl_;
@@ -234,8 +288,10 @@ public:
         NOT_STARTED, ///< The lexer is just initialized and has no token
         UNBALANCED_PAREN,       ///< Unbalanced parentheses detected
         UNEXPECTED_END, ///< The lexer reaches the end of line or file
-                       /// unexpectedly
+                        /// unexpectedly
         UNBALANCED_QUOTES,      ///< Unbalanced quotations detected
+        NO_TOKEN_PRODUCED, ///< No token was produced. This means programmer
+                           /// error and should never get out of the lexer.
         MAX_ERROR_CODE ///< Max integer corresponding to valid error codes.
                        /// (excluding this one). Mainly for internal use.
     };

src/lib/dns/master_lexer_inputsource.cc

@@ -13,6 +13,7 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <dns/master_lexer_inputsource.h>
+#include <dns/master_lexer.h>
 
 #include <cerrno>
 #include <cstring>
@@ -94,7 +95,7 @@ InputSource::getChar() {
         // This has to come after the .eof() check as some
         // implementations seem to check the eofbit also in .fail().
         if (input_.fail()) {
-            isc_throw(ReadError,
+            isc_throw(MasterLexer::ReadError,
                       "Error reading from the input stream: " << getName());
         }
         buffer_.push_back(c);

src/lib/dns/master_lexer_inputsource.h

@@ -56,14 +56,6 @@ public:
         {}
     };
 
-    /// \brief Exception thrown when we fail to read from the input
-    /// stream or file.
-    struct ReadError : public Unexpected {
-        ReadError(const char* file, size_t line, const char* what) :
-            Unexpected(file, line, what)
-        {}
-    };
-
     /// \brief Exception thrown when we fail to open the input file.
     struct OpenError : public Unexpected {
         OpenError(const char* file, size_t line, const char* what) :
@@ -124,8 +116,8 @@ public:
     /// \brief Returns a single character from the input source. If end
     /// of file is reached, \c END_OF_STREAM is returned.
     ///
-    /// \throws ReadError when reading from the input stream or file
-    /// fails.
+    /// \throws MasterLexer::ReadError when reading from the input stream or
+    /// file fails.
     int getChar();
 
     /// \brief Skips backward a single character in the input

src/lib/dns/master_lexer_state.h

@@ -17,6 +17,8 @@
 
 #include <dns/master_lexer.h>
 
+#include <boost/function.hpp>
+
 namespace isc {
 namespace dns {
 
@@ -67,7 +69,7 @@ public:
     /// tokenization session.  The lexer passes a reference to itself
     /// and options given in \c getNextToken().
     ///
-    /// \throw InputSource::ReadError Unexpected I/O error
+    /// \throw MasterLexer::ReadError Unexpected I/O error
     /// \throw std::bad_alloc Internal resource allocation failure
     ///
     /// \param lexer The lexer object that holds the main context.
@@ -80,16 +82,16 @@ public:
     /// \brief Handle the process of one specific state.
     ///
     /// This method is expected to be called on the object returned by
-    /// start(), and keep called on the returned object until NULL is
-    /// returned.  The call chain will form the complete state transition.
+    /// start(). In the usual state transition design pattern, it would
+    /// return the next state. But as we noticed, we never have another
+    /// state, so we simplify it by not returning anything instead of
+    /// returning NULL every time.
     ///
-    /// \throw InputSource::ReadError Unexpected I/O error
+    /// \throw MasterLexer::ReadError Unexpected I/O error
     /// \throw std::bad_alloc Internal resource allocation failure
     ///
     /// \param lexer The lexer object that holds the main context.
-    /// \return A pointer to the next state object or NULL if the transition
-    /// is completed.
-    virtual const State* handle(MasterLexer& lexer) const = 0;
+    virtual void handle(MasterLexer& lexer) const = 0;
 
     /// \brief Types of states.
     ///

src/lib/dns/master_loader_callbacks.h

@@ -0,0 +1,122 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef MASTER_LOADER_CALLBACKS_H
+#define MASTER_LOADER_CALLBACKS_H
+
+#include <exceptions/exceptions.h>
+
+#include <string>
+#include <boost/function.hpp>
+#include <boost/shared_ptr.hpp>
+
+namespace isc {
+namespace dns {
+
+class AbstractRRset;
+typedef boost::shared_ptr<AbstractRRset> RRsetPtr;
+
+/// \brief Type of callback to add a RRset.
+///
+/// This type of callback is used by the loader to report another loaded
+/// RRset. The RRset is no longer preserved by the loader and is fully
+/// owned by the callback.
+///
+/// \param RRset The rrset to add. It does not contain the accompanying
+///     RRSIG (if the zone is signed), they are reported with separate
+///     calls to the callback.
+typedef boost::function<void(const RRsetPtr& rrset)> AddRRsetCallback;
+
+/// \brief Set of issue callbacks for a loader.
+///
+/// This holds a set of callbacks by which a loader (such as MasterLoader)
+/// can report loaded RRsets, errors and other unusual conditions.
+///
+/// All the callbacks must be set.
+class MasterLoaderCallbacks {
+public:
+    /// \brief Type of one callback to report problems.
+    ///
+    /// This is the type of one callback used to report an unusual
+    /// condition or error.
+    ///
+    /// \param source_name The name of the source where the problem happened.
+    ///     This is usually a file name.
+    /// \param source_line Position of the problem, counted in lines from the
+    ///     beginning of the source.
+    /// \param reason Human readable description of what happened.
+    typedef boost::function<void(const std::string& source_name,
+                                 size_t source_line,
+                                 const std::string& reason)> IssueCallback;
+
+    /// \brief Constructor
+    ///
+    /// Initializes the callbacks.
+    ///
+    /// \param error The error callback to use.
+    /// \param warning The warning callback to use.
+    /// \throw isc::InvalidParameter if any of the callbacks is empty.
+    MasterLoaderCallbacks(const IssueCallback& error,
+                          const IssueCallback& warning) :
+        error_(error),
+        warning_(warning)
+    {
+        if (error_.empty() || warning_.empty()) {
+            isc_throw(isc::InvalidParameter,
+                      "Empty function passed as callback");
+        }
+    }
+
+    /// \brief Call callback for serious errors
+    ///
+    /// This is called whenever there's a serious problem which makes the data
+    /// being loaded unusable. Further processing may or may not happen after
+    /// this (for example to detect further errors), but the data should not
+    /// be used.
+    ///
+    /// It calls whatever was passed to the error parameter to the constructor.
+    ///
+    /// If the caller of the loader wants to abort, it is possible to throw
+    /// from the callback, which aborts the load.
+    void error(const std::string& source_name, size_t source_line,
+               const std::string& reason)
+    {
+        error_(source_name, source_line, reason);
+    }
+
+    /// \brief Call callback for potential problems
+    ///
+    /// This is called whenever a minor problem is discovered. This might mean
+    /// the data is completely OK, it just looks suspicious.
+    ///
+    /// It calls whatever was passed to the warn parameter to the constructor.
+    ///
+    /// The loading will continue after the callback. If the caller wants to
+    /// abort (which is probably not a very good idea, since warnings
+    /// may be false positives), it is possible to throw from inside the
+    /// callback.
+    void warning(const std::string& source_name, size_t source_line,
+                 const std::string& reason)
+    {
+        warning_(source_name, source_line, reason);
+    }
+
+private:
+    IssueCallback error_, warning_;
+};
+
+}
+}
+
+#endif // LOADER_CALLBACKS_H

src/lib/dns/tests/Makefile.am

@@ -71,6 +71,7 @@ run_unittests_SOURCES += tsigerror_unittest.cc
 run_unittests_SOURCES += tsigkey_unittest.cc
 run_unittests_SOURCES += tsigrecord_unittest.cc
 run_unittests_SOURCES += character_string_unittest.cc
+run_unittests_SOURCES += master_loader_callbacks_test.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 # We shouldn't need to include BOTAN_LIBS here, but there

src/lib/dns/tests/master_lexer_state_unittest.cc

@@ -227,7 +227,7 @@ TEST_F(MasterLexerStateTest, crlf) {
 
     // 1. A sequence of \r, \n is recognized as a single 'end-of-line'
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));   // recognize '\n'
+    s_crlf.handle(lexer);   // recognize '\n'
     EXPECT_EQ(Token::END_OF_LINE, s_crlf.getToken(lexer).getType());
     EXPECT_TRUE(s_crlf.wasLastEOL(lexer));
 
@@ -235,22 +235,22 @@ TEST_F(MasterLexerStateTest, crlf) {
     // 'end-of-line'.  then there will be "initial WS"
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
     // see ' ', "unget" it
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));
+    s_crlf.handle(lexer);
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // recognize ' '
     EXPECT_EQ(Token::INITIAL_WS, s_crlf.getToken(lexer).getType());
 
     // 3. comment between \r and \n
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
     // skip comments, recognize '\n'
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));
+    s_crlf.handle(lexer);
     EXPECT_EQ(Token::END_OF_LINE, s_crlf.getToken(lexer).getType());
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // skip 'a'
+    s_string.handle(lexer); // skip 'a'
 
     // 4. \r then EOF
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // recognize '\r'
     // see EOF, then "unget" it
-    EXPECT_EQ(s_null, s_crlf.handle(lexer));
+    s_crlf.handle(lexer);
     EXPECT_EQ(s_null, State::start(lexer, common_options));  // recognize EOF
     EXPECT_EQ(Token::END_OF_FILE, s_crlf.getToken(lexer).getType());
 }
@@ -281,41 +281,41 @@ TEST_F(MasterLexerStateTest, string) {
     lexer.pushSource(ss);
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \n
+    s_string.handle(lexer); // recognize str, see \n
     EXPECT_FALSE(s_string.wasLastEOL(lexer));
     stringTokenCheck("followed-by-EOL", s_string.getToken(lexer));
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \r
+    s_string.handle(lexer); // recognize str, see \r
     stringTokenCheck("followed-by-CR", s_string.getToken(lexer));
     EXPECT_EQ(&s_crlf, State::start(lexer, common_options)); // handle \r...
-    EXPECT_EQ(s_null, s_crlf.handle(lexer)); // ...and skip it
+    s_crlf.handle(lexer); // ...and skip it
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' '
+    s_string.handle(lexer); // recognize str, see ' '
     stringTokenCheck("followed-by-space", s_string.getToken(lexer));
 
     // skip ' ', then recognize the next string
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \t
+    s_string.handle(lexer); // recognize str, see \t
     stringTokenCheck("followed-by-tab", s_string.getToken(lexer));
 
     // skip \t, then recognize the next string
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see comment
+    s_string.handle(lexer); // recognize str, see comment
     stringTokenCheck("followed-by-comment", s_string.getToken(lexer));
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n after it
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see '('
+    s_string.handle(lexer); // recognize str, see '('
     stringTokenCheck("followed-by-paren", s_string.getToken(lexer));
     EXPECT_EQ(&s_string, State::start(lexer, common_options)); // str in ()
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize the str, see ')'
+    s_string.handle(lexer); // recognize the str, see ')'
     stringTokenCheck("closing", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see EOF
+    s_string.handle(lexer); // recognize str, see EOF
     stringTokenCheck("followed-by-EOF", s_string.getToken(lexer));
 }
 
@@ -331,32 +331,32 @@ TEST_F(MasterLexerStateTest, stringEscape) {
     lexer.pushSource(ss);
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\ space", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\\ttab", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\(paren", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\)close", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("escaped\\;comment", s_string.getToken(lexer));
 
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' in mid
+    s_string.handle(lexer); // recognize str, see ' ' in mid
     stringTokenCheck("escaped\\\\", s_string.getToken(lexer));
 
     // Confirm the word that follows the escaped '\' is correctly recognized.
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see ' ' at end
+    s_string.handle(lexer); // recognize str, see ' ' at end
     stringTokenCheck("backslash", s_string.getToken(lexer));
 }
 
@@ -376,7 +376,7 @@ TEST_F(MasterLexerStateTest, quotedString) {
 
     // by default, '"' doesn't have any special meaning and part of string
     EXPECT_EQ(&s_string, State::start(lexer, common_options));
-    EXPECT_EQ(s_null, s_string.handle(lexer)); // recognize str, see \n
+    s_string.handle(lexer); // recognize str, see \n
     stringTokenCheck("\"ignore-quotes\"", s_string.getToken(lexer));
     EXPECT_EQ(s_null, State::start(lexer, common_options)); // skip \n after it
     EXPECT_TRUE(s_string.wasLastEOL(lexer));
@@ -386,35 +386,35 @@ TEST_F(MasterLexerStateTest, quotedString) {
     const MasterLexer::Options options = common_options | MasterLexer::QSTRING;
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
     EXPECT_FALSE(s_string.wasLastEOL(lexer)); // EOL is canceled due to '"'
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("quoted string", s_string.getToken(lexer), true);
 
     // Also checks other separator characters within a qstring
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("quoted()\t\rstring", s_string.getToken(lexer), true);
 
     // escape character mostly doesn't have any effect in the qstring
     // processing
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("escape\\ in quote", s_string.getToken(lexer), true);
 
     // The only exception is the quotation mark itself.  Note that the escape
     // only works on the quotation mark immediately after it.
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("escaped\"", s_string.getToken(lexer), true);
 
     // quoted '\' then '"'.  Unlike the previous case '"' shouldn't be
     // escaped.
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("escaped backslash\\\\", s_string.getToken(lexer), true);
 
     // ';' has no meaning in a quoted string (not indicating a comment)
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("no;comment", s_string.getToken(lexer), true);
 }
 
@@ -427,7 +427,7 @@ TEST_F(MasterLexerStateTest, brokenQuotedString) {
     // EOL is encountered without closing the quote
     const MasterLexer::Options options = common_options | MasterLexer::QSTRING;
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     ASSERT_EQ(Token::ERROR, s_qstring.getToken(lexer).getType());
     EXPECT_EQ(Token::UNBALANCED_QUOTES,
               s_qstring.getToken(lexer).getErrorCode());
@@ -437,12 +437,12 @@ TEST_F(MasterLexerStateTest, brokenQuotedString) {
 
     // \n is okay in a quoted string if escaped
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     stringTokenCheck("quoted\\\n", s_string.getToken(lexer), true);
 
     // EOF is encountered without closing the quote
     EXPECT_EQ(&s_qstring, State::start(lexer, options));
-    EXPECT_EQ(s_null, s_qstring.handle(lexer));
+    s_qstring.handle(lexer);
     ASSERT_EQ(Token::ERROR, s_qstring.getToken(lexer).getType());
     EXPECT_EQ(Token::UNEXPECTED_END, s_qstring.getToken(lexer).getErrorCode());
     // If we continue we'll simply see the EOF

src/lib/dns/tests/master_lexer_token_unittest.cc

@@ -142,15 +142,18 @@ TEST_F(MasterLexerTokenTest, errors) {
     EXPECT_EQ("unbalanced quotes",
               MasterLexer::Token(MasterLexer::Token::UNBALANCED_QUOTES).
               getErrorText());
+    EXPECT_EQ("no token produced",
+              MasterLexer::Token(MasterLexer::Token::NO_TOKEN_PRODUCED).
+              getErrorText());
 
     // getErrorCode/Text() isn't allowed for non number types
     EXPECT_THROW(token_num.getErrorCode(), isc::InvalidOperation);
     EXPECT_THROW(token_num.getErrorText(), isc::InvalidOperation);
 
-    // Only the pre-defined error code is accepted.  Hardcoding '4' (max code
+    // Only the pre-defined error code is accepted.  Hardcoding '5' (max code
     // + 1) is intentional; it'd be actually better if we notice it when we
     // update the enum list (which shouldn't happen too often).
-    EXPECT_THROW(MasterLexer::Token(MasterLexer::Token::ErrorCode(4)),
+    EXPECT_THROW(MasterLexer::Token(MasterLexer::Token::ErrorCode(5)),
                  isc::InvalidParameter);
 
     // Check the coexistence of "from number" and "from error-code"

src/lib/dns/tests/master_lexer_unittest.cc

@@ -15,10 +15,14 @@
 #include <exceptions/exceptions.h>
 
 #include <dns/master_lexer.h>
+#include <dns/master_lexer_state.h>
 
 #include <gtest/gtest.h>
 
 #include <boost/lexical_cast.hpp>
+#include <boost/function.hpp>
+#include <boost/scoped_ptr.hpp>
+#include <boost/bind.hpp>
 
 #include <string>
 #include <sstream>
@@ -27,6 +31,8 @@ using namespace isc::dns;
 using std::string;
 using std::stringstream;
 using boost::lexical_cast;
+using boost::scoped_ptr;
+using master_lexer_internal::State;
 
 namespace {
 
@@ -124,4 +130,158 @@ TEST_F(MasterLexerTest, invalidPop) {
     EXPECT_THROW(lexer.popSource(), isc::InvalidOperation);
 }
 
+// Test it is not possible to get token when no source is available.
+TEST_F(MasterLexerTest, noSource) {
+    EXPECT_THROW(lexer.getNextToken(), isc::InvalidOperation);
+}
+
+// Test getting some tokens
+TEST_F(MasterLexerTest, getNextToken) {
+    ss << "\n   \n\"STRING\"\n";
+    lexer.pushSource(ss);
+
+    // First, the newline should get out.
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // Then the whitespace, if we specify the option.
+    EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+    // The newline
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // The (quoted) string
+    EXPECT_EQ(MasterLexer::Token::QSTRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+
+    // And the end of line and file
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+}
+
+// Test we correctly find end of file.
+TEST_F(MasterLexerTest, eof) {
+    // Let the ss empty.
+    lexer.pushSource(ss);
+
+    // The first one is found to be EOF
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+    // And it stays on EOF for any following attempts
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+    // And we can step back one token, but that is the EOF too.
+    lexer.ungetToken();
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+}
+
+// Check we properly return error when there's an opened parentheses and no
+// closing one
+TEST_F(MasterLexerTest, getUnbalancedParen) {
+    ss << "(\"string\"";
+    lexer.pushSource(ss);
+
+    // The string gets out first
+    EXPECT_EQ(MasterLexer::Token::STRING, lexer.getNextToken().getType());
+    // Then an unbalanced parenthesis
+    EXPECT_EQ(MasterLexer::Token::UNBALANCED_PAREN,
+              lexer.getNextToken().getErrorCode());
+    // And then EOF
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+}
+
+// Check we properly return error when there's an opened quoted string and no
+// closing one
+TEST_F(MasterLexerTest, getUnbalancedString) {
+    ss << "\"string";
+    lexer.pushSource(ss);
+
+    // Then an unbalanced qstring (reported as an unexpected end)
+    EXPECT_EQ(MasterLexer::Token::UNEXPECTED_END,
+              lexer.getNextToken(MasterLexer::QSTRING).getErrorCode());
+    // And then EOF
+    EXPECT_EQ(MasterLexer::Token::END_OF_FILE, lexer.getNextToken().getType());
+}
+
+// Test ungetting tokens works
+TEST_F(MasterLexerTest, ungetToken) {
+    ss << "\n (\"string\"\n) more";
+    lexer.pushSource(ss);
+
+    // Try getting the newline
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // Return it and get again
+    lexer.ungetToken();
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // Get the string and return it back
+    EXPECT_EQ(MasterLexer::Token::QSTRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+    lexer.ungetToken();
+    // But if we change the options, it honors them
+    EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::QSTRING |
+                                 MasterLexer::INITIAL_WS).getType());
+    // Get to the "more" string
+    EXPECT_EQ(MasterLexer::Token::QSTRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+    EXPECT_EQ(MasterLexer::Token::STRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+    // Return it back. It should get inside the parentheses.
+    // Upon next attempt to get it again, the newline inside the parentheses
+    // should be still ignored.
+    lexer.ungetToken();
+    EXPECT_EQ(MasterLexer::Token::STRING,
+              lexer.getNextToken(MasterLexer::QSTRING).getType());
+}
+
+// Check ungetting token without overriding the start method. We also
+// check it works well with changing options between the calls.
+TEST_F(MasterLexerTest, ungetRealOptions) {
+    ss << "\n    \n";
+    lexer.pushSource(ss);
+    // Skip the first newline
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+
+    // If we call it the usual way, it skips up to the newline and returns
+    // it
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+
+    // Now we return it. If we call it again, but with different options,
+    // we get the initial whitespace.
+    lexer.ungetToken();
+    EXPECT_EQ(MasterLexer::Token::INITIAL_WS,
+              lexer.getNextToken(MasterLexer::INITIAL_WS).getType());
+}
+
+// Test only one token can be ungotten
+TEST_F(MasterLexerTest, ungetTwice) {
+    ss << "\n";
+    lexer.pushSource(ss);
+
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // Unget the token. It can be done once
+    lexer.ungetToken();
+    // But not twice
+    EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
+}
+
+// Test we can't unget a token before we get one
+TEST_F(MasterLexerTest, ungetBeforeGet) {
+    lexer.pushSource(ss); // Just to eliminate the missing source problem
+    EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
+}
+
+// Test we can't unget a token after a source switch, even when we got
+// something before.
+TEST_F(MasterLexerTest, ungetAfterSwitch) {
+    ss << "\n\n";
+    lexer.pushSource(ss);
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // Switch the source
+    std::stringstream ss2;
+    ss2 << "\n\n";
+    lexer.pushSource(ss2);
+    EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
+    // We can get from the new source
+    EXPECT_EQ(MasterLexer::Token::END_OF_LINE, lexer.getNextToken().getType());
+    // And when we drop the current source, we can't unget again
+    lexer.popSource();
+    EXPECT_THROW(lexer.ungetToken(), isc::InvalidOperation);
+}
+
 }

src/lib/dns/tests/master_loader_callbacks_test.cc

@@ -0,0 +1,84 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/master_loader_callbacks.h>
+#include <dns/rrset.h>
+#include <dns/name.h>
+#include <dns/rrttl.h>
+#include <dns/rrclass.h>
+
+#include <exceptions/exceptions.h>
+
+#include <gtest/gtest.h>
+#include <boost/bind.hpp>
+
+namespace {
+
+using std::string;
+using namespace isc::dns;
+
+class MasterLoaderCallbacksTest : public ::testing::Test {
+protected:
+    MasterLoaderCallbacksTest() :
+        last_was_error_(false), // Not needed, but then cppcheck complains
+        issue_called_(false),
+        rrset_(new RRset(Name("example.org"), RRClass::IN(), RRType::A(),
+                         RRTTL(3600))),
+        error_(boost::bind(&MasterLoaderCallbacksTest::checkCallback, this,
+                           true, _1, _2, _3)),
+        warning_(boost::bind(&MasterLoaderCallbacksTest::checkCallback, this,
+                             false, _1, _2, _3)),
+        callbacks_(error_, warning_)
+    {}
+
+    void checkCallback(bool error, const string& source, size_t line,
+                       const string& reason)
+    {
+        issue_called_ = true;
+        last_was_error_ = error;
+        EXPECT_EQ("source", source);
+        EXPECT_EQ(1, line);
+        EXPECT_EQ("reason", reason);
+    }
+    bool last_was_error_;
+    bool issue_called_;
+    const RRsetPtr rrset_;
+    const MasterLoaderCallbacks::IssueCallback error_, warning_;
+    MasterLoaderCallbacks callbacks_;
+};
+
+// Check the constructor rejects empty callbacks, but accepts non-empty ones
+TEST_F(MasterLoaderCallbacksTest, constructor) {
+    EXPECT_THROW(MasterLoaderCallbacks(MasterLoaderCallbacks::IssueCallback(),
+                                       warning_), isc::InvalidParameter);
+    EXPECT_THROW(MasterLoaderCallbacks(error_,
+                                       MasterLoaderCallbacks::IssueCallback()),
+                 isc::InvalidParameter);
+    EXPECT_NO_THROW(MasterLoaderCallbacks(error_, warning_));
+}
+
+// Call the issue callbacks
+TEST_F(MasterLoaderCallbacksTest, issueCall) {
+    callbacks_.error("source", 1, "reason");
+    EXPECT_TRUE(last_was_error_);
+    EXPECT_TRUE(issue_called_);
+
+    issue_called_ = false;
+
+    callbacks_.warning("source", 1, "reason");
+    EXPECT_FALSE(last_was_error_);
+    EXPECT_TRUE(issue_called_);
+}
+
+}

src/lib/log/logger_manager_impl.cc

@@ -203,7 +203,7 @@ void LoggerManagerImpl::setConsoleAppenderLayout(
         log4cplus::SharedAppenderPtr& appender)
 {
     // Create the pattern we want for the output - local time.
-    string pattern = "%D{%Y-%m-%d %H:%M:%S.%q} %-5p [%c] %m\n";
+    string pattern = "%D{%Y-%m-%d %H:%M:%S.%q} %-5p [%c/%i] %m\n";
 
     // Finally the text of the message
     auto_ptr<log4cplus::Layout> layout(new log4cplus::PatternLayout(pattern));

src/lib/log/tests/Makefile.am

@@ -127,3 +127,10 @@ check-local:
 	$(SHELL) $(abs_builddir)/local_file_test.sh
 	$(SHELL) $(abs_builddir)/logger_lock_test.sh
 	$(SHELL) $(abs_builddir)/severity_test.sh
+
+noinst_SCRIPTS  = console_test.sh
+noinst_SCRIPTS += destination_test.sh
+noinst_SCRIPTS += init_logger_test.sh
+noinst_SCRIPTS += local_file_test.sh
+noinst_SCRIPTS += logger_lock_test.sh
+noinst_SCRIPTS += severity_test.sh

src/lib/log/tests/destination_test.sh.in

@@ -20,6 +20,8 @@ echo $testname
 
 failcount=0
 tempfile=@abs_builddir@/destination_test_tempfile_$$
+destfile1_tmp=@abs_builddir@/destination_test_destfile_1_tmp_$$
+destfile2_tmp=@abs_builddir@/destination_test_destfile_2_tmp_$$
 destfile1=@abs_builddir@/destination_test_destfile_1_$$
 destfile2=@abs_builddir@/destination_test_destfile_2_$$
 
@@ -40,7 +42,11 @@ FATAL [example.beta] LOG_BAD_SEVERITY unrecognized log severity: beta_fatal
 ERROR [example.beta] LOG_BAD_DESTINATION unrecognized log destination: beta_error
 .
 rm -f $destfile1 $destfile2
-./logger_example -s error -f $destfile1 -f $destfile2
+./logger_example -s error -f $destfile1_tmp -f $destfile2_tmp
+
+# strip the pids
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile1_tmp > $destfile1
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile2_tmp > $destfile2
 
 echo -n  "   - destination 1:"
 cut -d' ' -f3- $destfile1 | diff $tempfile -
@@ -50,9 +56,16 @@ echo -n  "   - destination 2:"
 cut -d' ' -f3- $destfile2 | diff $tempfile -
 passfail $?
 
+# Tidy up.
+rm -f $tempfile $destfile1_tmp $destfile2_tmp $destfile1 $destfile2
+
 echo     "2. Two loggers, different destinations and severities"
 rm -f $destfile1 $destfile2
-./logger_example -l example -s info -f $destfile1 -l alpha -s warn -f $destfile2
+./logger_example -l example -s info -f $destfile1_tmp -l alpha -s warn -f $destfile2_tmp
+
+# strip the pids
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile1_tmp > $destfile1
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile2_tmp > $destfile2
 
 # All output for example and example.beta should have gone to destfile1.
 # Output for example.alpha should have done to destfile2.
@@ -86,6 +99,6 @@ else
 fi
 
 # Tidy up.
-rm -f $tempfile $destfile1 $destfile2
+rm -f $tempfile $destfile1_tmp $destfile2_tmp $destfile1 $destfile2
 
 exit $failcount

src/lib/log/tests/init_logger_test.sh.in

@@ -21,6 +21,7 @@ echo $testname
 
 failcount=0
 tempfile=@abs_builddir@/init_logger_test_tempfile_$$
+destfile_tmp=@abs_builddir@/init_logger_test_destfile_tmp_$$
 destfile=@abs_builddir@/init_logger_test_destfile_$$
 
 passfail() {
@@ -45,6 +46,7 @@ ERROR [bind10.log] LOG_DUPLICATE_MESSAGE_ID duplicate message ID (error) in comp
 FATAL [bind10.log] LOG_NO_MESSAGE_ID line fatal: message definition line found without a message ID
 .
 B10_LOGGER_DESTINATION=stdout B10_LOGGER_SEVERITY=DEBUG B10_LOGGER_DBGLEVEL=99 ./init_logger_test | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
     cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
@@ -58,6 +60,7 @@ ERROR [bind10.log] LOG_DUPLICATE_MESSAGE_ID duplicate message ID (error) in comp
 FATAL [bind10.log] LOG_NO_MESSAGE_ID line fatal: message definition line found without a message ID
 .
 B10_LOGGER_DESTINATION=stdout B10_LOGGER_SEVERITY=DEBUG B10_LOGGER_DBGLEVEL=50 ./init_logger_test | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
     cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
@@ -68,6 +71,7 @@ ERROR [bind10.log] LOG_DUPLICATE_MESSAGE_ID duplicate message ID (error) in comp
 FATAL [bind10.log] LOG_NO_MESSAGE_ID line fatal: message definition line found without a message ID
 .
 B10_LOGGER_DESTINATION=stdout B10_LOGGER_SEVERITY=WARN ./init_logger_test | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
     cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
@@ -77,20 +81,23 @@ echo -n  "   - stdout: "
 cat > $tempfile << .
 FATAL [bind10.log] LOG_NO_MESSAGE_ID line fatal: message definition line found without a message ID
 .
-rm -f $destfile
-B10_LOGGER_SEVERITY=FATAL B10_LOGGER_DESTINATION=stdout ./init_logger_test 1> $destfile
+rm -f $destfile_tmp $destfile
+B10_LOGGER_SEVERITY=FATAL B10_LOGGER_DESTINATION=stdout ./init_logger_test 1> $destfile_tmp
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile_tmp > $destfile
 cut -d' ' -f3- $destfile | diff $tempfile -
 passfail $?
 
 echo -n  "   - stderr: "
-rm -f $destfile
-B10_LOGGER_SEVERITY=FATAL B10_LOGGER_DESTINATION=stderr ./init_logger_test 2> $destfile
+rm -f $destfile_tmp $destfile
+B10_LOGGER_SEVERITY=FATAL B10_LOGGER_DESTINATION=stderr ./init_logger_test 2> $destfile_tmp
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile_tmp > $destfile
 cut -d' ' -f3- $destfile | diff $tempfile -
 passfail $?
 
 echo -n  "   - file: "
-rm -f $destfile
-B10_LOGGER_SEVERITY=FATAL B10_LOGGER_DESTINATION=$destfile ./init_logger_test
+rm -f $destfile_tmp $destfile
+B10_LOGGER_SEVERITY=FATAL B10_LOGGER_DESTINATION=$destfile_tmp ./init_logger_test
+sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' < $destfile_tmp > $destfile
 cut -d' ' -f3- $destfile | diff $tempfile -
 passfail $?
 
@@ -105,6 +112,6 @@ else
 fi
 
 # Tidy up.
-rm -f $tempfile $destfile
+rm -f $tempfile $destfile_tmp $destfile
 
 exit $failcount

src/lib/log/tests/local_file_test.sh.in

@@ -51,7 +51,9 @@ FATAL [example.beta] LOG_BAD_SEVERITY unrecognized log severity: beta_fatal
 ERROR [example.beta] LOG_BAD_DESTINATION unrecognized log destination: beta_error
 WARN  [example.beta] LOG_BAD_STREAM bad log console output stream: beta_warn
 .
-./logger_example -c stdout -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
+./logger_example -c stdout -s warn $localmes | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
+    cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 echo -n "2. Report error if unable to read local message file:"
@@ -66,7 +68,9 @@ ERROR [example.beta] LOG_BAD_DESTINATION unrecognized log destination: beta_erro
 WARN  [example.beta] LOG_BAD_STREAM bad log console output stream: beta_warn
 .
 rm -f $localmes
-./logger_example -c stdout -s warn $localmes | cut -d' ' -f3- | diff $tempfile -
+./logger_example -c stdout -s warn $localmes | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
+    cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 if [ $failcount -eq 0 ]; then

src/lib/log/tests/logger_lock_test.sh.in

@@ -36,7 +36,8 @@ INFO  [bind10.log] LOG_LOCK_TEST_MESSAGE this is a test message.
 LOGGER_LOCK_TEST: UNLOCK
 .
 rm -f $destfile
-B10_LOGGER_SEVERITY=INFO B10_LOGGER_DESTINATION=stdout ./logger_lock_test > $destfile
+B10_LOGGER_SEVERITY=INFO B10_LOGGER_DESTINATION=stdout ./logger_lock_test | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' > $destfile
 cut -d' ' -f3- $destfile | diff $tempfile -
 passfail $?
 

src/lib/log/tests/severity_test.sh.in

@@ -43,7 +43,9 @@ ERROR [example.beta] LOG_BAD_DESTINATION unrecognized log destination: beta_erro
 WARN  [example.beta] LOG_BAD_STREAM bad log console output stream: beta_warn
 INFO  [example.beta] LOG_READ_ERROR error reading from message file beta: info
 .
-./logger_example -c stdout | cut -d' ' -f3- | diff $tempfile -
+./logger_example -c stdout | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
+    cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 echo -n "2. Severity filter:"
@@ -53,7 +55,9 @@ ERROR [example] LOG_READING_LOCAL_FILE reading local message file dummy/file
 FATAL [example.beta] LOG_BAD_SEVERITY unrecognized log severity: beta_fatal
 ERROR [example.beta] LOG_BAD_DESTINATION unrecognized log destination: beta_error
 .
-./logger_example -c stdout -s error | cut -d' ' -f3- | diff $tempfile -
+./logger_example -c stdout -s error | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
+    cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 echo -n "3. Debug level:"
@@ -72,7 +76,9 @@ WARN  [example.beta] LOG_BAD_STREAM bad log console output stream: beta_warn
 INFO  [example.beta] LOG_READ_ERROR error reading from message file beta: info
 DEBUG [example.beta] LOG_BAD_SEVERITY unrecognized log severity: beta/25
 .
-./logger_example -c stdout -s debug -d 25 | cut -d' ' -f3- | diff $tempfile -
+./logger_example -c stdout -s debug -d 25 | \
+    sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | \
+    cut -d' ' -f3- | diff $tempfile -
 passfail $?
 
 if [ $failcount -eq 0 ]; then

src/lib/python/isc/log/tests/check_output.sh

@@ -1,3 +1,3 @@
 #!/bin/sh
 
-"$1" 2>&1 | cut -d\  -f3- | diff - "$2" 1>&2
+"$1" 2>&1 | sed -e 's/\[\([a-z0-9\.]\+\)\/\([0-9]\+\)\]/[\1]/' | cut -d\  -f3- | diff - "$2" 1>&2

tests/tools/perfdhcp/tests/test_control_unittest.cc

@@ -896,7 +896,7 @@ TEST_F(TestControlTest, Packet6) {
     }
 }
 
-TEST_F(TestControlTest, Packet4Exchange) {
+TEST_F(TestControlTest, DISABLED_Packet4Exchange) {
     // Get the local loopback interface to open socket on
     // it and test packets exchanges. We don't want to fail
     // the test if interface is not available.
@@ -939,7 +939,7 @@ TEST_F(TestControlTest, Packet4Exchange) {
     EXPECT_EQ(12, iterations_performed);
 }
 
-TEST_F(TestControlTest, Packet6Exchange) {
+TEST_F(TestControlTest, DISABLED_Packet6Exchange) {
     // Get the local loopback interface to open socket on
     // it and test packets exchanges. We don't want to fail
     // the test if interface is not available.