[5033] Various fixes mostly cosmetic

doc/examples/kea6/with-ddns.json

@@ -39,8 +39,9 @@
       "subnet": "2001:db8:1::/64",
       "interface": "ethX"
     }
-  ]
+  ],
 
+# Enable dynamic DNS updates
   "dhcp-ddns" : {
         "enable-updates" : true,
         "server-ip" : "3001::1",
@@ -56,7 +57,7 @@
         "replace-client-name" : "when-present",
         "generated-prefix" : "test.prefix",
         "qualifying-suffix" : "test.suffix."
-  },
+  }
 
 },
 

src/lib/dhcpsrv/parsers/dhcp_parsers.cc

@@ -1335,7 +1335,7 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
         qualifying_suffix = getString(client_config, current_param);
     } catch (const std::exception& ex) {
         isc_throw(D2ClientError, "D2ClientConfig error: " << ex.what()
-                   << " " << getPosition(current_param, client_config));
+                  << " (" << getPosition(current_param, client_config) << ")");
     }
 
     // Now we check for logical errors. This repeats what is done in
@@ -1343,16 +1343,16 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
     // emit meaningful parameter position info in the error.
     if (ncr_format != dhcp_ddns::FMT_JSON) {
         isc_throw(D2ClientError, "D2ClientConfig error: NCR Format: "
-                    << dhcp_ddns::ncrFormatToString(ncr_format)
+                  << dhcp_ddns::ncrFormatToString(ncr_format)
-                    << " is not supported."
+                  << " is not supported. ("
-                    << " " << getPosition("ncr-format", client_config));
+                  << getPosition("ncr-format", client_config) << ")");
     }
 
     if (ncr_protocol != dhcp_ddns::NCR_UDP) {
         isc_throw(D2ClientError, "D2ClientConfig error: NCR Protocol: "
                   << dhcp_ddns::ncrProtocolToString(ncr_protocol)
-                  << " is not supported."
+                  << " is not supported. ("
-                  << " " << getPosition("ncr-protocol", client_config));
+                  << getPosition("ncr-protocol", client_config) << ")");
     }
 
     if (sender_ip.getFamily() != server_ip.getFamily()) {
@@ -1361,14 +1361,14 @@ D2ClientConfigParser::parse(isc::data::ConstElementPtr client_config) {
                   << " is: " << (server_ip.isV4() ? "IPv4" : "IPv6")
                   << " while sender-ip: "  << sender_ip.toText()
                   << " is: " << (sender_ip.isV4() ? "IPv4" : "IPv6")
-                  << " " << getPosition("sender-ip", client_config));
+                  << " (" << getPosition("sender-ip", client_config) << ")");
     }
 
     if (server_ip == sender_ip && server_port == sender_port) {
         isc_throw(D2ClientError, "D2ClientConfig error: server and sender cannot"
                   " share the exact same IP address/port: "
                   << server_ip.toText() << "/" << server_port
-                  << " " << getPosition("sender-ip", client_config));
+                  << " (" << getPosition("sender-ip", client_config) << ")");
     }
 
     try {

src/lib/dhcpsrv/parsers/dhcp_parsers.h

@@ -1024,16 +1024,10 @@ protected:
 /// @brief Parser for  D2ClientConfig
 ///
 /// This class parses the configuration element "dhcp-ddns" common to the
-/// spec files for both dhcp4 and dhcp6. It creates an instance of a
+/// config files for both dhcp4 and dhcp6. It creates an instance of a
 /// D2ClientConfig.
 class D2ClientConfigParser : public  isc::data::SimpleParser {
 public:
-    /// @brief Constructor
-    ///
-    D2ClientConfigParser(){};
-
-    /// @brief Destructor
-    virtual ~D2ClientConfigParser(){};
 
     /// @brief Parses a given dhcp-ddns element into D2ClientConfig.
     ///
@@ -1045,9 +1039,11 @@ public:
     /// -# qualifying-suffix
     /// -# server-ip
     /// -# server-port
+    /// -# sender-ip
+    /// -# sender-port
+    /// -# max-queue-size
     /// -# ncr-protocol
     /// -# ncr-format
-    /// -# remove-on-renew
     /// -# always-include-fqdn
     /// -# allow-client-update
     /// -# override-no-update

src/lib/dhcpsrv/srv_config.h

@@ -482,7 +482,7 @@ public:
 
     /// @brief Returns DHCP4o6 IPC port
     ///
-    /// See @ref setDhcp4o6Port or brief discussion.
+    /// See @ref setDhcp4o6Port for brief discussion.
     /// @return value of DHCP4o6 IPC port
     uint32_t getDhcp4o6Port() {
         return (dhcp4o6_port_);

src/lib/dhcpsrv/tests/dhcp_parsers_unittest.cc

@@ -458,17 +458,10 @@ public:
         ConstElementPtr d2_client = global->get("dhcp-ddns");
         /// @todo - what if it's not in global? should we add it?
         if (d2_client) {
-            // Because "dhcp-ddns" is a MapElement and global->get()
+            // Get the mutable form of d2 client config
-            // returns a ConstElementPtr, then we get a map we can't
+            ElementPtr mutable_d2 =
-            // change.  So go thru gyrations to create a non-const
+                boost::const_pointer_cast<Element>(d2_client);
-            // map, update it with default values and then replace
+            cnt += SimpleParser::setDefaults(mutable_d2, d2_client_defaults);
-            // the one in global with the new one. Ick.
-            std::map<std::string, ConstElementPtr> d2_map;
-            d2_client->getValue(d2_map);
-            ElementPtr new_map(new MapElement());
-            new_map->setValue(d2_map);
-            cnt += SimpleParser::setDefaults(new_map, d2_client_defaults);
-            global->set("dhcp-ddns", new_map);
         }
 
 

src/lib/dhcpsrv/tests/srv_config_unittest.cc

@@ -141,7 +141,9 @@ SrvConfigTest::addSubnet6(const unsigned int index) {
 
 void
 SrvConfigTest::enableDDNS(const bool enable) {
-    conf_.getD2ClientConfig()->enableUpdates(enable);
+    const D2ClientConfigPtr& d2_config = conf_.getD2ClientConfig();
+    ASSERT_TRUE(d2_config);
+    d2_config->enableUpdates(enable);
 }
 
 // Check that by default there are no logging entries

src/lib/dhcpsrv/testutils/config_result_check.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2014-2015 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
@@ -67,6 +67,12 @@ bool errorContainsPosition(ConstElementPtr error_element,
                     ++i;
                 }
 
+                // Make sure that there has been at least one digit and that the
+                // position is followed by the paren.
+                if ((i == 0) || (split_pos[2][i] != ')')) {
+                    return (false);
+                }
+
                 // All checks passed.
                 return (true);
             }