BIND 10 trac805, updated. 7319f208048b41a6279c36e5ae5d4b0e5c86751a [805] Convert asio based exceptions to IOError

BIND 10 source code commits bind10-changes at lists.isc.org
Tue Jan 3 14:00:06 UTC 2012


The branch, trac805 has been updated
       via  7319f208048b41a6279c36e5ae5d4b0e5c86751a (commit)
       via  849cf2de22c31a2ef5c446a30f9ad5bac23d8c42 (commit)
       via  e031e1adcfa042da3cb6baa9834e42f573ea66fb (commit)
      from  bb00c59b03275b564938cbbb32956639bf76e5e8 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 7319f208048b41a6279c36e5ae5d4b0e5c86751a
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Jan 3 14:59:46 2012 +0100

    [805] Convert asio based exceptions to IOError

commit 849cf2de22c31a2ef5c446a30f9ad5bac23d8c42
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Jan 3 14:12:38 2012 +0100

    [805] Log new servers

commit e031e1adcfa042da3cb6baa9834e42f573ea66fb
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date:   Tue Jan 3 13:37:31 2012 +0100

    [805] Details based on review
    
    * Use the address family instead of v6 boolean.
    * Check it by exception.
    * Documentation.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/asiodns/Makefile.am                        |    1 +
 src/lib/asiodns/asiodns_messages.mes               |    8 ++++
 src/lib/asiodns/dns_service.cc                     |   12 +++---
 src/lib/asiodns/dns_service.h                      |   30 ++++++++++++--
 src/lib/asiodns/io_fetch.cc                        |    8 +---
 src/lib/{server_common => asiodns}/logger.cc       |    8 ++-
 .../{server_common/logger.cc => asiodns/logger.h}  |    9 +++-
 src/lib/asiodns/tcp_server.cc                      |   23 ++++++++--
 src/lib/asiodns/tcp_server.h                       |    7 ++-
 src/lib/asiodns/tests/dns_server_unittest.cc       |   44 ++++++++++++++++---
 src/lib/asiodns/udp_server.cc                      |   31 ++++++++++----
 src/lib/asiodns/udp_server.h                       |    7 ++-
 src/lib/server_common/portconfig.cc                |    6 +-
 13 files changed, 143 insertions(+), 51 deletions(-)
 copy src/lib/{server_common => asiodns}/logger.cc (88%)
 copy src/lib/{server_common/logger.cc => asiodns/logger.h} (82%)

-----------------------------------------------------------------------
diff --git a/src/lib/asiodns/Makefile.am b/src/lib/asiodns/Makefile.am
index 2d246ef..b5d030d 100644
--- a/src/lib/asiodns/Makefile.am
+++ b/src/lib/asiodns/Makefile.am
@@ -25,6 +25,7 @@ libasiodns_la_SOURCES += dns_service.cc dns_service.h
 libasiodns_la_SOURCES += tcp_server.cc tcp_server.h
 libasiodns_la_SOURCES += udp_server.cc udp_server.h
 libasiodns_la_SOURCES += io_fetch.cc io_fetch.h
+libasiodns_la_SOURCES += logger.h logger.cc
 
 nodist_libasiodns_la_SOURCES = asiodns_messages.cc asiodns_messages.h
 
diff --git a/src/lib/asiodns/asiodns_messages.mes b/src/lib/asiodns/asiodns_messages.mes
index feb75d4..8fbafdd 100644
--- a/src/lib/asiodns/asiodns_messages.mes
+++ b/src/lib/asiodns/asiodns_messages.mes
@@ -14,6 +14,14 @@
 
 $NAMESPACE isc::asiodns
 
+% ASIODNS_FD_ADD_TCP adding a new TCP server by opened fd %1
+A debug message informing about installing a file descriptor as a server.
+The file descriptor number is noted.
+
+% ASIODNS_FD_ADD_UDP adding a new UDP server by opened fd %1
+A debug message informing about installing a file descriptor as a server.
+The file descriptor number is noted.
+
 % ASIODNS_FETCH_COMPLETED upstream fetch to %1(%2) has now completed
 A debug message, this records that the upstream fetch (a query made by the
 resolver on behalf of its client) to the specified address has completed.
diff --git a/src/lib/asiodns/dns_service.cc b/src/lib/asiodns/dns_service.cc
index 964b77e..c5a460c 100644
--- a/src/lib/asiodns/dns_service.cc
+++ b/src/lib/asiodns/dns_service.cc
@@ -78,8 +78,8 @@ public:
     DNSLookup *lookup_;
     DNSAnswer *answer_;
 
-    template<class Ptr, class Server> void addServerFromFD(int fd, bool v6) {
-        Ptr server(new Server(io_service_.get_io_service(), fd, v6, checkin_,
+    template<class Ptr, class Server> void addServerFromFD(int fd, int af) {
+        Ptr server(new Server(io_service_.get_io_service(), fd, af, checkin_,
                               lookup_, answer_));
         (*server)();
         servers_.push_back(server);
@@ -196,12 +196,12 @@ DNSService::addServer(uint16_t port, const std::string& address) {
     impl_->addServer(port, convertAddr(address));
 }
 
-void DNSService::addServerTCP(int fd, bool v6) {
-    impl_->addServerFromFD<DNSServiceImpl::TCPServerPtr, TCPServer>(fd, v6);
+void DNSService::addServerTCPFromFD(int fd, int af) {
+    impl_->addServerFromFD<DNSServiceImpl::TCPServerPtr, TCPServer>(fd, af);
 }
 
-void DNSService::addServerUDP(int fd, bool v6) {
-    impl_->addServerFromFD<DNSServiceImpl::UDPServerPtr, UDPServer>(fd, v6);
+void DNSService::addServerUDPFromFD(int fd, int af) {
+    impl_->addServerFromFD<DNSServiceImpl::UDPServerPtr, UDPServer>(fd, af);
 }
 
 void
diff --git a/src/lib/asiodns/dns_service.h b/src/lib/asiodns/dns_service.h
index 75b688e..b77b7f7 100644
--- a/src/lib/asiodns/dns_service.h
+++ b/src/lib/asiodns/dns_service.h
@@ -88,10 +88,32 @@ public:
     /// \brief Add another server to the service
     void addServer(uint16_t port, const std::string &address);
     void addServer(const char &port, const std::string &address);
-    /// \brief Add another server to the service from already opened file
-    /// descriptor
-    void addServerTCP(int fd, bool v6);
-    void addServerUDP(int fd, bool v6);
+    /// \brief Add another TCP server/listener to the service from already opened
+    ///    file descriptor
+    ///
+    /// Adds a new TCP server using an already opened file descriptor (eg. it
+    /// only wraps it so the file descriptor is usable within the event loop).
+    ///
+    /// \param fd the file descriptor to be used.
+    /// \param af the address family of the file descriptor. Must be either
+    ///     AF_INET or AF_INET6.
+    /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6.
+    /// \throw isc::asiolink::IOError when a low-level error happens, like the
+    ///     fd is not a valid descriptor or it can't be listened on.
+    void addServerTCPFromFD(int fd, int af);
+    /// \brief Add another UDP server to the service from already opened
+    ///    file descriptor
+    ///
+    /// Adds a new UDP server using an already opened file descriptor (eg. it
+    /// only wraps it so the file descriptor is usable within the event loop).
+    ///
+    /// \param fd the file descriptor to be used.
+    /// \param af the address family of the file descriptor. Must be either
+    ///     AF_INET or AF_INET6.
+    /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6.
+    /// \throw isc::asiolink::IOError when a low-level error happens, like the
+    ///     fd is not a valid descriptor or it can't be listened on.
+    void addServerUDPFromFD(int fd, int af);
     /// \brief Remove all servers from the service
     void clearServers();
 
diff --git a/src/lib/asiodns/io_fetch.cc b/src/lib/asiodns/io_fetch.cc
index 466be3e..6b40acd 100644
--- a/src/lib/asiodns/io_fetch.cc
+++ b/src/lib/asiodns/io_fetch.cc
@@ -38,15 +38,13 @@
 #include <dns/messagerenderer.h>
 #include <dns/opcode.h>
 #include <dns/rcode.h>
-#include <log/logger.h>
-#include <log/macros.h>
 
-#include <asiodns/asiodns_messages.h>
 #include <asiodns/io_fetch.h>
 
 #include <util/buffer.h>
 #include <util/random/qid_gen.h>
 
+#include <asiodns/logger.h>
 
 using namespace asio;
 using namespace isc::asiolink;
@@ -59,10 +57,6 @@ using namespace std;
 namespace isc {
 namespace asiodns {
 
-/// Use the ASIO logger
-
-isc::log::Logger logger("asiolink");
-
 // Log debug verbosity
 
 const int DBG_IMPORTANT = DBGLVL_TRACE_BASIC;
diff --git a/src/lib/asiodns/logger.cc b/src/lib/asiodns/logger.cc
new file mode 100644
index 0000000..e4c60e2
--- /dev/null
+++ b/src/lib/asiodns/logger.cc
@@ -0,0 +1,25 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <asiodns/logger.h>
+
+namespace isc {
+namespace asiodns {
+
+/// Use the ASIO logger
+
+isc::log::Logger logger("asiodns");
+
+}
+}
diff --git a/src/lib/asiodns/logger.h b/src/lib/asiodns/logger.h
new file mode 100644
index 0000000..d690edf
--- /dev/null
+++ b/src/lib/asiodns/logger.h
@@ -0,0 +1,26 @@
+// Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <log/logger.h>
+#include <log/macros.h>
+#include <log/log_dbglevels.h>
+#include <asiodns/asiodns_messages.h>
+
+namespace isc {
+namespace asiodns {
+
+extern isc::log::Logger logger;
+
+}
+}
diff --git a/src/lib/asiodns/tcp_server.cc b/src/lib/asiodns/tcp_server.cc
index 05cf140..f7e9138 100644
--- a/src/lib/asiodns/tcp_server.cc
+++ b/src/lib/asiodns/tcp_server.cc
@@ -29,8 +29,8 @@
 #include <asiolink/dummy_io_cb.h>
 #include <asiolink/tcp_endpoint.h>
 #include <asiolink/tcp_socket.h>
-#include <tcp_server.h>
-
+#include <asiodns/tcp_server.h>
+#include <asiodns/logger.h>
 
 using namespace asio;
 using asio::ip::udp;
@@ -69,7 +69,7 @@ TCPServer::TCPServer(io_service& io_service,
     acceptor_->listen();
 }
 
-TCPServer::TCPServer(io_service& io_service, int fd, bool v6,
+TCPServer::TCPServer(io_service& io_service, int fd, int af,
                      const SimpleCallback* checkin,
                      const DNSLookup* lookup,
                      const DNSAnswer* answer) :
@@ -77,9 +77,22 @@ TCPServer::TCPServer(io_service& io_service, int fd, bool v6,
     checkin_callback_(checkin), lookup_callback_(lookup),
     answer_callback_(answer)
 {
+    if (af != AF_INET && af != AF_INET6) {
+        isc_throw(InvalidParameter, "Address family must be either AF_INET "
+                  "or AF_INET6, not " << af);
+    }
+    logger.debug(DBGLVL_TRACE_BASIC, ASIODNS_FD_ADD_TCP).arg(fd);
+
     acceptor_.reset(new tcp::acceptor(io_service));
-    acceptor_->assign(v6 ? tcp::v6() : tcp::v4(), fd);
-    acceptor_->listen();
+    try {
+        acceptor_->assign(af == AF_INET6 ? tcp::v6() : tcp::v4(), fd);
+        acceptor_->listen();
+    }
+    // Whatever the thing throws, it is something from ASIO and we convert
+    // it
+    catch (const std::exception& exception) {
+        isc_throw(IOError, exception.what());
+    }
 }
 
 void
diff --git a/src/lib/asiodns/tcp_server.h b/src/lib/asiodns/tcp_server.h
index c7fac87..d079e97 100644
--- a/src/lib/asiodns/tcp_server.h
+++ b/src/lib/asiodns/tcp_server.h
@@ -46,11 +46,14 @@ public:
     /// \brief Constructor
     /// \param io_service the asio::io_service to work with
     /// \param fd the file descriptor of opened TCP socket
-    /// \param v6 the socket in fd is ipv6 one (if false, it is ipv4)
+    /// \param af address family of the socket, either AF_INET or AF_INET6
     /// \param checkin the callbackprovider for non-DNS events
     /// \param lookup the callbackprovider for DNS lookup events
     /// \param answer the callbackprovider for DNS answer events
-    TCPServer(asio::io_service& io_service, int fd, bool v6,
+    /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6
+    /// \throw isc::asiolink::IOError when a low-level error happens, like the
+    ///     fd is not a valid descriptor or it can't be listened on.
+    TCPServer(asio::io_service& io_service, int fd, int af,
               const isc::asiolink::SimpleCallback* checkin = NULL,
               const DNSLookup* lookup = NULL, const DNSAnswer* answer = NULL);
 
diff --git a/src/lib/asiodns/tests/dns_server_unittest.cc b/src/lib/asiodns/tests/dns_server_unittest.cc
index e67f4a2..5bbfe45 100644
--- a/src/lib/asiodns/tests/dns_server_unittest.cc
+++ b/src/lib/asiodns/tests/dns_server_unittest.cc
@@ -320,9 +320,17 @@ class TCPClient : public SimpleClient {
 // the same.
 class DNSServerTestBase : public::testing::Test {
     protected:
+        DNSServerTestBase() :
+            udp_server_(NULL),
+            tcp_server_(NULL)
+        { }
         void TearDown() {
-            udp_server_->stop();
-            tcp_server_->stop();
+            if (udp_server_) {
+                udp_server_->stop();
+            }
+            if (tcp_server_) {
+                tcp_server_->stop();
+            }
             delete checker_;
             delete lookup_;
             delete answer_;
@@ -360,7 +368,7 @@ class DNSServerTestBase : public::testing::Test {
             answer_ = new SimpleAnswer();
             udp_client_ = new UDPClient(service,
                                         ip::udp::endpoint(server_address_,
-                                                          server_port));
+                                                         server_port));
             tcp_client_ = new TCPClient(service,
                                         ip::tcp::endpoint(server_address_,
                                                           server_port));
@@ -452,12 +460,12 @@ protected:
         commonSetup();
         int fdUDP(getFd(SOCK_DGRAM));
         ASSERT_NE(-1, fdUDP) << strerror(errno);
-        udp_server_ = new UDPServer(service, fdUDP, false, checker_, lookup_,
-                                    answer_);
+        udp_server_ = new UDPServer(service, fdUDP, AF_INET6, checker_,
+                                    lookup_, answer_);
         int fdTCP(getFd(SOCK_STREAM));
         ASSERT_NE(-1, fdTCP) << strerror(errno);
-        tcp_server_ = new TCPServer(service, fdTCP, false, checker_, lookup_,
-                                    answer_);
+        tcp_server_ = new TCPServer(service, fdTCP, AF_INET6, checker_,
+                                    lookup_, answer_);
     }
 };
 
@@ -594,4 +602,26 @@ TYPED_TEST(DNSServerTest, stopTCPServeMoreThanOnce) {
     EXPECT_TRUE(this->serverStopSucceed());
 }
 
+// It raises an exception when invalid address family is passed
+TEST_F(DNSServerTestBase, invalidFamily) {
+    // We abuse DNSServerTestBase for this test, as we don't need the
+    // initialization.
+    commonSetup();
+    EXPECT_THROW(UDPServer(service, 0, AF_UNIX, checker_, lookup_,
+                           answer_), isc::InvalidParameter);
+    EXPECT_THROW(TCPServer(service, 0, AF_UNIX, checker_, lookup_,
+                           answer_), isc::InvalidParameter);
+}
+
+// It raises an exception when invalid address family is passed
+TEST_F(DNSServerTestBase, invalidFD) {
+    // We abuse DNSServerTestBase for this test, as we don't need the
+    // initialization.
+    commonSetup();
+    EXPECT_THROW(UDPServer(service, -1, AF_INET, checker_, lookup_,
+                           answer_), isc::asiolink::IOError);
+    EXPECT_THROW(TCPServer(service, -1, AF_INET, checker_, lookup_,
+                           answer_), isc::asiolink::IOError);
+}
+
 }
diff --git a/src/lib/asiodns/udp_server.cc b/src/lib/asiodns/udp_server.cc
index 176853d..9f6258f 100644
--- a/src/lib/asiodns/udp_server.cc
+++ b/src/lib/asiodns/udp_server.cc
@@ -29,6 +29,7 @@
 #include <asiolink/udp_endpoint.h>
 #include <asiolink/udp_socket.h>
 #include "udp_server.h"
+#include "logger.h"
 
 #include <dns/opcode.h>
 
@@ -74,17 +75,29 @@ struct UDPServer::Data {
         }
         socket_->bind(udp::endpoint(addr, port));
     }
-    Data(io_service& io_service, int fd, bool v6,
-        SimpleCallback* checkin, DNSLookup* lookup, DNSAnswer* answer) :
-        io_(io_service), done_(false),
-        checkin_callback_(checkin),lookup_callback_(lookup),
-        answer_callback_(answer)
+    Data(io_service& io_service, int fd, int af, SimpleCallback* checkin,
+         DNSLookup* lookup, DNSAnswer* answer) :
+         io_(io_service), done_(false),
+         checkin_callback_(checkin),lookup_callback_(lookup),
+         answer_callback_(answer)
     {
+        if (af != AF_INET && af != AF_INET6) {
+            isc_throw(InvalidParameter, "Address family must be either AF_INET "
+                      "or AF_INET6, not " << af);
+        }
+        logger.debug(DBGLVL_TRACE_BASIC, ASIODNS_FD_ADD_UDP).arg(fd);
         // We must use different instantiations for v4 and v6;
         // otherwise ASIO will bind to both
-        udp proto = v6 ? udp::v6() : udp::v4();
+        udp proto = af == AF_INET6 ? udp::v6() : udp::v4();
         socket_.reset(new udp::socket(io_service));
-        socket_->assign(proto, fd);
+        try {
+            socket_->assign(proto, fd);
+        }
+        // Whatever the thing throws, it is something from ASIO and we convert
+        // it
+        catch (const std::exception& exception) {
+            isc_throw(IOError, exception.what());
+        }
     }
 
     /*
@@ -179,10 +192,10 @@ UDPServer::UDPServer(io_service& io_service, const ip::address& addr,
     data_(new Data(io_service, addr, port, checkin, lookup, answer))
 { }
 
-UDPServer::UDPServer(io_service& io_service, int fd, bool v6,
+UDPServer::UDPServer(io_service& io_service, int fd, int af,
                      SimpleCallback* checkin, DNSLookup* lookup,
                      DNSAnswer* answer) :
-    data_(new Data(io_service, fd, v6, checkin, lookup, answer))
+    data_(new Data(io_service, fd, af, checkin, lookup, answer))
 { }
 
 /// The function operator is implemented with the "stackless coroutine"
diff --git a/src/lib/asiodns/udp_server.h b/src/lib/asiodns/udp_server.h
index f5a9b85..1c07d34 100644
--- a/src/lib/asiodns/udp_server.h
+++ b/src/lib/asiodns/udp_server.h
@@ -55,11 +55,14 @@ public:
     /// \brief Constructor
     /// \param io_service the asio::io_service to work with
     /// \param fd the file descriptor of opened UDP socket
-    /// \param v6 the socket in fd is ipv6 one (if false, it is ipv4)
+    /// \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 answer the callbackprovider for DNS answer events
-    UDPServer(asio::io_service& io_service, int fd, bool v6,
+    /// \throw isc::InvalidParameter if af is neither AF_INET nor AF_INET6
+    /// \throw isc::asiolink::IOError when a low-level error happens, like the
+    ///     fd is not a valid descriptor.
+    UDPServer(asio::io_service& io_service, int fd, int af,
               isc::asiolink::SimpleCallback* checkin = NULL,
               DNSLookup* lookup = NULL, DNSAnswer* answer = NULL);
 
diff --git a/src/lib/server_common/portconfig.cc b/src/lib/server_common/portconfig.cc
index 929ef71..589db60 100644
--- a/src/lib/server_common/portconfig.cc
+++ b/src/lib/server_common/portconfig.cc
@@ -92,7 +92,7 @@ setAddresses(DNSService& service, const AddressList& addresses) {
     }
     current_sockets.clear();
     BOOST_FOREACH(const AddressPair &address, addresses) {
-        bool is_v6(IOAddress(address.first).getFamily() == AF_INET6);
+        int af(IOAddress(address.first).getFamily());
         // TODO: Support sharing somehow in future.
         const SocketRequestor::SocketID
             tcp(socketRequestor().requestSocket(SocketRequestor::TCP,
@@ -101,7 +101,7 @@ setAddresses(DNSService& service, const AddressList& addresses) {
                                                 "dummy_app"));
         current_sockets.push_back(tcp.second);
         if (!test_mode) {
-            service.addServerTCP(tcp.first, is_v6);
+            service.addServerTCPFromFD(tcp.first, af);
         }
         const SocketRequestor::SocketID
             udp(socketRequestor().requestSocket(SocketRequestor::UDP,
@@ -110,7 +110,7 @@ setAddresses(DNSService& service, const AddressList& addresses) {
                                                 "dummy_app"));
         current_sockets.push_back(udp.second);
         if (!test_mode) {
-            service.addServerUDP(udp.first, is_v6);
+            service.addServerUDPFromFD(udp.first, af);
         }
     }
 }




More information about the bind10-changes mailing list