[#1548] fixed comments and code cleanup

ChangeLog

@@ -1,7 +1,7 @@
 2009.	[func]		tmark
-	Added new hook callout points: ddns4_udpate to kea_dhcp4
+	Added new hook callout points: ddns4_udpate to Kea DHCPv4
-	and ddns6_update to kea_dhcp6.  This enables use of the
+	server and ddns6_update to Kea DHCPv6 server. This enables
-	ddsn-tuning hook library.
+	use of the ddsn-tuning hook library.
 	(Gitlab #1548)
 
 2008.	[func]*		tomek

doc/sphinx/arm/hooks-ddns-tuning.rst

@@ -7,9 +7,9 @@ This hook library adds support for fine tuning various DNS update aspects.
 Currently it supports procedural host name generation. The DDNS Tuning hook
 is a premium feature.
 
-The library, which was added in Kea 2.1.4, is loaded in a
+The library, which was added in Kea 2.1.5, can be loaded by the ``kea-dhcp4``
-can be loaded by ``kea--dhcp4`` and ``kea-dhcp6`` by adding it
+and ``kea-dhcp6`` daemons by adding it to ``hooks-libraries`` element of the
-to  ``hooks-libraries`` element of the server's configuration:
+server's configuration:
 
 .. code-block:: javascript
 
@@ -55,8 +55,8 @@ expression is shown below:
 
 It is also possible to define this parameter in a subnet, using user-context mechanism.
 If defined at the subnet level, the expression applies to specific subnet only. If the
-subnet expression is defined as empty, "", it will suppresses (or disables) the use of
+subnet expression is defined as empty, "", it suppresses (or disables) the use of a
-a global expression for that subnet.  An example subnet expression is shown below:
+global expression for that subnet. An example subnet expression is shown below:
 
 .. code-block:: javascript
 
@@ -83,14 +83,15 @@ a global expression for that subnet.  An example subnet expression is shown belo
     }]
 
 .. note::
+
    The expression value above uses a slash, '\', to show line continuation. This is for
    clarity only and is not valid JSON supported by Kea parsing. The actually value has
    to be expressed in a single line.
 
 .. note::
 
-   Privacy should be taken into consideration when generating a host name. The host name is
+   Privacy should be taken into consideration when generating a host name. The host name
-   usually inserted into the DNS, which is a public system. Exposing identifiers that
+   is usually inserted into the DNS, which is a public system. Exposing identifiers that
    can be used to track devices, such as MAC address, are usually a very bad idea.
    The global expression example used MAC address for simplicity.
 
@@ -114,7 +115,7 @@ response to a client query (e.g. DISCOVER, REQUEST) is as follows:
   calculate the host name using the expression. If the calculated value is not a fully
   qualified name and there is an in-scope ddns-qualifying-suffix, append the suffix.
 
-  5. If value calculated by the hook is not an empty string and is different than the
+  5. If the value calculated by the hook is not an empty string and is different than
   the host name formed in the prior steps (1 or 2), the calculated value becomes the
   final host name.
 
@@ -144,6 +145,6 @@ response to a client query (e.g. SOLICIT, REQUEST, RENEW, REBIND) is as follows:
   calculate the host name using the expression. If the calculated value is not a fully
   qualified name and there is an in-scope ddns-qualifying-suffix, append the suffix.
 
-  6. If value calculated by the hook is not an empty string and is different than the
+  6. If the value calculated by the hook is not an empty string and is different than
   the host name formed in the prior steps (1 or 2), the calculated value becomes the
   final host name.

src/bin/dhcp4/dhcp4_hooks.dox

@@ -274,9 +274,9 @@ called before "subnet4_select".
    and various configuration values (e.g host reservations, DDNS behaviorial
    parameters, etc).  Upon return from the callout, any changes to these
    values will be applied as follows:
-   - If <b>hostname</b> has changed it will be used to update the
+   - If <b>hostname</b> has changed it will be used to update the outbound
-   outbound host name (option 12) if it exists, the output FQDN option
+   host name (option 12) if it exists, the output FQDN option (option 81)
-   (option 81) if it exists, and used as the FQDN sent in DNS updates
+   if it exists, and used as the FQDN sent in DNS updates
    - Forward DNS update(s) will be done if <b>fwd_udpate</b> is true (and
    <b>kea-dhcp-ddns</b> connectivity is enabled)
    - Reverse DNS update(s) will be done if <b>rev_udpate</b> is true (and

src/bin/dhcp4/dhcp4_messages.mes

@@ -393,7 +393,7 @@ The server will now abort processing of the packet as if it was never
 received. The lease will continue to be assigned to this client.
 
 % DHCP4_HOOK_DDNS_UPDATE A hook has updated the DDNS parameters: hostname %1=>%2, forward update %3=>%4, reverse update %5=>%6
-This message indicates that there was a hook called on ddns_update hook point
+This message indicates that there was a hook called on ddns4_update hook point
 and that hook updated the DDNS update parameters: hostname, or whether to
 conduct forward (A record) or reverse (PTR record) DDNS updates.
 

src/bin/dhcp4/dhcp4_srv.cc

@@ -94,7 +94,7 @@ struct Dhcp4Hooks {
     int hook_index_buffer4_send_;      ///< index for "buffer4_send" hook point
     int hook_index_lease4_decline_;    ///< index for "lease4_decline" hook point
     int hook_index_host4_identifier_;  ///< index for "host4_identifier" hook point
-    int hook_index_ddns4_update_;       ///< index for "ddns_update" hook point
+    int hook_index_ddns4_update_;      ///< index for "ddns4_update" hook point
 
     /// Constructor that registers hook points for DHCPv4 engine
     Dhcp4Hooks() {
@@ -2049,18 +2049,19 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
     // option. In that the server should prefer Client FQDN option and
     // ignore the Hostname option.
     try {
+        Pkt4Ptr query = ex.getQuery();
         Pkt4Ptr resp = ex.getResponse();
         Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>
-            (ex.getQuery()->getOption(DHO_FQDN));
+            (query->getOption(DHO_FQDN));
         if (fqdn) {
             LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL, DHCP4_CLIENT_FQDN_PROCESS)
-                .arg(ex.getQuery()->getLabel());
+                .arg(query->getLabel());
             processClientFqdnOption(ex);
 
         } else {
             LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL,
                       DHCP4_CLIENT_HOSTNAME_PROCESS)
-                    .arg(ex.getQuery()->getLabel());
+                    .arg(query->getLabel());
             processHostnameOption(ex);
         }
 
@@ -2103,12 +2104,10 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
         // Optionally, call a hook that may possibly override the decisions made
         // earlier.
         if (HooksManager::calloutsPresent(Hooks.hook_index_ddns4_update_)) {
-            Pkt4Ptr query = ex.getQuery();
-
             CalloutHandlePtr callout_handle = getCalloutHandle(query);
             Subnet4Ptr subnet = ex.getContext()->subnet_;
 
-            // Pass incoming packet as argument
+            // Setup the callout arguments.
             callout_handle->setArgument("query4", query);
             callout_handle->setArgument("response4", resp);
             callout_handle->setArgument("subnet4", subnet);
@@ -2128,7 +2127,8 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
             callout_handle->getArgument("fwd-update", hook_fqdn_fwd);
             callout_handle->getArgument("rev-update", hook_fqdn_rev);
 
-            // If there's anything changed by the hook, log it and then update the parameters
+            // If there's anything changed by the hook, log it and then update
+            // the parameters.
             if ((hostname != hook_hostname) || (fqdn_fwd != hook_fqdn_fwd) ||
                 (fqdn_rev != hook_fqdn_rev)) {
                 LOG_DEBUG(hooks_logger, DBGLVL_PKT_HANDLING, DHCP4_HOOK_DDNS_UPDATE)
@@ -2138,7 +2138,8 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
                 fqdn_fwd = hook_fqdn_fwd;
                 fqdn_rev = hook_fqdn_rev;
 
-                // If there's an outbound host-name option in the response we need to updated it.
+                // If there's an outbound host-name option in the response we
+                // need to updated it with the new host name.
                 OptionStringPtr hostname_opt = boost::dynamic_pointer_cast<OptionString>
                                               (resp->getOption(DHO_HOST_NAME));
                 if (hostname_opt) {

src/bin/dhcp6/dhcp6_hooks.dox

@@ -325,9 +325,9 @@ called before "subnet6_select".
    and various configuration values (e.g host reservations, DDNS behaviorial
    parameters, etc).  Upon return from the callout, any changes to these
    values will be applied as follows:
-   - If <b>hostname</b> has changed it will be used to update the outbound FQDN
+   - If <b>hostname</b> has changed it will be used to update the outbound
-   option (option 39) if it exists, and used as the FQDN sent
+   FQDN option (option 39) if it exists, and used as the FQDN sent in DNS
-   in DNS updates
+   updates
    - Forward DNS update(s) will be done if <b>fwd_udpate</b> is true (and
    <b>kea-dhcp-ddns</b> connectivity is enabled)
    - Reverse DNS update(s) will be done if <b>rev_udpate</b> is true (and

src/bin/dhcp6/dhcp6_messages.mes

@@ -445,7 +445,7 @@ book. The argument specifies the client and transaction identification
 information.
 
 % DHCP6_HOOK_DDNS_UPDATE A hook has updated the DDNS parameters: hostname %1=>%2, forward update %3=>%4, reverse update %5=>%6
-This message indicates that there was a hook called on ddns_update hook point
+This message indicates that there was a hook called on ddns6_update hook point
 and that hook updated the DDNS update parameters: hostname, or whether to
 conduct forward (A record) or reverse (PTR record) DDNS updates.
 

src/bin/dhcp6/dhcp6_srv.cc

@@ -1970,7 +1970,8 @@ Dhcpv6Srv::processClientFqdn(const Pkt6Ptr& question, const Pkt6Ptr& answer,
         if ((ctx.hostname_ != hook_hostname) || (ctx.fwd_dns_update_!= hook_fwd_dns_update) ||
             (ctx.rev_dns_update_ != hook_rev_dns_update)) {
             LOG_DEBUG(hooks_logger, DBGLVL_PKT_HANDLING, DHCP6_HOOK_DDNS_UPDATE)
-                      .arg(ctx.hostname_).arg(hook_hostname).arg(ctx.fwd_dns_update_).arg(hook_fwd_dns_update)
+                      .arg(ctx.hostname_).arg(hook_hostname)
+                      .arg(ctx.fwd_dns_update_).arg(hook_fwd_dns_update)
                       .arg(ctx.rev_dns_update_).arg(hook_rev_dns_update);
 
             // Update the FQDN option in the response.
@@ -2070,8 +2071,6 @@ Dhcpv6Srv::createNameChangeRequests(const Pkt6Ptr& answer,
                      (*l)->fqdn_fwd_ == do_fwd && (*l)->fqdn_rev_ == do_rev)) {
                     extended_only = true;
                 } else {
-                    std::cout << __LINE__ << "**** doing remove - l.hostname: " << (*l)->hostname_
-                              << ", opt-fqdn: " << opt_fqdn->getDomainName() << std::endl;
                     // Queue a CHG_REMOVE of the old data.
                     // NCR will only be created if the lease hostname is not
                     // empty and at least one of the direction flags is true