BIND 10 trac1651, updated. afc4bd6b79b9469b5717d3a1b0ac729cf6aacef7 [1651] Changes after review in dhcp4 and IfaceMgr

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jun 11 15:53:55 UTC 2012


The branch, trac1651 has been updated
       via  afc4bd6b79b9469b5717d3a1b0ac729cf6aacef7 (commit)
      from  327e505ad6ebd867ed5b7bcbf104da0d473c6944 (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 afc4bd6b79b9469b5717d3a1b0ac729cf6aacef7
Author: Tomek Mrugalski <tomasz at isc.org>
Date:   Mon Jun 11 17:53:40 2012 +0200

    [1651] Changes after review in dhcp4 and IfaceMgr

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

Summary of changes:
 src/bin/dhcp4/ctrl_dhcp4_srv.cc       |   13 ------------
 src/bin/dhcp4/ctrl_dhcp4_srv.h        |   16 ++++++++------
 src/lib/cc/tests/session_unittests.cc |    2 +-
 src/lib/dhcp/iface_mgr.cc             |   37 ++++++++++++++++-----------------
 src/lib/dhcp/iface_mgr.h              |    5 ++++-
 5 files changed, 33 insertions(+), 40 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.cc b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
index e4091e9..b8749b7 100644
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.cc
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.cc
@@ -13,32 +13,19 @@
 // PERFORMANCE OF THIS SOFTWARE.
 
 #include <config.h>
-
-//#include <sys/types.h>
-//#include <sys/socket.h>
-//#include <sys/select.h>
-//#include <netdb.h>
-//#include <netinet/in.h>
-//#include <stdlib.h>
-//#include <errno.h>
-
 #include <cassert>
 #include <iostream>
 
 #include <cc/session.h>
 #include <cc/data.h>
-
 #include <exceptions/exceptions.h>
 #include <cc/session.h>
 #include <config/ccsession.h>
-
 #include <util/buffer.h>
 #include <log/dummylog.h>
-
 #include <dhcp4/spec_config.h>
 #include <dhcp4/ctrl_dhcp4_srv.h>
 #include <dhcp/iface_mgr.h>
-
 #include <asiolink/asiolink.h>
 #include <log/logger_support.h>
 
diff --git a/src/bin/dhcp4/ctrl_dhcp4_srv.h b/src/bin/dhcp4/ctrl_dhcp4_srv.h
index 79f79cb..237335e 100644
--- a/src/bin/dhcp4/ctrl_dhcp4_srv.h
+++ b/src/bin/dhcp4/ctrl_dhcp4_srv.h
@@ -45,13 +45,16 @@ public:
     ControlledDhcpv4Srv(uint16_t port = DHCP4_SERVER_PORT,
                         bool verbose = false);
 
+    /// @brief Destructor.
+    ~ControlledDhcpv4Srv();
+
     /// @brief Establishes msgq session.
     ///
     /// Creates session that will be used to receive commands and updated
     /// configuration from boss (or indirectly from user via bindctl).
     void establishSession();
 
-    /// @brief Terminates existing session.
+    /// @brief Terminates existing msgq session.
     ///
     /// This method terminates existing session with msgq. After calling
     /// it, not further messages over msgq (commands or configuration updates)
@@ -63,8 +66,6 @@ public:
     /// @brief Initiates shutdown procedure for the whole DHCPv4 server.
     void shutdown();
 
-    ~ControlledDhcpv4Srv();
-
     /// @brief Session callback, processes received commands.
     ///
     /// @param command_id text represenation of the command (e.g. "shutdown")
@@ -84,6 +85,9 @@ protected:
 
     /// @brief A callback for handling incoming configuration updates.
     ///
+    /// As pointer to this method is used a callback in ASIO used in
+    /// ModuleCCSession, it has to be static.
+    ///
     /// @param new_config textual representation of the new configuration
     ///
     /// @return status of the config update
@@ -99,16 +103,16 @@ protected:
     static isc::data::ConstElementPtr
     dhcp4CommandHandler(const std::string& command, isc::data::ConstElementPtr args);
 
-    /// @brief callback that will be called from iface_mgr when command/config arrives
+    /// @brief Callback that will be called from iface_mgr when command/config arrives.
     ///
     /// This static callback method is called from IfaceMgr::receive4() method,
     /// when there is a new command or configuration sent over msgq.
     static void sessionReader(void);
 
-    /// @brief IOService object, used for all ASIO operations
+    /// @brief IOService object, used for all ASIO operations.
     isc::asiolink::IOService io_service_;
 
-    /// @brief Helper session object that represents raw connection to msgq
+    /// @brief Helper session object that represents raw connection to msgq.
     isc::cc::Session* cc_session_;
 
     /// @brief Session that receives configuation and commands
diff --git a/src/lib/cc/tests/session_unittests.cc b/src/lib/cc/tests/session_unittests.cc
index b0cf252..528dda9 100644
--- a/src/lib/cc/tests/session_unittests.cc
+++ b/src/lib/cc/tests/session_unittests.cc
@@ -247,5 +247,5 @@ TEST_F(SessionTest, get_socket_descr) {
     EXPECT_NO_THROW(socket = sess.getSocketDesc());
 
     // expect actual socket handle to be returned, not 0
-    EXPECT_TRUE(0 < socket);
+    EXPECT_LT(0, socket);
 }
diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 6f47889..447b162 100644
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -123,7 +123,7 @@ bool IfaceMgr::Iface::delSocket(uint16_t sockfd) {
 IfaceMgr::IfaceMgr()
     :control_buf_len_(CMSG_SPACE(sizeof(struct in6_pktinfo))),
      control_buf_(new char[control_buf_len_]),
-     session_socket_(0), session_callback_(NULL)
+     session_socket_(InvalidSocket), session_callback_(NULL)
 {
 
     cout << "IfaceMgr initialization." << endl;
@@ -685,7 +685,7 @@ IfaceMgr::send(const Pkt4Ptr& pkt)
 
 
 boost::shared_ptr<Pkt4>
-IfaceMgr::receive4(unsigned int timeout) {
+IfaceMgr::receive4(uint32_t timeout) {
 
     const SocketInfo* candidate = 0;
     IfaceCollection::const_iterator iface;
@@ -696,27 +696,29 @@ IfaceMgr::receive4(unsigned int timeout) {
 
     stringstream names;
 
+    /// @todo: marginal performance optimization. We could create the set once
+    /// and then use its copy for select(). Please note that select() modifies
+    /// provided set to indicated which sockets have something to read.
     for (iface = ifaces_.begin(); iface != ifaces_.end(); ++iface) {
 
         for (SocketCollection::const_iterator s = iface->sockets_.begin();
              s != iface->sockets_.end(); ++s) {
 
-            // We don't want IPv6 addresses here.
-            if (s->addr_.getFamily() != AF_INET) {
-                continue;
-            }
-
-            names << s->sockfd_ << "(" << iface->getName() << ") ";
+            // Only deal with IPv4 addresses.
+            if (s->addr_.getFamily() == AF_INET) {
+                names << s->sockfd_ << "(" << iface->getName() << ") ";
 
-            // add this socket to listening set
-            FD_SET(s->sockfd_, &sockets);
-            if (maxfd < s->sockfd_)
-                maxfd = s->sockfd_;
+                // Add this socket to listening set
+                FD_SET(s->sockfd_, &sockets);
+                if (maxfd < s->sockfd_) {
+                    maxfd = s->sockfd_;
+                }
+            }
         }
     }
 
     // if there is session socket registered...
-    if (session_socket_) {
+    if (session_socket_ != InvalidSocket) {
         // at it to the set as well
         FD_SET(session_socket_, &sockets);
         if (maxfd < session_socket_)
@@ -737,11 +739,8 @@ IfaceMgr::receive4(unsigned int timeout) {
     if (result == 0) {
         // nothing received and timeout has been reached
         return (Pkt4Ptr()); // NULL
-    }
-    if (result < 0) {
-        char buf[512];
-        strncpy(buf, strerror(errno), 512);
-        cout << "Socket read error: " << buf << endl;
+    } else if (result < 0) {
+        cout << "Socket read error: " << strerror(errno) << endl;
 
         /// @todo: perhaps throw here?
         return (Pkt4Ptr()); // NULL
@@ -749,7 +748,7 @@ IfaceMgr::receive4(unsigned int timeout) {
 
     // Let's find out which socket has the data
 
-    if (session_socket_ && (FD_ISSET(session_socket_, &sockets))) {
+    if ((session_socket_ != InvalidSocket) && (FD_ISSET(session_socket_, &sockets))) {
         // something received over session socket
         cout << "BIND10 command or config available over session socket." << endl;
 
diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index aacc443..1b2c487 100644
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -357,7 +357,7 @@ public:
     /// @param timeout specifies timeout (in seconds)
     ///
     /// @return Pkt4 object representing received packet (or NULL)
-    Pkt4Ptr receive4(unsigned int timeout);
+    Pkt4Ptr receive4(uint32_t timeout);
 
     /// Opens UDP/IP socket and binds it to address, interface and port.
     ///
@@ -414,6 +414,9 @@ public:
         session_callback_ = callback;
     }
 
+    /// A value of socket descriptor representing "not specified" state.
+    static const int InvalidSocket = -1;
+
     // don't use private, we need derived classes in tests
 protected:
 



More information about the bind10-changes mailing list