Part 1 of 1186 review changes

- include paths fixed
- dangling spaces removed
- many parameters are now passed by reference
- added doxygen comments for all methods in dhcp6_srv.h
- src/bin/dhcp6/Makefile.am no longer references Pkt6.{cc|h}

src/bin/dhcp6/Makefile.am

@@ -4,9 +4,7 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/lib -I$(top_builddir)/src/lib
 AM_CPPFLAGS += -I$(top_srcdir)/src/bin -I$(top_builddir)/src/bin
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/dns -I$(top_builddir)/src/lib/dns
 AM_CPPFLAGS += -I$(top_srcdir)/src/lib/cc -I$(top_builddir)/src/lib/cc
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/asiolink
+ AM_CPPFLAGS += $(BOOST_INCLUDES)
-AM_CPPFLAGS += -I$(top_builddir)/src/lib/asiolink
-AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 AM_CXXFLAGS = $(B10_CXXFLAGS)
 
@@ -31,8 +29,10 @@ spec_config.h: spec_config.h.pre
 
 BUILT_SOURCES = spec_config.h
 pkglibexec_PROGRAMS = b10-dhcp6
+
 b10_dhcp6_SOURCES = main.cc iface_mgr.cc dhcp6_srv.cc
 b10_dhcp6_SOURCES += iface_mgr.h dhcp6_srv.h
+
 b10_dhcp6_LDADD = $(top_builddir)/src/lib/dhcp/libdhcp.la
 b10_dhcp6_LDADD += $(top_builddir)/src/lib/config/libcfgclient.la
 b10_dhcp6_LDADD += $(top_builddir)/src/lib/cc/libcc.la

src/bin/dhcp6/dhcp6_srv.cc

@@ -18,7 +18,7 @@
 #include "dhcp6/dhcp6_srv.h"
 #include "dhcp/option6_ia.h"
 #include "dhcp/option6_iaaddr.h"
-#include "io_address.h"
+#include "asiolink/io_address.h"
 
 using namespace std;
 using namespace isc;
@@ -32,9 +32,9 @@ Dhcpv6Srv::Dhcpv6Srv() {
     // it may throw something if things go wrong
     IfaceMgr::instance();
 
-    if (!setServerID()) {
+    /// @todo: instantiate LeaseMgr here once it is imlpemented.
-        isc_throw(Unexpected, "Failed to set up server-id.");
+
-    }
+    setServerID();
 }
 
 Dhcpv6Srv::~Dhcpv6Srv() {
@@ -76,6 +76,9 @@ Dhcpv6Srv::run() {
             case DHCPV6_DECLINE:
                 rsp = processDecline(query);
                 break;
+            case DHCPV6_INFORMATION_REQUEST:
+                rsp = processInfRequest(query);
+                break;
             default:
                 cout << "Unknown pkt type received:"
                      << query->getType() << endl;
@@ -84,13 +87,13 @@ Dhcpv6Srv::run() {
             cout << "Received " << query->data_len_ << " bytes packet type="
                  << query->getType() << endl;
             cout << query->toText();
-            if (rsp != boost::shared_ptr<Pkt6>()) {
+            if (rsp) {
                 rsp->remote_addr_ = query->remote_addr_;
                 rsp->local_addr_ = query->local_addr_;
                 rsp->remote_port_ = DHCP6_CLIENT_PORT;
                 rsp->local_port_ = DHCP6_SERVER_PORT;
                 rsp->ifindex_ = query->ifindex_;
-                rsp->iface_ = query->iface_; 
+                rsp->iface_ = query->iface_;
                 cout << "Replying with:" << rsp->getType() << endl;
                 cout << rsp->toText();
                 cout << "----" << endl;
@@ -108,12 +111,7 @@ Dhcpv6Srv::run() {
     return (true);
 }
 
-boost::shared_ptr<Option>
+void
-Dhcpv6Srv::getServerID() {
-    return serverid_;
-}
-
-bool
 Dhcpv6Srv::setServerID() {
     /// TODO implement this for real once interface detection is done.
     /// Use hardcoded server-id for now
@@ -131,28 +129,31 @@ Dhcpv6Srv::setServerID() {
                                                      D6O_SERVERID,
                                                      srvid,
                                                      0, 14));
-    return (true);
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6> solicit) {
+Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6>& solicit) {
 
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_ADVERTISE,
                                            solicit->getTransid(),
                                            Pkt6::UDP));
 
-    // answer client's IA (this is mostly a dummy, 
+    // answer client's IA (this is mostly a dummy,
     // so let's answer only first IA and hope there is only one)
     boost::shared_ptr<Option> ia_opt = solicit->getOption(D6O_IA_NA);
     if (ia_opt) {
         // found IA
-        Option * tmp = ia_opt.get();
+        Option* tmp = ia_opt.get();
-        Option6IA * ia_req = dynamic_cast<Option6IA*> (tmp); 
+        Option6IA* ia_req = dynamic_cast<Option6IA*>(tmp);
         if (ia_req) {
-            boost::shared_ptr<Option6IA> ia_rsp(new Option6IA(Option::V6, D6O_IA_NA, ia_req->getIAID()));
+            boost::shared_ptr<Option6IA>
+                ia_rsp(new Option6IA(Option::V6, D6O_IA_NA, ia_req->getIAID()));
             ia_rsp->setT1(1500);
             ia_rsp->setT2(2600);
-            boost::shared_ptr<Option6IAAddr> addr(new Option6IAAddr(D6O_IAADDR, IOAddress("2001:db8:1234:5678::abcd"), 5000, 7000));
+            boost::shared_ptr<Option6IAAddr>
+                addr(new Option6IAAddr(D6O_IAADDR,
+                                       IOAddress("2001:db8:1234:5678::abcd"),
+                                       5000, 7000));
             ia_rsp->addOption(addr);
             reply->addOption(ia_rsp);
         }
@@ -170,7 +171,7 @@ Dhcpv6Srv::processSolicit(boost::shared_ptr<Pkt6> solicit) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6> request) {
+Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6>& request) {
     /// TODO: Implement processRequest() for real
     boost::shared_ptr<Pkt6> reply = processSolicit(request);
     reply->setType(DHCPV6_REPLY);
@@ -178,7 +179,7 @@ Dhcpv6Srv::processRequest(boost::shared_ptr<Pkt6> request) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6> renew) {
+Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6>& renew) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            renew->getTransid(),
                                            Pkt6::UDP));
@@ -186,7 +187,7 @@ Dhcpv6Srv::processRenew(boost::shared_ptr<Pkt6> renew) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6> rebind) {
+Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6>& rebind) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            rebind->getTransid(),
                                            Pkt6::UDP));
@@ -194,7 +195,7 @@ Dhcpv6Srv::processRebind(boost::shared_ptr<Pkt6> rebind) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6> confirm) {
+Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6>& confirm) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            confirm->getTransid(),
                                            Pkt6::UDP));
@@ -202,7 +203,7 @@ Dhcpv6Srv::processConfirm(boost::shared_ptr<Pkt6> confirm) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6> release) {
+Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6>& release) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            release->getTransid(),
                                            Pkt6::UDP));
@@ -210,10 +211,17 @@ Dhcpv6Srv::processRelease(boost::shared_ptr<Pkt6> release) {
 }
 
 boost::shared_ptr<Pkt6>
-Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6> decline) {
+Dhcpv6Srv::processDecline(boost::shared_ptr<Pkt6>& decline) {
     boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
                                            decline->getTransid(),
                                            Pkt6::UDP));
     return reply;
 }
 
+boost::shared_ptr<Pkt6>
+Dhcpv6Srv::processInfRequest(boost::shared_ptr<Pkt6>& infRequest) {
+    boost::shared_ptr<Pkt6> reply(new Pkt6(DHCPV6_REPLY,
+                                           infRequest->getTransid(),
+                                           Pkt6::UDP));
+    return reply;
+}

src/bin/dhcp6/dhcp6_srv.h

@@ -25,49 +25,135 @@ class Dhcpv6SrvTest_Solicit_basic_Test;
 }
 
 namespace isc {
+    /// @brief DHCPv6 server service.
+    ///
+    /// This singleton class represents DHCPv6 server. It contains all
+    /// top-level methods and routines necessary for server operation.
+    /// In particular, it instantiates IfaceMgr, loads or generates DUID
+    /// that is going to be used as server-identifier, receives incoming
+    /// packets, processes them, manages leases assignment and generates
+    /// appropriate responses.
     class Dhcpv6Srv {
     private:
-        // defined private on purpose. We don't want to have more than
-        // one copy
-        Dhcpv6Srv(const Dhcpv6Srv& src);
-        Dhcpv6Srv& operator=(const Dhcpv6Srv& src);
 
-        boost::shared_ptr<isc::dhcp::Option> getServerID();
+        /// @brief A private copy constructor.
+        ///
+        /// Creates a single Dhcpv6Srv instance and all required objects,
+        /// including interface manager (IfaceMgr). It is defined private on
+        /// purpose. We don't want to have more than one copy of Dhcpv6Srv.
+        ///
+        /// It may throw exceptions if server creation failed, e.g. due to
+        /// failures in IfaceMgr (e.g. socket creation) or unavailable
+        /// interfaces when attempting to create new DUID.
+        Dhcpv6Srv(const Dhcpv6Srv& src);
 
-        // this method sets server-identifier. it loads it from a file or
+        /// @brief A private assignment operator.
-        // generates using interface link-layer addresses (EUI-64)
+        ///
-        bool setServerID();
+        /// Note that there is no implementation of this assignment operator.
+        /// Its definition is here to prevent creation of any copies.
+        ///
+        Dhcpv6Srv& operator=(const Dhcpv6Srv& src);
 
+        /// @brief Returns server-intentifier option
+        ///
+        /// @return reference to server-id option
+        ///
+        boost::shared_ptr<isc::dhcp::Option>&
+        getServerID() { return serverid_; }
+
+        /// @brief Sets server-identifier.
+        ///
+        /// This method attempts to set server-identifier DUID. It loads it
+        /// from a file. If file load fails, it generates new DUID using
+        /// interface link-layer addresses (EUI-64) + timestamp (DUID type
+        /// duid-llt, see RFC3315, section 9.2). If there are no suitable
+        /// interfaces present, exception it thrown
+        ///
+        /// @throws isc::Unexpected Failed to read DUID file and no suitable
+        ///         interfaces for new DUID generation are detected.
+        void setServerID();
+
+        /// server DUID (to be sent in server-identifier option)
         boost::shared_ptr<isc::dhcp::Option> serverid_;
 
     public:
-        // default constructor
+        /// @brief Default constructor.
+        ///
+        /// Instantiates necessary services, required to run DHCPv6 server.
+        /// In particular, creates IfaceMgr that will be responsible for
+        /// network interaction. Will instantiate lease manager, and load
+        /// old or create new DUID.
         Dhcpv6Srv();
+
+        /// @brief Destructor. Shuts down DHCPv6 service.
         ~Dhcpv6Srv();
 
+        /// @brief Main server processing loop.
+        ///
+        /// Main server processing loop. Receives incoming packets, verifies
+        /// their correctness, generates appropriate answer (if needed) and
+        /// transmits respones.
+        ///
+        /// @return true, if being shut down gracefully, fail if experienced
+        ///         critical error.
         bool run();
 
     protected:
+        /// @brief Processes incoming SOLICIT and returns response.
+        ///
+        /// Processes received SOLICIT message and verifies that its sender
+        /// should be served. In particular IA, TA and PD options are populated
+        /// with to-be assinged addresses, temporary addresses and delegated
+        /// prefixes, respectively. In the usual 4 message exchange, server is
+        /// expected to respond with ADVERTISE message. However, if client
+        /// requests rapid-commit and server supports it, REPLY will be sent
+        /// instead of ADVERTISE and requested leases will be assigned
+        /// immediately.
+        ///
+        /// @param solicit SOLICIT message received from client
+        ///
+        /// @return ADVERTISE, REPLY message or NULL
+        ///
+        boost::shared_ptr<Pkt6>
+        processSolicit(boost::shared_ptr<Pkt6>& solicit);
+
+        /// @brief Processes incoming REQUEST and returns REPLY response.
+        ///
+        /// Processes incoming REQUEST message and verifies that its sender
+        /// should be served. In particular IA, TA and PD options are populated
+        /// with assinged addresses, temporary addresses and delegated
+        /// prefixes, respectively. Uses LeaseMgr to allocate or update existing
+        /// leases.
+        ///
+        /// @param request REQUEST message received from client
+        ///
+        /// @return REPLY message or NULL
         boost::shared_ptr<Pkt6>
-        processSolicit(boost::shared_ptr<Pkt6> solicit);
+        processRequest(boost::shared_ptr<Pkt6>& request);
 
+        /// @brief Stub function that will handle incoming RENEW messages.
         boost::shared_ptr<Pkt6>
-        processRequest(boost::shared_ptr<Pkt6> solicit);
+        processRenew(boost::shared_ptr<Pkt6>& renew);
 
+         /// @brief Stub function that will handle incoming REBIND messages.
         boost::shared_ptr<Pkt6>
-        processRenew(boost::shared_ptr<Pkt6> solicit);
+        processRebind(boost::shared_ptr<Pkt6>& rebind);
 
+        /// @brief Stub function that will handle incoming CONFIRM messages.
         boost::shared_ptr<Pkt6>
-        processRebind(boost::shared_ptr<Pkt6> solicit);
+        processConfirm(boost::shared_ptr<Pkt6>& confirm);
 
+        /// @brief Stub function that will handle incoming RELEASE messages.
         boost::shared_ptr<Pkt6>
-        processConfirm(boost::shared_ptr<Pkt6> solicit);
+        processRelease(boost::shared_ptr<Pkt6>& release);
 
+        /// @brief Stub function that will handle incoming DECLINE messages.
         boost::shared_ptr<Pkt6>
-        processRelease(boost::shared_ptr<Pkt6> solicit);
+        processDecline(boost::shared_ptr<Pkt6>& decline);
 
+        /// @brief Stub function that will handle incoming INF-REQUEST messages.
         boost::shared_ptr<Pkt6>
-        processDecline(boost::shared_ptr<Pkt6> solicit);
+        processInfRequest(boost::shared_ptr<Pkt6>& infRequest);
 
         bool shutdown;
 

src/bin/dhcp6/iface_mgr.h

@@ -17,7 +17,7 @@
 
 #include <list>
 #include <boost/shared_ptr.hpp>
-#include "io_address.h"
+#include "asiolink/io_address.h"
 #include "dhcp/pkt6.h"
 
 namespace isc {

src/lib/dhcp/Makefile.am

@@ -1,8 +1,6 @@
 SUBDIRS = . tests
 
 AM_CPPFLAGS = -I$(top_builddir)/src/lib -I$(top_srcdir)/src/lib
-AM_CPPFLAGS += -I$(top_srcdir)/src/lib/asiolink
-AM_CPPFLAGS += -I$(top_builddir)/src/lib/asiolink
 AM_CPPFLAGS += $(BOOST_INCLUDES)
 
 CLEANFILES = *.gcno *.gcda

src/lib/dhcp/option6_addrlst.cc

@@ -17,10 +17,10 @@
 #include <sstream>
 #include "exceptions/exceptions.h"
 
-#include "libdhcp.h"
+#include "asiolink/io_address.h"
-#include "option6_addrlst.h"
+#include "dhcp/libdhcp.h"
-#include "dhcp6.h"
+#include "dhcp/option6_addrlst.h"
-#include "io_address.h"
+#include "dhcp/dhcp6.h"
 
 using namespace std;
 using namespace isc;

src/lib/dhcp/option6_addrlst.h

@@ -16,8 +16,8 @@
 #define OPTION6_ADDRLST_H_
 
 #include <vector>
-#include "io_address.h"
+#include "asiolink/io_address.h"
-#include "option.h"
+#include "dhcp/option.h"
 
 namespace isc {
 namespace dhcp {

src/lib/dhcp/option6_iaaddr.cc

@@ -17,10 +17,10 @@
 #include <sstream>
 #include "exceptions/exceptions.h"
 
-#include "libdhcp.h"
+#include "dhcp/libdhcp.h"
-#include "option6_iaaddr.h"
+#include "dhcp/option6_iaaddr.h"
-#include "dhcp6.h"
+#include "dhcp/dhcp6.h"
-#include "io_address.h"
+#include "asiolink/io_address.h"
 
 using namespace std;
 using namespace isc;

src/lib/dhcp/option6_iaaddr.h

@@ -15,8 +15,8 @@
 #ifndef OPTION6_IAADDR_H_
 #define OPTION6_IAADDR_H_
 
-#include "io_address.h"
+#include "asiolink/io_address.h"
-#include "option.h"
+#include "dhcp/option.h"
 
 namespace isc {
 namespace dhcp {

src/lib/dhcp/pkt6.h

@@ -18,7 +18,7 @@
 #include <iostream>
 #include <boost/shared_ptr.hpp>
 #include <boost/shared_array.hpp>
-#include "io_address.h"
+#include "asiolink/io_address.h"
 #include "option.h"
 
 namespace isc {

src/lib/dhcp/tests/option6_ia_unittest.cc

@@ -211,13 +211,13 @@ TEST_F(Option6IATest, suboptions_unpack) {
         buf[i] = 0;
     memcpy(&buf[0], expected, 48);
 
-    Option6IA * ia;
+    Option6IA* ia = 0;
     EXPECT_NO_THROW({
-            ia = new Option6IA(Option::V6, D6O_IA_NA,
+        ia = new Option6IA(Option::V6, D6O_IA_NA,
-                               buf, 128, 4, 44);
+                           buf, 128, 4, 44);
+        cout << "Parsed option:" << endl << ia->toText() << endl;
     });
-
+    ASSERT_TRUE(ia);
-    cout << "Parsed option:" << endl << ia->toText() << endl;
 
     EXPECT_EQ(D6O_IA_NA, ia->getType());
     EXPECT_EQ(0x13579ace, ia->getIAID());
@@ -251,6 +251,4 @@ TEST_F(Option6IATest, suboptions_unpack) {
     delete ia;
 }
 
-
-
 }