[5317] Addressed review comments.

diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
index 918455df2b67b62bcfb30999cd34fcf3663ab795..cc3d3da9b94eb294be5e0203edf0490ab20ad22d 100644 (file)
--- a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
+++ b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
@@ -130,7 +130,7 @@ public:
         // make the actual configuration text.
         std::string config_txt = header + socket_path_  + footer;
 
-        server_.reset(new NakedControlledDhcpv4Srv());
+        ASSERT_NO_THROW(server_.reset(new NakedControlledDhcpv4Srv()));
 
         ConstElementPtr config;
         ASSERT_NO_THROW(config = parseDHCP4(config_txt));
@@ -190,11 +190,14 @@ public:
         client.reset(new UnixControlClient());
         ASSERT_TRUE(client);
 
-        // Connect.
+        // Connect to the server. This is expected to trigger server's acceptor
+        // handler when IOService::poll() is run.
         ASSERT_TRUE(client->connectToServer(socket_path_));
         ASSERT_NO_THROW(getIOService()->poll());
 
-        // Send the command.
+        // Send the command. This will trigger server's handler which receives
+        // data over the unix domain socket. The server will start sending
+        // response to the client.
         ASSERT_TRUE(client->sendCommand(command));
         ASSERT_NO_THROW(getIOService()->poll());
 
@@ -683,7 +686,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configSet) {
     EXPECT_EQ("{ \"result\": 0, \"text\": \"Configuration successful.\" }",
               response);
 
-    /// Check that the config was indeed applied.
+    // Check that the config was indeed applied.
     const Subnet4Collection* subnets =
         CfgMgr::instance().getCurrentCfg()->getCfgSubnets4()->getAll();
     EXPECT_EQ(1, subnets->size());
@@ -1080,7 +1083,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, concurrentConnections) {
     ASSERT_TRUE(client1);
 
     boost::scoped_ptr<UnixControlClient> client2(new UnixControlClient());
-    ASSERT_TRUE(client1);
+    ASSERT_TRUE(client2);
 
     // Client 1 connects.
     ASSERT_TRUE(client1->connectToServer(socket_path_));

diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
index eae45a4a309bedf319154b17b7fffcc852a41063..a477fdefdbafdb0e215ca45fbe02ccc009a14291 100644 (file)
--- a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
+++ b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
@@ -200,11 +200,14 @@ public:
         client.reset(new UnixControlClient());
         ASSERT_TRUE(client);
 
-        // Connect.
+        // Connect to the server. This is expected to trigger server's acceptor
+        // handler when IOService::poll() is run.
         ASSERT_TRUE(client->connectToServer(socket_path_));
         ASSERT_NO_THROW(getIOService()->poll());
 
-        // Send the command.
+        // Send the command. This will trigger server's handler which receives
+        // data over the unix domain socket. The server will start sending
+        // response to the client.
         ASSERT_TRUE(client->sendCommand(command));
         ASSERT_NO_THROW(getIOService()->poll());
 
@@ -1102,7 +1105,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, concurrentConnections) {
     ASSERT_TRUE(client1);
 
     boost::scoped_ptr<UnixControlClient> client2(new UnixControlClient());
-    ASSERT_TRUE(client1);
+    ASSERT_TRUE(client2);
 
     // Client 1 connects.
     ASSERT_TRUE(client1->connectToServer(socket_path_));

diff --git a/src/lib/asiolink/io_acceptor.h b/src/lib/asiolink/io_acceptor.h
index 4597f61d41f286f7cb1b39818428e7b13e48e8fe..c493d3427e1ff0d8fd3974606cdbb3f5422e8ab0 100644 (file)
--- a/src/lib/asiolink/io_acceptor.h
+++ b/src/lib/asiolink/io_acceptor.h
@@ -22,6 +22,11 @@ namespace asiolink {
 /// This is a wrapper class for ASIO acceptor service. Classes implementing
 /// services for specific protocol types should derive from this class.
 ///
+/// Acceptor is an IO object which accepts incoming connections into a socket
+/// object. This socket is then used for data transmission from the client
+/// to server and back. The acceptor is continued to be used to accept new
+/// connections while the accepted connection is active.
+///
 /// @tparam ProtocolType ASIO protocol type, e.g. stream_protocol
 /// @tparam CallbackType Callback function type which should have the following
 /// signature: @c void(const boost::system::error_code&).

diff --git a/src/lib/asiolink/unix_domain_socket_acceptor.h b/src/lib/asiolink/unix_domain_socket_acceptor.h
index 1cec340c67ad289bd03aba35fbd8754e145fbc6c..8aa11cadcaa33578dd75425510a91dc130e29d36 100644 (file)
--- a/src/lib/asiolink/unix_domain_socket_acceptor.h
+++ b/src/lib/asiolink/unix_domain_socket_acceptor.h
@@ -19,6 +19,9 @@ namespace isc {
 namespace asiolink {
 
 /// @brief Implements acceptor service for @ref UnixDomainSocket.
+///
+/// This class is used to accept new incoming connections over unix domain
+/// sockets.
 class UnixDomainSocketAcceptor : public IOAcceptor<boost::asio::local::stream_protocol,
                                  std::function<void(const boost::system::error_code&)> > {
 public:

diff --git a/src/lib/asiolink/unix_domain_socket_endpoint.h b/src/lib/asiolink/unix_domain_socket_endpoint.h
index 85f019cf5fb747507cf3f2683d2d3f0baa705a59..9378a8371e742a16ea8b31c57a57eab99fa5ba88 100644 (file)
--- a/src/lib/asiolink/unix_domain_socket_endpoint.h
+++ b/src/lib/asiolink/unix_domain_socket_endpoint.h
@@ -18,7 +18,9 @@ namespace asiolink {
 
 /// @brief Endpoint for @ref UnixDomainSocket.
 ///
-/// This is a simple class encapsulating ASIO unix domain socket.
+/// This is a simple class encapsulating ASIO unix domain socket endpoint.
+/// It is used to represent endpoints taking part in communication via
+/// unix domain sockets.
 class UnixDomainSocketEndpoint {
 public:
 

diff --git a/src/lib/config/command_mgr.cc b/src/lib/config/command_mgr.cc
index f04a88470fc431e08041ed865ccc57a33e1b1ef0..2be8124e94a182e3cf7f84129ad0f5c12d5f7f9d 100644 (file)
--- a/src/lib/config/command_mgr.cc
+++ b/src/lib/config/command_mgr.cc
@@ -200,32 +200,33 @@ Connection::receiveHandler(const boost::system::error_code& ec,
     // No response generated. Connection will be closed.
     if (!rsp) {
         LOG_WARN(command_logger, COMMAND_RESPONSE_ERROR);
+        rsp = createAnswer(CONTROL_RESULT_ERROR,
+                           "internal server error: no response generated");
 
-    } else {
+    }
 
-        // Let's convert JSON response to text. Note that at this stage
-        // the rsp pointer is always set.
-        std::string txt = rsp->str();
-        size_t len = txt.length();
-        if (len > 65535) {
-            // Hmm, our response is too large. Let's send the first
-            // 64KB and hope for the best.
-            LOG_ERROR(command_logger, COMMAND_SOCKET_RESPONSE_TOOLARGE).arg(len);
+    // Let's convert JSON response to text. Note that at this stage
+    // the rsp pointer is always set.
+    std::string txt = rsp->str();
+    size_t len = txt.length();
+    if (len > 65535) {
+        // Hmm, our response is too large. Let's send the first
+        // 64KB and hope for the best.
+        LOG_ERROR(command_logger, COMMAND_SOCKET_RESPONSE_TOOLARGE).arg(len);
 
-            len = 65535;
-        }
+        len = 65535;
+    }
 
-        try {
-            // Send the data back over socket.
-            socket_->write(txt.c_str(), len);
+    try {
+        // Send the data back over socket.
+        socket_->write(txt.c_str(), len);
 
-        } catch (const std::exception& ex) {
-            // Response transmission failed. Since the response failed, it doesn't
-            // make sense to send any status codes. Let's log it and be done with
-            // it.
-            LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
-                .arg(len).arg(socket_->getNative()).arg(ex.what());
-        }
+    } catch (const std::exception& ex) {
+        // Response transmission failed. Since the response failed, it doesn't
+        // make sense to send any status codes. Let's log it and be done with
+        // it.
+        LOG_ERROR(command_logger, COMMAND_SOCKET_WRITE_FAIL)
+            .arg(len).arg(socket_->getNative()).arg(ex.what());
     }
 
     connection_pool_.stop(shared_from_this());

diff --git a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc
index d84215d4e9cf5f6c1032ea82f8ec9df4ace014a1..e0d95dd2fc6b8312419a883eda4170ee4458a29f 100644 (file)
--- a/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/timer_mgr_unittest.cc
@@ -367,4 +367,11 @@ TEST_F(TimerMgrTest, callbackWithException) {
     doWait(500);
 }
 
+// This test verifies that IO service specified for the TimerMgr
+// must not be null.
+TEST_F(TimerMgrTest, setIOService) {
+    EXPECT_THROW(timer_mgr_->setIOService(IOServicePtr()),
+                 BadValue);
+}
+
 } // end of anonymous namespace

diff --git a/src/lib/dhcpsrv/timer_mgr.cc b/src/lib/dhcpsrv/timer_mgr.cc
index 659068841738db33b74ee1b818779b20a7ef30e5..aaf7c729cf6959b5529925452513ea4fb28ba1ce 100644 (file)
--- a/src/lib/dhcpsrv/timer_mgr.cc
+++ b/src/lib/dhcpsrv/timer_mgr.cc
@@ -160,6 +160,9 @@ TimerMgrImpl::TimerMgrImpl() :
 
 void
 TimerMgrImpl::setIOService(const IOServicePtr& io_service) {
+    if (!io_service) {
+        isc_throw(BadValue, "IO service object must not be null for TimerMgr");
+    }
     io_service_ = io_service;
 }
 

diff --git a/src/lib/dhcpsrv/timer_mgr.h b/src/lib/dhcpsrv/timer_mgr.h
index 63ca7bb9b4cd88ea66a414a5b13d7c80445099f2..28ba36f85d6d9c6ef523c393abb86626456198a0 100644 (file)
--- a/src/lib/dhcpsrv/timer_mgr.h
+++ b/src/lib/dhcpsrv/timer_mgr.h
@@ -24,7 +24,7 @@ class TimerMgr;
 /// @brief Type definition of the shared pointer to @c TimerMgr.
 typedef boost::shared_ptr<TimerMgr> TimerMgrPtr;
 
-/// @brief Manages a pool of asynchronous interval timers for Kea server.
+/// @brief Manages a pool of asynchronous interval timers.
 ///
 /// This class holds a pool of asynchronous interval timers.
 ///