[#4258] Used an iterator argument

diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc
index 18e923b2feab97650b985eb0fec68a7c2e416218..121bbe026286d06784423292fcb9295ad2fcec14 100644 (file)
--- a/src/lib/dhcp/iface_mgr.cc
+++ b/src/lib/dhcp/iface_mgr.cc
@@ -922,29 +922,24 @@ IfaceMgr::clearBoundAddresses() {
 }
 
 void
-IfaceMgr::handleClosedExternalSocket(SocketCallbackInfo const& s) {
+IfaceMgr::handleClosedExternalSocket(SocketCallbackInfoIterator it) {
     errno = 0;
-    if (fcntl(s.socket_, F_GETFD) < 0 && (errno == EBADF)) {
-        SocketCallbackInfo x(s);
+    if (fcntl(it->socket_, F_GETFD) < 0 && (errno == EBADF)) {
+        SocketCallbackInfo x(*it);
         x.unusable_ = true;
-        auto& idx = callbacks_.get<1>();
-        auto it = idx.find(s.socket_);
-        // Expect that the external socket is still there!
-        if (it != idx.end()) {
-            idx.replace(it, x);
-        }
-        isc_throw(SocketFDError, "unexpected state (closed) for fd: " << s.socket_);
+        callbacks_.replace(it, x);
+        isc_throw(SocketFDError, "unexpected state (closed) for fd: " << x.socket_);
     }
 }
 
 void
 IfaceMgr::handleClosedExternalSockets() {
     std::lock_guard<std::mutex> lock(callbacks_mutex_);
-    for (SocketCallbackInfo s : callbacks_) {
-        if (s.unusable_) {
+    for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+        if (it->unusable_) {
             continue;
         }
-        handleClosedExternalSocket(s);
+        handleClosedExternalSocket(it);
     }
 }
 
@@ -1206,13 +1201,13 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
     {
         std::lock_guard<std::mutex> lock(callbacks_mutex_);
         if (!callbacks_.empty()) {
-            for (SocketCallbackInfo const& s : callbacks_) {
-                if (s.unusable_) {
+            for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+                if (it->unusable_) {
                     continue;
                 }
-                handleClosedExternalSocket(s);
+                handleClosedExternalSocket(it);
                 // Add this socket to listening set
-                fd_event_handler_->add(s.socket_);
+                fd_event_handler_->add(it->socket_);
             }
         }
     }
@@ -1273,20 +1268,20 @@ Pkt4Ptr IfaceMgr::receive4Indirect(uint32_t timeout_sec, uint32_t timeout_usec /
         bool found = false;
         {
             std::lock_guard<std::mutex> lock(callbacks_mutex_);
-            for (SocketCallbackInfo const& s : callbacks_) {
-                if (s.unusable_) {
+            for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+                if (it->unusable_) {
                     continue;
                 }
-                handleClosedExternalSocket(s);
-                if (fd_event_handler_->readReady(s.socket_) ||
-                    fd_event_handler_->hasError(s.socket_)) {
+                handleClosedExternalSocket(it);
+                if (fd_event_handler_->readReady(it->socket_) ||
+                    fd_event_handler_->hasError(it->socket_)) {
                     found = true;
 
                     // something received over external socket
-                    if (s.callback_) {
+                    if (it->callback_) {
                         // Note the external socket to call its callback without
                         // the lock taken so it can be deleted.
-                        ex_sock = s;
+                        ex_sock = *it;
                         break;
                     }
                 }
@@ -1341,13 +1336,13 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /*
     {
         std::lock_guard<std::mutex> lock(callbacks_mutex_);
         if (!callbacks_.empty()) {
-            for (SocketCallbackInfo const& s : callbacks_) {
-                if (s.unusable_) {
+            for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+                if (it->unusable_) {
                     continue;
                 }
-                handleClosedExternalSocket(s);
+                handleClosedExternalSocket(it);
                 // Add this socket to listening set
-                fd_event_handler_->add(s.socket_);
+                fd_event_handler_->add(it->socket_);
             }
         }
     }
@@ -1382,20 +1377,20 @@ Pkt4Ptr IfaceMgr::receive4Direct(uint32_t timeout_sec, uint32_t timeout_usec /*
     bool found = false;
     {
         std::lock_guard<std::mutex> lock(callbacks_mutex_);
-        for (SocketCallbackInfo const& s : callbacks_) {
-            if (s.unusable_) {
+        for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+            if (it->unusable_) {
                 continue;
             }
-            handleClosedExternalSocket(s);
-            if (fd_event_handler_->readReady(s.socket_) ||
-                fd_event_handler_->hasError(s.socket_)) {
+            handleClosedExternalSocket(it);
+            if (fd_event_handler_->readReady(it->socket_) ||
+                fd_event_handler_->hasError(it->socket_)) {
                 found = true;
 
                 // something received over external socket
-                if (s.callback_) {
+                if (it->callback_) {
                     // Note the external socket to call its callback without
                     // the lock taken so it can be deleted.
-                    ex_sock = s;
+                    ex_sock = *it;
                     break;
                 }
             }
@@ -1491,13 +1486,13 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ )
     {
         std::lock_guard<std::mutex> lock(callbacks_mutex_);
         if (!callbacks_.empty()) {
-            for (SocketCallbackInfo const& s : callbacks_) {
-                if (s.unusable_) {
+            for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+                if (it->unusable_) {
                     continue;
                 }
-                handleClosedExternalSocket(s);
+                handleClosedExternalSocket(it);
                 // Add this socket to listening set
-                fd_event_handler_->add(s.socket_);
+                fd_event_handler_->add(it->socket_);
             }
         }
     }
@@ -1532,20 +1527,20 @@ IfaceMgr::receive6Direct(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */ )
     bool found = false;
     {
         std::lock_guard<std::mutex> lock(callbacks_mutex_);
-        for (SocketCallbackInfo const& s : callbacks_) {
-            if (s.unusable_) {
+        for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+            if (it->unusable_) {
                 continue;
             }
-            handleClosedExternalSocket(s);
-            if (fd_event_handler_->readReady(s.socket_) ||
-                fd_event_handler_->hasError(s.socket_)) {
+            handleClosedExternalSocket(it);
+            if (fd_event_handler_->readReady(it->socket_) ||
+                fd_event_handler_->hasError(it->socket_)) {
                 found = true;
 
                 // something received over external socket
-                if (s.callback_) {
+                if (it->callback_) {
                     // Note the external socket to call its callback without
                     // the lock taken so it can be deleted.
-                    ex_sock = s;
+                    ex_sock = *it;
                     break;
                 }
             }
@@ -1615,13 +1610,13 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
     {
         std::lock_guard<std::mutex> lock(callbacks_mutex_);
         if (!callbacks_.empty()) {
-            for (SocketCallbackInfo const& s : callbacks_) {
-                if (s.unusable_) {
+            for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+                if (it->unusable_) {
                     continue;
                 }
-                handleClosedExternalSocket(s);
+                handleClosedExternalSocket(it);
                 // Add this socket to listening set
-                fd_event_handler_->add(s.socket_);
+                fd_event_handler_->add(it->socket_);
             }
         }
     }
@@ -1682,20 +1677,20 @@ IfaceMgr::receive6Indirect(uint32_t timeout_sec, uint32_t timeout_usec /* = 0 */
         bool found = false;
         {
             std::lock_guard<std::mutex> lock(callbacks_mutex_);
-            for (SocketCallbackInfo const& s : callbacks_) {
-                if (s.unusable_) {
+            for (auto it = callbacks_.begin(); it != callbacks_.end(); ++it) {
+                if (it->unusable_) {
                     continue;
                 }
-                handleClosedExternalSocket(s);
-                if (fd_event_handler_->readReady(s.socket_) ||
-                    fd_event_handler_->hasError(s.socket_)) {
+                handleClosedExternalSocket(it);
+                if (fd_event_handler_->readReady(it->socket_) ||
+                    fd_event_handler_->hasError(it->socket_)) {
                     found = true;
 
                     // something received over external socket
-                    if (s.callback_) {
+                    if (it->callback_) {
                         // Note the external socket to call its callback without
                         // the lock taken so it can be deleted.
-                        ex_sock = s;
+                        ex_sock = *it;
                         break;
                     }
                 }

diff --git a/src/lib/dhcp/iface_mgr.h b/src/lib/dhcp/iface_mgr.h
index 2542f5c82f9747143de8f46f6a5245123999ab7c..59b8695ed3b74d84c220fd44ef083f9fb4527ade 100644 (file)
--- a/src/lib/dhcp/iface_mgr.h
+++ b/src/lib/dhcp/iface_mgr.h
@@ -735,6 +735,9 @@ public:
         >
     > SocketCallbackInfoContainer;
 
+    /// @brief SocketCallbackInfo iterator type.
+    typedef SocketCallbackInfoContainer::iterator SocketCallbackInfoIterator;
+
     /// @brief Packet reception buffer size
     ///
     /// RFC 8415 states that server responses may be
@@ -1678,8 +1681,8 @@ private:
 
     /// @brief Handle closed external socket.
     ///
-    /// @param s The external socket info.
-    void handleClosedExternalSocket(SocketCallbackInfo const& s);
+    /// @param it The external socket info iterator.
+    void handleClosedExternalSocket(SocketCallbackInfoIterator it);
 
     /// @brief Handle closed external sockets.
     void handleClosedExternalSockets();

diff --git a/src/lib/dhcp/tests/iface_mgr_unittest.cc b/src/lib/dhcp/tests/iface_mgr_unittest.cc
index 97786e6c0580f1e54a834871dff615f29d20a89b..c1a3d5a2c19903e62516c97f72fceb8703c5f437 100644 (file)
--- a/src/lib/dhcp/tests/iface_mgr_unittest.cc
+++ b/src/lib/dhcp/tests/iface_mgr_unittest.cc
@@ -724,6 +724,7 @@ public:
                             callback_ok = (pipefd[0] == fd);
                         }));
         ASSERT_TRUE(ifacemgr->isExternalSocket(pipefd[0]));
+        ASSERT_FALSE(ifacemgr->isExternalSocketUnusable(pipefd[0]));
 
         // Let's create a second pipe and register it as well
         int secondpipe[2];
@@ -734,6 +735,7 @@ public:
                             callback2_ok = (secondpipe[0] == fd);
                         }));
         ASSERT_TRUE(ifacemgr->isExternalSocket(secondpipe[0]));
+        ASSERT_FALSE(ifacemgr->isExternalSocketUnusable(secondpipe[0]));
 
         // Verify a call with no data and normal external sockets works ok.
         Pkt4Ptr pkt4;
@@ -752,11 +754,14 @@ public:
         try {
             pkt4 = ifacemgr->receive4(RECEIVE_WAIT_MS(10));
         } catch (const SocketFDError& ex) {
-            std::string err_msg("unexpected state (closed) for fd: ");
-            EXPECT_EQ(err_msg, std::string(ex.what()).substr(0, err_msg.length()));
+            std::ostringstream err_msg;
+            err_msg << "unexpected state (closed) for fd: " << pipefd[0];
+            EXPECT_EQ(err_msg.str(), ex.what());
         } catch (const std::exception& ex) {
             ADD_FAILURE() << "wrong exception thrown: " << ex.what();
         }
+        EXPECT_TRUE(ifacemgr->isExternalSocketUnusable(pipefd[0]));
+        EXPECT_FALSE(ifacemgr->isExternalSocketUnusable(secondpipe[0]));
 
         // No callback invocations and no DHCPv4 pkt.
         EXPECT_FALSE(callback_ok);
@@ -812,6 +817,7 @@ public:
                             callback_ok = (pipefd[0] == fd);
                         }));
         ASSERT_TRUE(ifacemgr->isExternalSocket(pipefd[0]));
+        ASSERT_FALSE(ifacemgr->isExternalSocketUnusable(pipefd[0]));
 
         // Let's create a second pipe and register it as well
         int secondpipe[2];
@@ -822,6 +828,7 @@ public:
                             callback2_ok = (secondpipe[0] == fd);
                         }));
         ASSERT_TRUE(ifacemgr->isExternalSocket(secondpipe[0]));
+        ASSERT_FALSE(ifacemgr->isExternalSocketUnusable(secondpipe[0]));
 
         // Verify a call with no data and normal external sockets works ok.
         Pkt6Ptr pkt6;
@@ -840,11 +847,14 @@ public:
         try {
             pkt6 = ifacemgr->receive6(RECEIVE_WAIT_MS(10));
         } catch (const SocketFDError& ex) {
-            std::string err_msg("unexpected state (closed) for fd: ");
-            EXPECT_EQ(err_msg, std::string(ex.what()).substr(0, err_msg.length()));
+            std::ostringstream err_msg;
+            err_msg << "unexpected state (closed) for fd: " << pipefd[0];
+            EXPECT_EQ(err_msg.str(), ex.what());
         } catch (const std::exception& ex) {
             ADD_FAILURE() << "wrong exception thrown: " << ex.what();
         }
+        EXPECT_TRUE(ifacemgr->isExternalSocketUnusable(pipefd[0]));
+        EXPECT_FALSE(ifacemgr->isExternalSocketUnusable(secondpipe[0]));
 
         // No callback invocations and no DHCPv6 pkt.
         EXPECT_FALSE(callback_ok);