[805] Details based on review

* Use the address family instead of v6 boolean.
* Check it by exception.
* Documentation.

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

src/lib/asiodns/dns_service.h

@@ -88,10 +88,34 @@ 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).
+    ///
+    /// If you pass bad data (eg. opened UDP socket as fd), this will not detect
+    /// it right away, but successfull operations might fail.
+    ///
+    /// \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.
+    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).
+    ///
+    /// If you pass bad data (eg. opened TCP socket as fd), this will not detect
+    /// it right away, but successfull operations might fail.
+    ///
+    /// \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.
+    void addServerUDPFromFD(int fd, int af);
     /// \brief Remove all servers from the service
     void clearServers();
 

src/lib/asiodns/tcp_server.cc

@@ -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,8 +77,12 @@ 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);
+    }
     acceptor_.reset(new tcp::acceptor(io_service));
-    acceptor_->assign(v6 ? tcp::v6() : tcp::v4(), fd);
+    acceptor_->assign(af == AF_INET6 ? tcp::v6() : tcp::v4(), fd);
     acceptor_->listen();
 }
 

src/lib/asiodns/tcp_server.h

@@ -46,11 +46,12 @@ 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
+    TCPServer(asio::io_service& io_service, int fd, int af,
               const isc::asiolink::SimpleCallback* checkin = NULL,
               const DNSLookup* lookup = NULL, const DNSAnswer* answer = NULL);
 

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,16 @@ TYPED_TEST(DNSServerTest, stopTCPServeMoreThanOnce) {
     EXPECT_TRUE(this->serverStopSucceed());
 }
 
+// It raises an exception when invalid address family is passed
+TEST_F(DNSServerTestBase, exceptions) {
+    // We abuse DNSServerTestBase for this test, as we don't need the
+    // initialization.
+    commonSetup();
+    // We use the 0 fd as it should not be touched anyway
+    EXPECT_THROW(UDPServer(service, 0, AF_UNIX, checker_, lookup_,
+                           answer_), isc::InvalidParameter);
+    EXPECT_THROW(TCPServer(service, 0, AF_UNIX, checker_, lookup_,
+                           answer_), isc::InvalidParameter);
+}
+
 }

src/lib/asiodns/udp_server.cc

@@ -74,15 +74,19 @@ 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);
+        }
         // 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);
     }
@@ -179,10 +183,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"

src/lib/asiodns/udp_server.h

@@ -55,11 +55,12 @@ 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
+    UDPServer(asio::io_service& io_service, int fd, int af,
               isc::asiolink::SimpleCallback* checkin = NULL,
               DNSLookup* lookup = NULL, DNSAnswer* answer = NULL);
 

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