[2903] more (unrelated) cleanups for SyncUDPServer, removing dead code.

also, it now rejects NULL lookup callback explicitly as it doesn't make
sense for this class.  It will also help make the receive handler code
simpler.

diff --git a/src/lib/asiodns/sync_udp_server.cc b/src/lib/asiodns/sync_udp_server.cc
index b90bbea7a097b86cc10b4e255e84868d784fb2a5..a3d9ce05032bb90bccca1f5032a075269bda1c80 100644 (file)
--- a/src/lib/asiodns/sync_udp_server.cc
+++ b/src/lib/asiodns/sync_udp_server.cc
@@ -51,6 +51,10 @@ SyncUDPServer::SyncUDPServer(asio::io_service& io_service, const int fd,
         isc_throw(InvalidParameter, "Address family must be either AF_INET "
                   "or AF_INET6, not " << af);
     }
+    if (!lookup) {
+        isc_throw(InvalidParameter, "null lookup callback given to "
+                  "SyncUDPServer");
+    }
     LOG_DEBUG(logger, DBGLVL_TRACE_BASIC, ASIODNS_FD_ADD_UDP).arg(fd);
     try {
         socket_.reset(new asio::ip::udp::socket(io_service));
@@ -110,13 +114,6 @@ SyncUDPServer::handleRead(const asio::error_code& ec, const size_t length) {
         }
     }
 
-    // If we don't have a DNS Lookup provider, there's no point in
-    // continuing; we exit the coroutine permanently.
-    if (lookup_callback_ == NULL) {
-        scheduleRead();
-        return;
-    }
-
     // Make sure the buffers are fresh.  Note that we don't touch query_
     // because it's supposed to be cleared in lookup_callback_.  We should
     // eventually even remove this member variable (and remove it from
@@ -138,19 +135,11 @@ SyncUDPServer::handleRead(const asio::error_code& ec, const size_t length) {
                   "No resume called from the lookup callback");
     }
 
-    if (stopped_) {
-        return;
-    }
-
     if (done_) {
         // Good, there's an answer.
         // Call the answer callback to render it.
         (*answer_callback_)(message, query_, answer_, output_buffer_);
 
-        if (stopped_) {
-            return;
-        }
-
         asio::error_code ec;
         socket_->send_to(asio::const_buffers_1(output_buffer_->getData(),
                                                output_buffer_->getLength()),

diff --git a/src/lib/asiodns/sync_udp_server.h b/src/lib/asiodns/sync_udp_server.h
index 14ec42a66b22150455c07d5a1efa8f191e2796b0..0ab4bbc501dcffee010345fe4b6d046573114f1a 100644 (file)
--- a/src/lib/asiodns/sync_udp_server.h
+++ b/src/lib/asiodns/sync_udp_server.h
@@ -43,13 +43,21 @@ namespace asiodns {
 class SyncUDPServer : public DNSServer, public boost::noncopyable {
 public:
     /// \brief Constructor
+    ///
+    /// Due to the nature of this server, it's meaningless if the lookup
+    /// callback is NULL.  So the constructor explicitly rejects that case
+    /// with an exception.
+    ///
     /// \param io_service the asio::io_service to work with
     /// \param fd the file descriptor of opened UDP socket
     /// \param af address family, either AF_INET or AF_INET6
     /// \param checkin the callbackprovider for non-DNS events
-    /// \param lookup the callbackprovider for DNS lookup events
+    /// \param lookup the callbackprovider for DNS lookup events (must not be
+    ///        NULL)
     /// \param answer the callbackprovider for DNS answer events
+    ///
     /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6
+    /// \throw isc::InvalidParameter lookup is NULL
     /// \throw isc::asiolink::IOError when a low-level error happens, like the
     ///     fd is not a valid descriptor.
     SyncUDPServer(asio::io_service& io_service, const int fd, const int af,
@@ -144,3 +152,7 @@ private:
 } // namespace asiodns
 } // namespace isc
 #endif // SYNC_UDP_SERVER_H
+
+// Local Variables:
+// mode: c++
+// End:

diff --git a/src/lib/asiodns/tests/dns_server_unittest.cc b/src/lib/asiodns/tests/dns_server_unittest.cc
index 51fb6b8680d89784a37182ba51da814c53504df9..f4a7403d2cd84242235c0aceaf5e884b330675b3 100644 (file)
--- a/src/lib/asiodns/tests/dns_server_unittest.cc
+++ b/src/lib/asiodns/tests/dns_server_unittest.cc
@@ -723,4 +723,10 @@ TEST_F(SyncServerTest, mustResume) {
                  isc::Unexpected);
 }
 
+// SyncUDPServer doesn't allow NULL lookup callback.
+TEST_F(SyncServerTest, nullLookupCallback) {
+    EXPECT_THROW(SyncUDPServer(service, 0, AF_INET, checker_, NULL, answer_),
+                 isc::InvalidParameter);
+}
+
 }