[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.

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()),

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:

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);
+}
+
 }