From: Francis Dupont Date: Thu, 11 Dec 2025 09:15:43 +0000 (+0100) Subject: [#4258] Used an iterator argument X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f4558fb3d539277ad185d74c256ec660bfc33df;p=thirdparty%2Fkea.git [#4258] Used an iterator argument --- diff --git a/src/lib/dhcp/iface_mgr.cc b/src/lib/dhcp/iface_mgr.cc index 18e923b2fe..121bbe0262 100644 --- 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 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 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 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 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 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 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 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 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 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 2542f5c82f..59b8695ed3 100644 --- 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 97786e6c05..c1a3d5a2c1 100644 --- 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);