[5145b] Merged trac5126 diffs

configure.ac

@@ -201,6 +201,21 @@ for retry in "none" "--std=c++11" "--std=c++0x" "--std=c++1x" "fail"; do
 		[AC_MSG_RESULT([no])
 		 continue])
 
+	AC_MSG_CHECKING(template alias)
+	feature="template alias"
+	AC_COMPILE_IFELSE(
+		[AC_LANG_PROGRAM(
+			[template<int i>
+			 class I {
+			 public: int get() { return i; };
+			 };
+			 using Zero = I<0>;],
+			[Zero Z;
+			 return Z.get();])],
+		[AC_MSG_RESULT([yes])],
+		[AC_MSG_RESULT([no])
+		 continue])
+
 	AC_MSG_CHECKING(lambda support)
 	feature="lambda"
 	AC_COMPILE_IFELSE(
@@ -1467,11 +1482,11 @@ AC_ARG_ENABLE(generate_parser, [AC_HELP_STRING([--enable-generate-parser],
    enable_generate_parser=$enableval, enable_generate_parser=no)
 
 # Check if flex is avaible. Flex is not needed for building Kea sources,
-# unless you want to regenerate grammar in src/lib/eval
+# unless you want to regenerate grammars
 AC_PROG_LEX
 
 # Check if bison is available. Bison is not needed for building Kea sources,
-# unless you want to regenerate grammar in src/lib/eval
+# unless you want to regenerate grammars
 AC_PROG_YACC
 
 if test "x$enable_generate_parser" != "xno"; then
@@ -1485,7 +1500,7 @@ if test "x$enable_generate_parser" != "xno"; then
     fi
 
 # Ok, let's check if we have at least 3.0.0 version of the bison. The code used
-# to generate src/lib/eval parser is roughly based on bison 3.0 examples.
+# to generate parsers is roughly based on bison 3.0 examples.
    cat > bisontest.y << EOF
 %require "3.0.0"
 %token X
@@ -1599,20 +1614,28 @@ AM_COND_IF([HAVE_OPTRESET], [AC_DEFINE([HAVE_OPTRESET], [1], [Check for optreset
 
 AC_DEFINE([CONFIG_H_WAS_INCLUDED], [1], [config.h inclusion marker])
 
-AC_CONFIG_FILES([compatcheck/Makefile
+AC_CONFIG_FILES([Makefile
+                 compatcheck/Makefile
                  dns++.pc
-                 doc/design/datasrc/Makefile
+                 doc/Makefile
                  doc/design/Makefile
+                 doc/design/datasrc/Makefile
                  doc/guide/Makefile
-                 doc/Makefile
                  doc/version.ent
+                 ext/Makefile
                  ext/coroutine/Makefile
                  ext/gtest/Makefile
-                 ext/Makefile
                  m4macros/Makefile
-                 Makefile
                  src/Makefile
                  src/bin/Makefile
+                 src/bin/admin/Makefile
+                 src/bin/admin/kea-admin
+                 src/bin/admin/tests/Makefile
+                 src/bin/admin/tests/cql_tests.sh
+                 src/bin/admin/tests/data/Makefile
+                 src/bin/admin/tests/memfile_tests.sh
+                 src/bin/admin/tests/mysql_tests.sh
+                 src/bin/admin/tests/pgsql_tests.sh
                  src/bin/agent/Makefile
                  src/bin/agent/tests/Makefile
                  src/bin/agent/tests/ca_process_tests.sh
@@ -1644,15 +1667,6 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/bin/perfdhcp/Makefile
                  src/bin/perfdhcp/tests/Makefile
                  src/bin/perfdhcp/tests/testdata/Makefile
-                 src/bin/admin/Makefile
-                 src/bin/admin/kea-admin
-                 src/bin/admin/tests/Makefile
-                 src/bin/admin/tests/data/Makefile
-                 src/bin/admin/tests/memfile_tests.sh
-                 src/bin/admin/tests/mysql_tests.sh
-                 src/bin/admin/tests/pgsql_tests.sh
-                 src/bin/admin/tests/cql_tests.sh
-                 src/bin/agent/tests/test_libraries.h
                  src/hooks/Makefile
                  src/hooks/dhcp/Makefile
                  src/hooks/dhcp/user_chk/Makefile
@@ -1685,6 +1699,8 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/lib/dns/gen-rdatacode.py
                  src/lib/dns/tests/Makefile
                  src/lib/dns/tests/testdata/Makefile
+                 src/lib/eval/Makefile
+                 src/lib/eval/tests/Makefile
                  src/lib/exceptions/Makefile
                  src/lib/exceptions/tests/Makefile
                  src/lib/hooks/Makefile
@@ -1710,10 +1726,10 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/lib/process/spec_config.h.pre
                  src/lib/process/tests/Makefile
                  src/lib/process/testutils/Makefile
-                 src/lib/testutils/Makefile
-                 src/lib/testutils/dhcp_test_lib.sh
                  src/lib/stats/Makefile
                  src/lib/stats/tests/Makefile
+                 src/lib/testutils/Makefile
+                 src/lib/testutils/dhcp_test_lib.sh
                  src/lib/util/Makefile
                  src/lib/util/io/Makefile
                  src/lib/util/python/Makefile
@@ -1723,11 +1739,10 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/lib/util/threads/Makefile
                  src/lib/util/threads/tests/Makefile
                  src/lib/util/unittests/Makefile
-                 src/lib/eval/Makefile
-                 src/lib/eval/tests/Makefile
                  src/share/Makefile
                  src/share/database/Makefile
                  src/share/database/scripts/Makefile
+                 src/share/database/scripts/cql/Makefile
                  src/share/database/scripts/mysql/Makefile
                  src/share/database/scripts/mysql/upgrade_1.0_to_2.0.sh
                  src/share/database/scripts/mysql/upgrade_2.0_to_3.0.sh
@@ -1737,17 +1752,16 @@ AC_CONFIG_FILES([compatcheck/Makefile
                  src/share/database/scripts/pgsql/Makefile
                  src/share/database/scripts/pgsql/upgrade_1.0_to_2.0.sh
                  src/share/database/scripts/pgsql/upgrade_2.0_to_3.0.sh
-                 src/share/database/scripts/cql/Makefile
                  tools/Makefile
                  tools/path_replacer.sh
 ])
 
- AC_CONFIG_COMMANDS([permissions], [
+AC_CONFIG_COMMANDS([permissions], [
+           chmod +x src/bin/admin/kea-admin
            chmod +x src/bin/dhcp4/tests/dhcp4_process_tests.sh
            chmod +x src/bin/dhcp6/tests/dhcp6_process_tests.sh
            chmod +x src/bin/keactrl/keactrl
            chmod +x src/bin/keactrl/tests/keactrl_tests.sh
-           chmod +x src/bin/admin/kea-admin
            chmod +x src/lib/dns/gen-rdatacode.py
            chmod +x src/lib/log/tests/console_test.sh
            chmod +x src/lib/log/tests/destination_test.sh

src/bin/dhcp4/dhcp4to6_ipc.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -35,7 +35,7 @@ Dhcp4to6Ipc& Dhcp4to6Ipc::instance() {
 }
 
 void Dhcp4to6Ipc::open() {
-    uint32_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
+    uint16_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
     if (port == 0) {
         Dhcp4o6IpcBase::close();
         return;
@@ -45,8 +45,7 @@ void Dhcp4to6Ipc::open() {
     }
 
     int old_fd = socket_fd_;
-    socket_fd_ = Dhcp4o6IpcBase::open(static_cast<uint16_t>(port),
-                                      ENDPOINT_TYPE_V4);
+    socket_fd_ = Dhcp4o6IpcBase::open(port, ENDPOINT_TYPE_V4);
     if ((old_fd == -1) && (socket_fd_ != old_fd)) {
         IfaceMgr::instance().addExternalSocket(socket_fd_,
                                                Dhcp4to6Ipc::handler);

src/bin/dhcp4/json_config_parser.cc

@@ -341,24 +341,9 @@ public:
         cfg->setDeclinePeriod(probation_period);
 
         // Set the DHCPv4-over-DHCPv6 interserver port.
-        // @todo Change for uint16_t
-        uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port");
+        uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port");
         cfg->setDhcp4o6Port(dhcp4o6_port);
     }
-
-private:
-
-    /// @brief Returns a value converted to uint32_t
-    ///
-    /// Instantiation of getIntType() to uint32_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return an uint32_t value
-    uint32_t getUint32(isc::data::ConstElementPtr scope,
-                       const std::string& name) {
-        return (getIntType<uint32_t>(scope, name));
-    }
 };
 
 } // anonymous namespace

src/bin/dhcp4/tests/config_parser_unittest.cc

@@ -4557,14 +4557,6 @@ TEST_F(Dhcp4ParserTest, invalidClientClassDictionary) {
         " } ] \n"
         "} \n";
 
-    ConstElementPtr json;
-    ASSERT_NO_THROW(json = parseJSON(config));
-
-    ConstElementPtr status;
-    EXPECT_NO_THROW(status = configureDhcp4Server(*srv_, json));
-    ASSERT_TRUE(status);
-    checkResult(status, 1);
-
     EXPECT_THROW(parseDHCP4(config), Dhcp4ParseError);
 }
 

src/bin/dhcp6/dhcp6to4_ipc.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -34,7 +34,7 @@ Dhcp6to4Ipc& Dhcp6to4Ipc::instance() {
 }
 
 void Dhcp6to4Ipc::open() {
-    uint32_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
+    uint16_t port = CfgMgr::instance().getStagingCfg()->getDhcp4o6Port();
     if (port == 0) {
         Dhcp4o6IpcBase::close();
         return;
@@ -44,8 +44,7 @@ void Dhcp6to4Ipc::open() {
     }
 
     int old_fd = socket_fd_;
-    socket_fd_ = Dhcp4o6IpcBase::open(static_cast<uint16_t>(port),
-                                      ENDPOINT_TYPE_V6);
+    socket_fd_ = Dhcp4o6IpcBase::open(port, ENDPOINT_TYPE_V6);
     if ((old_fd == -1) && (socket_fd_ != old_fd)) {
         IfaceMgr::instance().addExternalSocket(socket_fd_,
                                                Dhcp6to4Ipc::handler);

src/bin/dhcp6/json_config_parser.cc

@@ -214,18 +214,6 @@ public:
 
 private:
 
-    /// @brief Get an uint8_t value
-    ///
-    /// Instantiation of getIntType() to uint8_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return uint8_t value
-    /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t
-    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
-        return (getIntType<uint8_t>(scope, name));
-    }
-
     /// Pointer to the created pool object.
     isc::dhcp::Pool6Ptr pool_;
 
@@ -547,24 +535,9 @@ public:
         srv_config->setDeclinePeriod(probation_period);
 
         // Set the DHCPv4-over-DHCPv6 interserver port.
-        // @todo Change for uint16_t
-        uint32_t dhcp4o6_port = getUint32(global, "dhcp4o6-port");
+        uint16_t dhcp4o6_port = getUint16(global, "dhcp4o6-port");
         srv_config->setDhcp4o6Port(dhcp4o6_port);
     }
-
-private:
-
-    /// @brief Returns a value converted to uint32_t
-    ///
-    /// Instantiation of getIntType() to uint32_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return an uint32_t value
-    uint32_t getUint32(isc::data::ConstElementPtr scope,
-                       const std::string& name) {
-        return (getIntType<uint32_t>(scope, name));
-    }
 };
 
 } // anonymous namespace

src/bin/dhcp6/tests/config_parser_unittest.cc

@@ -4969,13 +4969,6 @@ TEST_F(Dhcp6ParserTest, invalidClientClassDictionary) {
         " } ] \n"
         "} \n";
 
-    ConstElementPtr json = parseJSON(config);
-
-    ConstElementPtr status;
-    EXPECT_NO_THROW(status = configureDhcp6Server(srv_, json));
-    ASSERT_TRUE(status);
-    checkResult(status, 1);
-
     EXPECT_THROW(parseDHCP6(config), Dhcp6ParseError);
 }
 

src/lib/cc/simple_parser.cc

@@ -74,7 +74,7 @@ SimpleParser::getPosition(const std::string& name, const data::ConstElementPtr p
     }
     ConstElementPtr elem = parent->get(name);
     if (!elem) {
-        return (data::Element::ZERO_POSITION());
+        return (parent->getPosition());
     }
     return (elem->getPosition());
 }

src/lib/cc/simple_parser.h

@@ -104,9 +104,9 @@ class SimpleParser {
 
     /// @brief Utility method that returns position of an element
     ///
-    /// It's mostly useful for logging. When any necessary parameter is
-    /// missing (either parent is null or it doesn't contain specified
-    /// name) ZERO_POSITION is returned.
+    /// It's mostly useful for logging. If the element is missing
+    /// the parent position is returned or ZERO_POSITION if parent
+    /// is null.
     ///
     /// @param name position of that element will be returned
     /// @param parent parent element (optional)
@@ -203,6 +203,44 @@ protected:
                       << "' (" << getPosition(name, scope) << ")");
         }
     }
+
+    /// @brief Returns a value converted to uint32_t
+    ///
+    /// Instantiation of getIntType() to uint32_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an uint32_t value
+    /// @throw isc::dhcp::DhcpConfigError when it is not an uint32_t
+    uint32_t getUint32(isc::data::ConstElementPtr scope,
+                       const std::string& name) {
+        return (getIntType<uint32_t>(scope, name));
+    }
+
+    /// @brief Returns a value converted to uint16_t
+    ///
+    /// Instantiation of getIntType() to uint16_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an uint16_t value
+    /// @throw isc::dhcp::DhcpConfigError when it is not an uint16_t
+    uint16_t getUint16(isc::data::ConstElementPtr scope,
+                       const std::string& name) {
+        return (getIntType<uint16_t>(scope, name));
+    }
+
+    /// @brief Get an uint8_t value
+    ///
+    /// Instantiation of getIntType() to uint8_t
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return uint8_t value
+    /// @throw isc::dhcp::DhcpConfigError when it is not an uint8_t
+    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
+        return (getIntType<uint8_t>(scope, name));
+    }
 };
 
 };

src/lib/cc/tests/simple_parser_unittest.cc

@@ -58,14 +58,9 @@ public:
 
 class SimpleParserClassTest : public SimpleParser {
 public:
-    /// @brief Instantiation of getIntType for uint8_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter for error report
-    /// @return an uint8_t value
-    uint8_t getUint8(ConstElementPtr scope, const std::string& name) {
-        return (getIntType<uint8_t>(scope, name));
-    }
+
+    /// Make getUint8 public
+    using SimpleParser::getUint8;
 
     /// @brief Instantiation of getAndConvert
     ///

src/lib/dhcpsrv/parsers/client_class_def_parser.cc

@@ -62,86 +62,87 @@ void
 ClientClassDefParser::parse(ClientClassDictionaryPtr& class_dictionary,
                             ConstElementPtr class_def_cfg,
                             uint16_t family) {
+    // name is now mandatory
+    std::string name = getString(class_def_cfg, "name");
+    if (name.empty()) {
+        isc_throw(DhcpConfigError,
+                  "not empty parameter 'name' is required "
+                  << getPosition("name", class_def_cfg) << ")");
+    }
 
-    try {
-        std::string name;
-        std::string next_server_txt = "0.0.0.0";
-        std::string sname;
-        std::string filename;
-        ExpressionPtr match_expr;
-        CfgOptionPtr options(new CfgOption());
-
-        // Parse the elements that make up the client class definition.
-        BOOST_FOREACH(ConfigPair param, class_def_cfg->mapValue()) {
-            std::string entry(param.first);
-            ConstElementPtr value(param.second);
-
-            if (entry == "name") {
-                name = value->stringValue();
-
-            } else if (entry == "test") {
-                ExpressionParser parser;
-                parser.parse(match_expr, value, family);
-                
-            } else if (entry == "option-data") {
-                OptionDataListParser opts_parser(family);
-                opts_parser.parse(options, value);
-
-            } else if (entry == "next-server") {
-                next_server_txt = value->stringValue();
-
-            } else if (entry == "server-hostname") {
-                sname = value->stringValue();
-
-            } else if (entry == "boot-file-name") {
-                filename = value->stringValue();
-
-            } else {
-                isc_throw(DhcpConfigError, "invalid parameter '" << entry
-                          << "' (" << value->getPosition() << ")");
-            }
-        }
+    // Parse matching expression
+    ExpressionPtr match_expr;
+    ConstElementPtr test = class_def_cfg->get("test");
+    if (test) {
+        ExpressionParser parser;
+        parser.parse(match_expr, test, family);
+    }
 
-        // name is now mandatory
-        if (name.empty()) {
-            isc_throw(DhcpConfigError,
-                      "not empty parameter 'name' is required");
-        }
+    // Parse option data
+    CfgOptionPtr options(new CfgOption());
+    ConstElementPtr option_data = class_def_cfg->get("option-data");
+    if (option_data) {
+        OptionDataListParser opts_parser(family);
+        opts_parser.parse(options, option_data);
+    }
 
-        // Let's parse the next-server field
-        IOAddress next_server("0.0.0.0");
+    // Let's try to parse the next-server field
+    IOAddress next_server("0.0.0.0");
+    if (class_def_cfg->contains("next-server")) {
+        std::string next_server_txt = getString(class_def_cfg, "next-server");
         try {
             next_server = IOAddress(next_server_txt);
         } catch (const IOError& ex) {
-            isc_throw(DhcpConfigError, "Invalid next-server value specified: '"
-                      << next_server_txt);
+            isc_throw(DhcpConfigError,
+                      "Invalid next-server value specified: '"
+                      << next_server_txt << "' ("
+                      << getPosition("next-server", class_def_cfg) << ")");
         }
 
         if (next_server.getFamily() != AF_INET) {
             isc_throw(DhcpConfigError, "Invalid next-server value: '"
-                      << next_server_txt << "', must be IPv4 address");
+                      << next_server_txt
+                      << "', must be IPv4 address ("
+                      << getPosition("next-server", class_def_cfg) << ")");
         }
 
         if (next_server.isV4Bcast()) {
             isc_throw(DhcpConfigError, "Invalid next-server value: '"
-                      << next_server_txt << "', must not be a broadcast");
+                      << next_server_txt
+                      << "', must not be a broadcast ("
+                      << getPosition("next-server", class_def_cfg) << ")");
         }
+    }
+
+    // Let's try to parse server-hostname
+    std::string sname;
+    if (class_def_cfg->contains("server-hostname")) {
+        sname = getString(class_def_cfg, "server-hostname");
 
-        // Let's try to parse server-hostname
         if (sname.length() >= Pkt4::MAX_SNAME_LEN) {
             isc_throw(DhcpConfigError, "server-hostname must be at most "
                       << Pkt4::MAX_SNAME_LEN - 1 << " bytes long, it is "
-                      << sname.length());
+                      << sname.length() << " ("
+                      << getPosition("server-hostname", class_def_cfg) << ")");
         }
+    }
+
+    // Let's try to parse boot-file-name
+    std::string filename;
+    if (class_def_cfg->contains("boot-file-name")) {
+        filename = getString(class_def_cfg, "boot-file-name");
 
-        // Let's try to parse boot-file-name
         if (filename.length() > Pkt4::MAX_FILE_LEN) {
             isc_throw(DhcpConfigError, "boot-file-name must be at most "
                       << Pkt4::MAX_FILE_LEN - 1 << " bytes long, it is "
-                      << filename.length());
+                      << filename.length() << " ("
+                      << getPosition("boot-file-name", class_def_cfg) << ")");
         }
 
-        // Add the client class definition
+    }
+
+    // Add the client class definition
+    try {
         class_dictionary->addClass(name, match_expr, options, next_server,
                                    sname, filename);
     } catch (const std::exception& ex) {

src/lib/dhcpsrv/parsers/dbaccess_parser.h

@@ -43,7 +43,7 @@ public:
     /// @brief Constructor
     ///
     /// @param db_type Specifies database type (lease or hosts)
-    DbAccessParser(DBType db_type);
+    explicit DbAccessParser(DBType db_type);
 
     /// The destructor.
     virtual ~DbAccessParser()

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -295,10 +295,7 @@ OptionDataParser::extractSpace(ConstElementPtr parent) const {
         }
 
     } catch (std::exception& ex) {
-        // Append position of the option space parameter. Note, that in the case
-        // when 'space' was not specified a default value will be used and we
-        // should never get here. Therefore, it is ok to call getPosition for
-        // the space parameter here as this parameter will always be specified.
+        // Append position of the option space parameter.
         isc_throw(DhcpConfigError, ex.what() << " ("
                   << getPosition("space", parent) << ")");
     }
@@ -600,57 +597,31 @@ RelayInfoParser::RelayInfoParser(const Option::Universe& family)
     : family_(family) {
 };
 
+// Can't use a constructor as a function
+namespace {
+IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); }
+};
+
+IOAddress
+RelayInfoParser::getIOAddress(ConstElementPtr scope,
+                              const std::string& name) {
+    return (getAndConvert<IOAddress,
+            buildIOAddress>(scope, name, "address"));
+}
+
 void
 RelayInfoParser::parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg,
                        ConstElementPtr relay_info) {
-    // Let's start with some sanity checks.
-    if (!relay_info || !cfg) {
-        isc_throw(DhcpConfigError, "Logic error: RelayInfoParser::parse() called "
-                  "with at least one NULL parameter.");
-    }
-
-    if (relay_info->getType() != Element::map) {
-        isc_throw(DhcpConfigError, "Configuration error: RelayInfoParser::parse() "
-                  "called with non-map parameter");
-    }
-
-    // Now create the default value.
-    isc::asiolink::IOAddress ip(family_ == Option::V4 ? IOAddress::IPV4_ZERO_ADDRESS()
-                                : IOAddress::IPV6_ZERO_ADDRESS());
-
-    // Now iterate over all parameters. Currently there's only one supported
-    // parameter, so it should be an easy thing to check.
-    bool ip_address_specified = false;
-    BOOST_FOREACH(ConfigPair param, relay_info->mapValue()) {
-        if (param.first == "ip-address") {
-            ip_address_specified = true;
-
-            try {
-                ip = asiolink::IOAddress(param.second->stringValue());
-            } catch (...)  {
-                isc_throw(DhcpConfigError, "Failed to parse ip-address "
-                          "value: " << param.second
-                          << " (" << param.second->getPosition() << ")");
-            }
-
-            // Check if the address family matches.
-            if ( (ip.isV4() && family_ != Option::V4) ||
-                 (ip.isV6() && family_ != Option::V6) ) {
-                isc_throw(DhcpConfigError, "ip-address field " << ip.toText()
-                          << " does not have IP address of expected family type: "
-                          << (family_ == Option::V4 ? "IPv4" : "IPv6")
-                          << " (" << param.second->getPosition() << ")");
-            }
-        } else {
-            isc_throw(NotImplemented,
-                      "parser error: RelayInfoParser parameter not supported: "
-                      << param.second);
-        }
-    }
-
-    if (!ip_address_specified) {
-        isc_throw(DhcpConfigError, "'relay' specified, but mandatory 'ip-address' "
-                  "paramter in it is missing");
+    // There is only one parameter which is mandatory
+    IOAddress ip = getIOAddress(relay_info, "ip-address");
+
+    // Check if the address family matches.
+    if ((ip.isV4() && family_ != Option::V4) ||
+        (ip.isV6() && family_ != Option::V6) ) {
+        isc_throw(DhcpConfigError, "ip-address field " << ip.toText()
+                  << " does not have IP address of expected family type: "
+                  << (family_ == Option::V4 ? "IPv4" : "IPv6")
+                  << " (" << getPosition("ip-address", relay_info) << ")");
     }
 
     // Ok, we're done with parsing. Let's store the result in the structure
@@ -914,17 +885,10 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
     try {
         std::string hr_mode = getString(params, "reservation-mode");
         subnet_->setHostReservationMode(hrModeFromText(hr_mode));
-    } catch (const BadValue& ex) {
-        ConstElementPtr mode = params->get("reservation-mode");
-        string pos;
-        if (mode) {
-            pos = mode->getPosition().str();
-        } else {
-            pos = params->getPosition().str();
-        }
-        isc_throw(DhcpConfigError, "Failed to process specified value "
+    } catch (const BadValue& ex) { 
+       isc_throw(DhcpConfigError, "Failed to process specified value "
                   " of reservation-mode parameter: " << ex.what()
-                  << "(" << pos << ")");
+                  << "(" << getPosition("reservation-mode", params) << ")");
     }
 
     // Try setting up client class.
@@ -943,17 +907,6 @@ SubnetConfigParser::createSubnet(ConstElementPtr params) {
 
 //**************************** D2ClientConfigParser **********************
 
-uint32_t
-D2ClientConfigParser::getUint32(ConstElementPtr scope,
-                                const std::string& name) {
-    return (getIntType<uint32_t>(scope, name));
-}
-
-// Can't use a constructor as a function
-namespace {
-IOAddress buildIOAddress(const std::string& str) { return (IOAddress(str)); }
-};
-
 IOAddress
 D2ClientConfigParser::getIOAddress(ConstElementPtr scope,
                                    const std::string& name) {

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -365,7 +365,7 @@ public:
     /// @brief Constructor.
     ///
     /// @param address_family Address family: @c AF_INET or @c AF_INET6.
-    OptionDataParser(const uint16_t address_family);
+    explicit OptionDataParser(const uint16_t address_family);
 
     /// @brief Parses ElementPtr containing option definition
     ///
@@ -477,7 +477,7 @@ public:
     /// @brief Constructor.
     ///
     /// @param address_family Address family: @c AF_INET or AF_INET6
-    OptionDataListParser(const uint16_t address_family);
+    explicit OptionDataListParser(const uint16_t address_family);
 
     /// @brief Parses a list of options, instantiates them and stores in cfg
     ///
@@ -620,7 +620,7 @@ public:
 
     /// @brief constructor
     /// @param family specifies protocol family (IPv4 or IPv6)
-    RelayInfoParser(const isc::dhcp::Option::Universe& family);
+    explicit RelayInfoParser(const isc::dhcp::Option::Universe& family);
 
     /// @brief parses the actual relay parameters
     ///
@@ -632,7 +632,18 @@ public:
     void parse(const isc::dhcp::Subnet::RelayInfoPtr& cfg,
                isc::data::ConstElementPtr relay_info);
 
-protected:
+private:
+
+    /// @brief Returns a value converted to IOAddress
+    ///
+    /// Instantiation of getAndConvert() to IOAddress
+    ///
+    /// @param scope specified parameter will be extracted from this scope
+    /// @param name name of the parameter
+    /// @return an IOAddress value
+    isc::asiolink::IOAddress
+    getIOAddress(isc::data::ConstElementPtr scope, const std::string& name);
+
     /// Protocol family (IPv4 or IPv6)
     Option::Universe family_;
 };
@@ -664,7 +675,7 @@ public:
     /// @brief constructor
     ///
     /// @param family address family: @c AF_INET or @c AF_INET6
-    SubnetConfigParser(uint16_t family);
+    explicit SubnetConfigParser(uint16_t family);
 
     /// @brief virtual destructor (does nothing)
     virtual ~SubnetConfigParser() { }
@@ -776,16 +787,6 @@ public:
 
 private:
 
-    /// @brief Returns a value converted to uint32_t
-    ///
-    /// Instantiation of getIntType() to uint32_t
-    ///
-    /// @param scope specified parameter will be extracted from this scope
-    /// @param name name of the parameter
-    /// @return an uint32_t value
-    uint32_t
-    getUint32(isc::data::ConstElementPtr scope, const std::string& name);
-
     /// @brief Returns a value converted to IOAddress
     ///
     /// Instantiation of getAndConvert() to IOAddress

src/lib/dhcpsrv/parsers/duid_config_parser.cc

@@ -24,107 +24,62 @@ namespace isc {
 namespace dhcp {
 
 void
-DUIDConfigParser::parse(const CfgDUIDPtr& cfg, isc::data::ConstElementPtr duid_configuration) {
-    if (!cfg) {
-        isc_throw(DhcpConfigError, "Must provide valid pointer to cfg when parsing duid");
-    }
-
-    bool type_present = false;
-    BOOST_FOREACH(ConfigPair element, duid_configuration->mapValue()) {
-        try {
-            if (element.first == "type") {
-                type_present = true;
-                setType(cfg, element.second->stringValue());
-            } else if (element.first == "identifier") {
-                setIdentifier(cfg, element.second->stringValue());
-            } else if (element.first == "htype") {
-                setHType(cfg, element.second->intValue());
-            } else if (element.first == "time") {
-                setTime(cfg, element.second->intValue());
-            } else if (element.first == "enterprise-id") {
-                setEnterpriseId(cfg, element.second->intValue());
-            } else if (element.first == "persist") {
-                setPersist(cfg, element.second->boolValue());
-            } else {
-                isc_throw(DhcpConfigError, "unsupported configuration "
-                          "parameter '" << element.first << "'");
-            }
-        } catch (const std::exception& ex) {
-            // Append position.
-            isc_throw(DhcpConfigError, ex.what() << " ("
-                      << element.second->getPosition() << ")");
+DUIDConfigParser::parse(const CfgDUIDPtr& cfg,
+                        isc::data::ConstElementPtr duid_configuration) {
+
+    std::string param;
+    try {
+        param = "type";
+        std::string duid_type = getString(duid_configuration, "type");
+        // Map DUID type represented as text into numeric value.
+        DUID::DUIDType numeric_type = DUID::DUID_UNKNOWN;
+        if (duid_type == "LLT") {
+            numeric_type = DUID::DUID_LLT;
+        } else if (duid_type == "EN") {
+            numeric_type = DUID::DUID_EN;
+        } else if (duid_type == "LL") {
+            numeric_type = DUID::DUID_LL;
+        } else {
+            isc_throw(BadValue, "unsupported DUID type '"
+                      << duid_type << "'. Expected: LLT, EN or LL");
         }
-    }
-
-    // "type" is mandatory
-    if (!type_present) {
-        isc_throw(DhcpConfigError, "mandatory parameter \"type\" not specified"
-                  " for the DUID configuration ("
-                  << duid_configuration->getPosition() << ")");
-    }
-
-    LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIGURE_SERVERID);
-}
 
-void
-DUIDConfigParser::setType(const CfgDUIDPtr& cfg, const std::string& duid_type) const {
-    // Map DUID type represented as text into numeric value.
-    DUID::DUIDType numeric_type = DUID::DUID_UNKNOWN;
-    if (duid_type == "LLT") {
-        numeric_type = DUID::DUID_LLT;
-    } else if (duid_type == "EN") {
-        numeric_type = DUID::DUID_EN;
-    } else if (duid_type == "LL") {
-        numeric_type = DUID::DUID_LL;
-    } else {
-        isc_throw(DhcpConfigError, "unsupported DUID type '"
-                  << duid_type << "'. Expected: LLT, EN or LL");
-    }
-
-    cfg->setType(static_cast<DUID::DUIDType>(numeric_type));
-}
+        cfg->setType(static_cast<DUID::DUIDType>(numeric_type));
 
-void
-DUIDConfigParser::setIdentifier(const CfgDUIDPtr& cfg, const std::string& identifier) const {
-    cfg->setIdentifier(identifier);
-}
-
-void
-DUIDConfigParser::setHType(const CfgDUIDPtr& cfg, const int64_t htype) const {
-    checkRange<uint16_t>("htype", htype);
-    cfg->setHType(static_cast<uint16_t>(htype));
-}
+        param = "identifier";
+        if (duid_configuration->contains(param)) {
+            cfg->setIdentifier(getString(duid_configuration, param));
+        }
 
-void
-DUIDConfigParser::setTime(const CfgDUIDPtr& cfg, const int64_t new_time) const {
-    checkRange<uint32_t>("time", new_time);
-    cfg->setTime(static_cast<uint32_t>(new_time));
-}
+        param = "htype";
+        if (duid_configuration->contains(param)) {
+            cfg->setHType(getUint16(duid_configuration, param));
+        }
 
-void
-DUIDConfigParser::setEnterpriseId(const CfgDUIDPtr& cfg, const int64_t enterprise_id) const {
-    checkRange<uint32_t>("enterprise-id", enterprise_id);
-    cfg->setEnterpriseId(static_cast<uint32_t>(enterprise_id));
-}
+        param = "time";
+        if (duid_configuration->contains(param)) {
+            cfg->setTime(getUint32(duid_configuration, param));
+        }
 
-void
-DUIDConfigParser::setPersist(const CfgDUIDPtr& cfg, const bool persist) {
-    cfg->setPersist(persist);
-}
+        param = "enterprise-id";
+        if (duid_configuration->contains(param)) {
+            cfg->setEnterpriseId(getUint32(duid_configuration, param));
+        }
 
-template<typename NumericType>
-void
-DUIDConfigParser::checkRange(const std::string& parameter_name,
-                             const int64_t parameter_value) const {
-    if ((parameter_value < 0) ||
-        (parameter_value > std::numeric_limits<NumericType>::max())) {
-        isc_throw(DhcpConfigError, "out of range value '" << parameter_value
-                  << "' specified for parameter '" << parameter_name
-                  << "'; expected value in range of [0.."
-                  << std::numeric_limits<NumericType>::max() << "]");
+        param = "persist";
+        if (duid_configuration->contains(param)) {
+            cfg->setPersist(getBoolean(duid_configuration, param));
+        }
+    } catch (const DhcpConfigError&) {
+        throw;
+    } catch (const std::exception& ex) {
+        // Append position.
+        isc_throw(DhcpConfigError, ex.what() << " ("
+                  << getPosition(param, duid_configuration) << ")");
     }
-}
 
+    LOG_WARN(dhcpsrv_logger, DHCPSRV_CFGMGR_CONFIGURE_SERVERID);
+}
 
 } // end of namespace isc::dhcp
 } // end of namespace isc

src/lib/dhcpsrv/parsers/duid_config_parser.h

@@ -33,57 +33,6 @@ public:
     ///
     /// @throw DhcpConfigError If the configuration is invalid.
     void parse(const CfgDUIDPtr& cfg, isc::data::ConstElementPtr duid_configuration);
-private:
-
-    /// @brief Validate and set DUID type.
-    ///
-    /// @param cfg parsed information will be stored here
-    /// @param duid_type DUID type in textual format.
-    void setType(const CfgDUIDPtr& cfg, const std::string& duid_type) const;
-
-    /// @brief Validate and set identifier.
-    ///
-    /// @param cfg parsed information will be stored here
-    /// @param identifier Identifier.
-    void setIdentifier(const CfgDUIDPtr& cfg, const std::string& identifier) const;
-
-    /// @brief Validate and set hardware type.
-    ///
-    /// @param cfg parsed information will be stored here
-    /// @param htype Hardware type.
-    void setHType(const CfgDUIDPtr& cfg, const int64_t htype) const;
-
-    /// @brief Validate and set time value.
-    ///
-    /// @param cfg parsed information will be stored here
-    /// @param new_time Time value to be used for DUID.
-    void setTime(const CfgDUIDPtr& cfg, const int64_t new_time) const;
-
-    /// @brief Validate and set enterprise id.
-    ///
-    /// @param cfg parsed information will be stored here
-    /// @param enterprise_id Enterprise id.
-    void setEnterpriseId(const CfgDUIDPtr& cfg, const int64_t enterprise_id) const;
-
-    /// @brief Set persistence flag.
-    ///
-    /// @param cfg parsed information will be stored here
-    /// @param persist A boolean value indicating if the server
-    /// identifier should be stored on the disk (if true) or
-    /// not (if false).
-    void setPersist(const CfgDUIDPtr& cfg, const bool persist);
-
-    /// @brief Verifies if the specified parameter is in range.
-    ///
-    /// Each numeric value must be in range of [0 .. max_value], where
-    /// max_value is a maximum value for the numeric type used for this
-    /// parameter.
-    ///
-    /// @param parameter_name Parameter name.
-    /// @tparam Numeric type of the specified parameter.
-    template<typename NumericType>
-    void checkRange(const std::string& parameter_name,
-                    const int64_t parameter_value) const;
 };
 
 }

src/lib/dhcpsrv/parsers/expiration_config_parser.cc

@@ -20,43 +20,46 @@ void
 ExpirationConfigParser::parse(ConstElementPtr expiration_config) {
     CfgExpirationPtr cfg = CfgMgr::instance().getStagingCfg()->getCfgExpiration();
 
-    BOOST_FOREACH(ConfigPair config_element, expiration_config->mapValue()) {
+    std::string param;
 
-        // Get parameter name and value.
-        std::string param_name = config_element.first;
-        ConstElementPtr param_value = config_element.second;
-
-        try {
-            // Set configuration parameters.
-            if (param_name == "reclaim-timer-wait-time") {
-                cfg->setReclaimTimerWaitTime(param_value->intValue());
-
-            } else if (param_name == "flush-reclaimed-timer-wait-time") {
-                cfg->setFlushReclaimedTimerWaitTime(param_value->intValue());
-
-            } else if (param_name == "hold-reclaimed-time") {
-                cfg->setHoldReclaimedTime(param_value->intValue());
+    try {
+        param = "reclaim-timer-wait-time";
+        if (expiration_config->contains(param)) {
+            cfg->setReclaimTimerWaitTime(getInteger(expiration_config, param));
+        }
 
-            } else if (param_name == "max-reclaim-leases") {
-                cfg->setMaxReclaimLeases(param_value->intValue());
+        param = "flush-reclaimed-timer-wait-time";
+        if (expiration_config->contains(param)) {
+            cfg->setFlushReclaimedTimerWaitTime(getInteger(expiration_config,
+                                                           param));
+        }
 
-            } else if (param_name == "max-reclaim-time") {
-                cfg->setMaxReclaimTime(param_value->intValue());
+        param = "hold-reclaimed-time";
+        if (expiration_config->contains(param)) {
+            cfg->setHoldReclaimedTime(getInteger(expiration_config, param));
+        }
 
-            } else if (param_name == "unwarned-reclaim-cycles") {
-                cfg->setUnwarnedReclaimCycles(param_value->intValue());
+        param = "max-reclaim-leases";
+        if (expiration_config->contains(param)) {
+            cfg->setMaxReclaimLeases(getInteger(expiration_config, param));
+        }                          
 
-            } else {
-                isc_throw(DhcpConfigError, "unsupported parameter '"
-                          << param_name << "'");
-            }
+        param = "max-reclaim-time";
+        if (expiration_config->contains(param)) {
+            cfg->setMaxReclaimTime(getInteger(expiration_config, param));
+        }
 
-        } catch (const std::exception& ex) {
-            // Append position of the configuration parameter to the error
-            // message.
-            isc_throw(DhcpConfigError, ex.what() << " ("
-                      << param_value->getPosition() << ")");
+        param = "unwarned-reclaim-cycles";
+        if (expiration_config->contains(param)) {
+            cfg->setUnwarnedReclaimCycles(
+                getInteger(expiration_config, param));
         }
+    } catch (const DhcpConfigError&) {
+        throw;
+    } catch (const std::exception& ex) {
+        // Append position of the configuration parameter to the error message.
+        isc_throw(DhcpConfigError, ex.what() << " ("
+                  << getPosition(param, expiration_config) << ")");
     }
 }
 

src/lib/dhcpsrv/parsers/ifaces_config_parser.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2015-2016 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2015-2017 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -29,7 +29,7 @@ public:
     /// @brief Constructor
     ///
     /// @param protocol AF_INET for DHCPv4 and AF_INET6 for DHCPv6.
-    IfacesConfigParser(const uint16_t protocol);
+    explicit IfacesConfigParser(const uint16_t protocol);
 
     /// @brief Parses content of the "interfaces-config".
     ///

src/lib/dhcpsrv/srv_config.h

@@ -507,7 +507,7 @@ public:
     /// this socket is bound and connected to this port and port + 1
     ///
     /// @param port port and port + 1 to use
-    void setDhcp4o6Port(uint32_t port) {
+    void setDhcp4o6Port(uint16_t port) {
         /// @todo: Port is supposed to be uint16_t, not uint32_t
         dhcp4o6_port_ = port;
     }
@@ -516,7 +516,7 @@ public:
     ///
     /// See @ref setDhcp4o6Port for brief discussion.
     /// @return value of DHCP4o6 IPC port
-    uint32_t getDhcp4o6Port() {
+    uint16_t getDhcp4o6Port() {
         return (dhcp4o6_port_);
     }
 
@@ -624,7 +624,7 @@ private:
     ///
     /// DHCPv4-over-DHCPv6 uses a UDP socket for interserver communication,
     /// this socket is bound and connected to this port and port + 1
-    uint32_t dhcp4o6_port_;
+    uint16_t dhcp4o6_port_;
 
     D2ClientConfigPtr d2_client_config_;
 };

src/lib/dhcpsrv/tests/client_class_def_parser_unittest.cc

@@ -459,20 +459,6 @@ TEST_F(ClientClassDefParserTest, blankClassName) {
                  DhcpConfigError);
 }
 
-
-// Verifies that a class with an unknown element, fails to parse.
-TEST_F(ClientClassDefParserTest, unknownElement) {
-    std::string cfg_text =
-        "{ \n"
-        "    \"name\": \"one\", \n"
-        "    \"bogus\": \"bad\" \n"
-        "} \n";
-
-    ClientClassDefPtr cclass;
-    ASSERT_THROW(cclass = parseClientClassDef(cfg_text, AF_INET),
-                 DhcpConfigError);
-}
-
 // Verifies that a class with an invalid expression, fails to parse.
 TEST_F(ClientClassDefParserTest, invalidExpression) {
     std::string cfg_text =
@@ -565,22 +551,6 @@ TEST_F(ClientClassDefListParserTest, duplicateClass) {
                  DhcpConfigError);
 }
 
-// Verifies that a class list containing an invalid class entry, fails to
-// parse.
-TEST_F(ClientClassDefListParserTest, invalidClass) {
-    std::string cfg_text =
-        "[ \n"
-        "   { \n"
-        "       \"name\": \"one\", \n"
-        "       \"bogus\": \"bad\" \n"
-        "   } \n"
-        "] \n";
-
-    ClientClassDictionaryPtr dictionary;
-    ASSERT_THROW(dictionary = parseClientClassDefList(cfg_text, AF_INET6),
-                 DhcpConfigError);
-}
-
 // Test verifies that without any class specified, the fixed fields have their
 // default, empty value.
 // @todo same with AF_INET6

src/lib/dhcpsrv/tests/expiration_config_parser_unittest.cc

@@ -221,15 +221,6 @@ TEST_F(ExpirationConfigParserTest, otherParameters) {
     EXPECT_EQ(20, cfg->getUnwarnedReclaimCycles());
 }
 
-// This test verifies that the exception is thrown if unsupported
-// parameter is specified.
-TEST_F(ExpirationConfigParserTest, invalidParameter) {
-   addParam("reclaim-timer-wait-time", 20);
-   addParam("invalid-parameter", 20);
-
-   EXPECT_THROW(renderConfig(), DhcpConfigError);
-}
-
 // This test verifies that negative parameter values are not allowed.
 TEST_F(ExpirationConfigParserTest, outOfRangeValues) {
     testOutOfRange("reclaim-timer-wait-time",