[357] update doxygen and comments

src/bin/auth/auth_config.cc

@@ -117,6 +117,7 @@ private:
     AddrListPtr rollbackAddresses_;
 };
 
+/// \brief Configuration for TCP receive timeouts
 class TCPRecvTimeoutConfig : public AuthConfigParser {
 public:
     TCPRecvTimeoutConfig(AuthSrv& server) : server_(server)

src/bin/auth/auth_srv.h

@@ -320,6 +320,13 @@ public:
     std::vector<isc::dns::RRClass> getClientListClasses() const;
 
     /// \brief Sets the timeout for incoming TCP connections
+    ///
+    /// Incoming TCP connections that have not sent their data
+    /// withing this time are dropped.
+    ///
+    /// \param timeout The timeout (in milliseconds). If se to
+    /// zero, no timeouts are used, and the connection will remain
+    /// open forever.
     void setTCPRecvTimeout(size_t timeout);
 
 private:

src/bin/auth/tests/config_unittest.cc

@@ -143,6 +143,7 @@ TEST_F(AuthConfigTest, listenAddressConfig) {
     EXPECT_EQ(DNSService::SERVER_SYNC_OK, dnss_.getUDPFdParams().at(1).options);
 }
 
+// Try setting tcp receive timeout through config
 TEST_F(AuthConfigTest, tcpRecvTimeoutConfig) {
     configureAuthServer(server, Element::fromJSON(
     "{ \"tcp_recv_timeout\": 123 }"));

src/lib/asiodns/dns_server.h

@@ -99,6 +99,16 @@ public:
     virtual DNSServer* clone() { return (self_->clone()); }
     //@}
 
+    /// \brief Set timeout for incoming TCP connections
+    ///
+    /// Since this value is not relevant for every type of DNSServer
+    /// (like UDPServer), it has a no-op default implementation.
+    /// It is in the base class because the AuthSrv's DNSService has
+    /// no direct access to the derived API's after initialization,
+    /// and it does need to update running servers if the timeout
+    /// setting is changed.
+    ///
+    /// \param timeout The timeout in milliseconds
     virtual void setTCPRecvTimeout(size_t) {}
 
 protected:

src/lib/asiodns/dns_service.cc

@@ -40,7 +40,7 @@ public:
     DNSServiceImpl(IOService& io_service, SimpleCallback* checkin,
                    DNSLookup* lookup, DNSAnswer* answer) :
             io_service_(io_service), checkin_(checkin), lookup_(lookup),
-            answer_(answer)
+            answer_(answer), tcp_recv_timeout_(1000)
     {}
 
     IOService& io_service_;

src/lib/asiodns/dns_service.h

@@ -84,7 +84,19 @@ public:
                                     ServerFlag options = SERVER_DEFAULT) = 0;
     virtual void clearServers() = 0;
 
-    /// Sets the timeout TODO
+    /// \brief Set the timeout for TCP DNS services
+    ///
+    /// The timeout is used for incoming TCP connections, so
+    /// that the connection is dropped if not all query data
+    /// is read.
+    ///
+    /// For existing DNSServer objects, where the timeout is
+    /// relevant (i.e. TCPServer instances), the timeout value
+    /// is updated.
+    /// The given value is also kept to use for DNSServer instances
+    /// which are created later
+    ///
+    /// \param timeout The timeout in milliseconds
     virtual void setTCPRecvTimeout(size_t timeout) = 0;
 
     virtual asiolink::IOService& getIOService() = 0;

src/lib/asiodns/tcp_server.cc

@@ -47,8 +47,6 @@ namespace asiodns {
 /// The following functions implement the \c TCPServer class.
 ///
 /// The constructor
-// timeout is initialized to be sure, but it should be updated
-// quite immediately anyway
 TCPServer::TCPServer(io_service& io_service, int fd, int af,
                      const SimpleCallback* checkin,
                      const DNSLookup* lookup,
@@ -73,7 +71,7 @@ TCPServer::TCPServer(io_service& io_service, int fd, int af,
         isc_throw(IOError, exception.what());
     }
     // Set it to some value. It should be set to the right one
-    // immediately, but set something non-zero just in case.
+    // immediately, but set it to something non-zero just in case.
     tcp_recv_timeout_.reset(new size_t(1000));
 }
 
@@ -133,11 +131,14 @@ TCPServer::operator()(asio::error_code ec, size_t length) {
         /// asynchronous read call.
         data_.reset(new char[MAX_LENGTH]);
 
-        timeout_.reset(new asio::deadline_timer(io_));
-        timeout_->expires_from_now(
-            boost::posix_time::milliseconds(*tcp_recv_timeout_));
-        timeout_->async_wait(boost::bind(&do_timeout, boost::ref(*socket_),
-                             asio::placeholders::error));
+        /// Start a timer to drop the connection if it is idle
+        if (*tcp_recv_timeout_ > 0) {
+            timeout_.reset(new asio::deadline_timer(io_));
+            timeout_->expires_from_now(
+                boost::posix_time::milliseconds(*tcp_recv_timeout_));
+            timeout_->async_wait(boost::bind(&do_timeout, boost::ref(*socket_),
+                                 asio::placeholders::error));
+        }
 
         /// Read the message, in two parts.  First, the message length:
         CORO_YIELD async_read(*socket_, asio::buffer(data_.get(),

src/lib/asiodns/tests/dns_server_unittest.cc

@@ -281,10 +281,16 @@ class TCPClient : public SimpleClient {
                                 std::string(received_data_ + 2));
     }
 
+    /// Set the delay before the data len is sent (in seconds)
+    /// If this is non-zero, the actual data is never sent
+    /// (it is used to test timeout, in which case the connection
+    /// should have been closed by the other side anyway)
     void setSendDataLenDelay(size_t send_data_len_delay) {
         send_data_len_delay_ = send_data_len_delay;
     }
 
+    /// Set the delay before the packet data itself is sent
+    /// (in seconds)
     void setSendDataDelay(size_t send_data_delay) {
         send_data_delay_ = send_data_delay;
     }
@@ -328,8 +334,6 @@ class TCPClient : public SimpleClient {
     std::string data_to_send_;
     uint16_t data_to_send_len_;
 
-    // if 0, send body immediately
-    // if >0, send after the delay (in seconds)
     size_t send_data_delay_;
     size_t send_data_len_delay_;
 };