[1957] Mostly code cleanup and comments updated.

src/lib/dhcp/iface_mgr.cc

@@ -401,6 +401,7 @@ int IfaceMgr::openSocketFromIface(const std::string& ifname,
                                   const uint16_t port,
                                   const uint8_t family) {
     int sock = 0;
+    // Search for specified interface among detected interfaces.
     for (IfaceCollection::iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
@@ -410,19 +411,21 @@ int IfaceMgr::openSocketFromIface(const std::string& ifname,
             continue;
         }
 
+        // Interface is now detected. Search for address on interface
+        // that matches address family (v6 or v4).
         AddressCollection addrs = iface->getAddresses();
+        for (AddressCollection::iterator addr_it = addrs.begin();
+             addr_it != addrs.end();
+             ++addr_it) {
 
-        for (AddressCollection::iterator addr= addrs.begin();
-             addr != addrs.end();
-             ++addr) {
-
-            if (addr->getFamily() != family) {
+            if (addr_it->getFamily() != family) {
                 continue;
             }
 
-            sock = openSocket(iface->getName(), *addr, port);
-            if (sock<0) {
-                cout << "Failed to open unicast socket." << endl;
+            // We have interface and address so let's open socket.
+            sock = openSocket(iface->getName(), *addr_it, port);
+            if (sock < 0) {
+                isc_throw(Unexpected, "Failed to open socket.");
             }
             return (sock);
         }
@@ -433,6 +436,8 @@ int IfaceMgr::openSocketFromIface(const std::string& ifname,
 int IfaceMgr::openSocketFromAddress(const IOAddress& addr,
                                     const uint16_t port) {
     int sock = 0;
+    // Search through detected interfaces and addresses to match
+    // local address we got.
     for (IfaceCollection::iterator iface=ifaces_.begin();
          iface!=ifaces_.end();
          ++iface) {
@@ -443,19 +448,18 @@ int IfaceMgr::openSocketFromAddress(const IOAddress& addr,
              addr_it != addrs.end();
              ++addr_it) {
 
-            try {
-                if (addr_it->getAddress() != addr.getAddress()) {
-                    continue;
-                }
-            } catch (const isc::asiolink::IOError& e) {
-                cout << "Failed to open socket from address: "
-                     << e.what() << endl;
-                return (sock);
+            // Local address must match one of the addresses
+            // on detected interfaces. If it does, we have
+            // address and interface detected so we can open
+            // socket.
+            if (*addr_it != addr) {
+                continue;
             }
 
+            // Open socket using local interface, address and port.
             sock = openSocket(iface->getName(), *addr_it, port);
             if (sock < 0) {
-                cout << "Failed to open unicast socket." << endl;
+                isc_throw(Unexpected, "Failed to open unicast socket.");
             }
             return (sock);
         }
@@ -465,49 +469,38 @@ int IfaceMgr::openSocketFromAddress(const IOAddress& addr,
 
 int IfaceMgr::openSocketFromRemoteAddress(const IOAddress& remote_addr,
                                           const uint16_t port) {
-    int sock = 0;
+    // Get local address to be used to connect to remote location.
     IOAddress local_address(getLocalAddress(remote_addr, port).getAddress());
-
-    for (IfaceCollection::iterator iface=ifaces_.begin();
-         iface!=ifaces_.end();
-         ++iface) {
-
-        AddressCollection addrs = iface->getAddresses();
-
-        for (AddressCollection::iterator addr = addrs.begin();
-             addr != addrs.end();
-             ++addr) {
-
-            if (addr->getAddress() != local_address.getAddress()) {
-                continue;
-            }
-
-            sock = openSocket(iface->getName(), *addr, port);
-            if (sock < 0) {
-                cout << "Failed to open unicast socket." << endl;
-            }
-            return (sock);
-        }
-    }
-    return (sock);
+    return openSocketFromAddress(local_address, port);
 }
 
 isc::asiolink::IOAddress
 IfaceMgr::getLocalAddress(const IOAddress& remote_addr, const uint16_t port) {
+    // Create remote endpoint, we will be connecting to it.
     boost::shared_ptr<const UDPEndpoint>
         remote_endpoint(static_cast<const UDPEndpoint*>
                         (UDPEndpoint::create(IPPROTO_UDP, remote_addr, port)));
+    if (!remote_endpoint) {
+        isc_throw(Unexpected, "Unable to create remote endpoint");
+    }
+
+    // Create socket that will be used to connect to remote endpoint.
     asio::io_service io_service;
     asio::ip::udp::socket sock(io_service);
+
+    // Try to connect to remote endpoint and check if attempt is successful.
     asio::error_code err_code;
     sock.connect(remote_endpoint->getASIOEndpoint(), err_code);
     if (err_code) {
-        isc_throw(Unexpected,"Failed to connect to remote address.");
+        isc_throw(Unexpected,"Failed to connect to remote endpoint.");
     }
 
-    asio::ip::udp::socket::endpoint_type local_endpoint =  sock.local_endpoint();
+    // Once we are connected socket object holds local endpoint.
+    asio::ip::udp::socket::endpoint_type local_endpoint =
+        sock.local_endpoint();
     asio::ip::address local_address(local_endpoint.address());
 
+    // Return address of local endpoint.
     return IOAddress(local_address);
 }
 

src/lib/dhcp/iface_mgr.h

@@ -388,6 +388,7 @@ public:
     /// @param port UDP port
     /// @return socket descriptor, if socket creation, binding and multicast
     /// group join were all successful.
+    /// @throw isc::Unexpected if failed to create and bind socket.
     int openSocketFromIface(const std::string& ifname,
                             const uint16_t port,
                             const uint8_t family);
@@ -401,6 +402,7 @@ public:
     /// @param port UDP port
     /// @return socket descriptor, if socket creation, binding and multicast
     /// group join were all successful.
+    /// @throw isc::Unexpected if failed to create and bind socket
     int openSocketFromAddress(const isc::asiolink::IOAddress& addr,
                               const uint16_t port);
 
@@ -415,6 +417,7 @@ public:
     /// @param port UDP port
     /// @return socket descriptor, if socket creation, binding and multicast
     /// group join were all successful.
+    /// @throw isc::Unexpected if failed to create and bind socket
     int openSocketFromRemoteAddress(const isc::asiolink::IOAddress& remote_addr,
                                     const uint16_t port);
 
@@ -577,7 +580,7 @@ private:
     ///
     /// This method identifies local network address that can be used
     /// to connect to remote address specified.
-    /// This method creates socket and makes attempt to connect
+    /// It first creates socket and makes attempt to connect
     /// to remote location via this socket. If connection
     /// is established successfully, the local address to which
     /// socket is bound is returned.
@@ -585,6 +588,7 @@ private:
     /// @param remote_addr remote address to connect to
     /// @param port port to be used
     /// @return local address to be used to connect to remote address
+    /// @throw isc::Unexpected if unable to indentify local address
     isc::asiolink::IOAddress
     getLocalAddress(const isc::asiolink::IOAddress& remote_addr,
                     const uint16_t port);

src/lib/dhcp/tests/iface_mgr_unittest.cc

@@ -244,14 +244,21 @@ TEST_F(IfaceMgrTest, socketsFromIface) {
     NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
 
     // open v6 socket on loopback interface and bind to 10547 port
-    int socket1 = ifacemgr->openSocketFromIface(LOOPBACK, 10547, AF_INET6);
+    int socket1 = 0;
+    EXPECT_NO_THROW(
+        socket1 = ifacemgr->openSocketFromIface(LOOPBACK, 10547, AF_INET6);
+    );
+    // socket descriptor must be positive integer
     EXPECT_GT(socket1, 0);
+    close(socket1);
 
     // open v4 sicket on loopback interface and bind to 10548 port
-    int socket2 = ifacemgr->openSocketFromIface(LOOPBACK, 10547, AF_INET);
+    int socket2 = 0;
+    EXPECT_NO_THROW(
+        socket2 = ifacemgr->openSocketFromIface(LOOPBACK, 10547, AF_INET);
+    );
+    // socket descriptor must be positive integer
     EXPECT_GT(socket2, 0);
-
-    close(socket1);
     close(socket2);
 
     delete ifacemgr;
@@ -262,16 +269,23 @@ TEST_F(IfaceMgrTest, socketsFromAddress) {
     NakedIfaceMgr* ifacemgr = new NakedIfaceMgr();
 
     // open v6 socket on loopback address and bind to 10547 port
+    int socket1 = 0;
     IOAddress loAddr6("::1");
-    int socket1 = ifacemgr->openSocketFromAddress(loAddr6, 10547);
+    EXPECT_NO_THROW(
+        socket1 = ifacemgr->openSocketFromAddress(loAddr6, 10547);
+    );
+    // socket descriptor must be positive integer
     EXPECT_GT(socket1, 0);
+    close(socket1);
 
-    // open v4 socket on loopip::udp::socketback address and bind to 10548 port
+    // open v4 socket on loopback address and bind to 10548 port
+    int socket2 = 0;
     IOAddress loAddr("127.0.0.1");
-    int socket2 = ifacemgr->openSocketFromAddress(loAddr, 10548);
+    EXPECT_NO_THROW(
+        socket2 = ifacemgr->openSocketFromAddress(loAddr, 10548);
+    );
+    // socket descriptor must be positive integer
     EXPECT_GT(socket2, 0);
-
-    close(socket1);
     close(socket2);
 
     delete ifacemgr;
@@ -283,16 +297,21 @@ TEST_F(IfaceMgrTest, socketsFromRemoteAddress) {
     // Open v6 socket to connect to remote address.
     // Loopback address is the only one that we know
     // so let's treat it as remote address.
+    int socket1 = 0;
     IOAddress loAddr6("::1");
-    int socket1 = ifacemgr->openSocketFromRemoteAddress(loAddr6, 10547);
+    EXPECT_NO_THROW(
+        socket1 = ifacemgr->openSocketFromRemoteAddress(loAddr6, 10547);
+    );
     EXPECT_GT(socket1, 0);
+    close(socket1);
 
     // Open v4 socket to connect to remote address.
+    int socket2 = 0;
     IOAddress loAddr("127.0.0.1");
-    int socket2 = ifacemgr->openSocketFromRemoteAddress(loAddr, 10548);
+    EXPECT_NO_THROW(
+        socket2 = ifacemgr->openSocketFromRemoteAddress(loAddr, 10548);
+    );
     EXPECT_GT(socket2, 0);
-
-    close(socket1);
     close(socket2);
 
     delete ifacemgr;