From: Razvan Becheriu Date: Thu, 4 Apr 2024 07:08:49 +0000 (+0300) Subject: [#3281] addressed review comments X-Git-Tag: Kea-2.5.8~86 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2cb3999f620dbcb0c8032d8e69e820d721c43621;p=thirdparty%2Fkea.git [#3281] addressed review comments --- diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index 088b4b0222..db0c632be8 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -2474,7 +2474,6 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) { bool fqdn_fwd = false; bool fqdn_rev = false; - OptionStringPtr opt_hostname; fqdn = boost::dynamic_pointer_cast(resp->getOption(DHO_FQDN)); if (fqdn) { @@ -2556,8 +2555,7 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) { // If there's an outbound FQDN option in the response we need // to update it with the new host name. - Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast - (resp->getOption(DHO_FQDN)); + fqdn = boost::dynamic_pointer_cast(resp->getOption(DHO_FQDN)); if (fqdn) { fqdn->setDomainName(hook_hostname, Option4ClientFqdn::FULL); // Hook disabled updates, Set flags back to client accordingly. @@ -2613,7 +2611,6 @@ Dhcpv4Srv::processClientFqdnOption(Dhcpv4Exchange& ex) { if (ex.getContext()->currentHost() && !ex.getContext()->currentHost()->getHostname().empty()) { - D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr(); fqdn_resp->setDomainName(d2_mgr.qualifyName(ex.getContext()->currentHost()->getHostname(), *(ex.getContext()->getDdnsParams()), true), Option4ClientFqdn::FULL); @@ -4669,8 +4666,8 @@ void Dhcpv4Srv::requiredClassify(Dhcpv4Exchange& ex) { if (!addr.isV4Zero()) { PoolPtr pool = subnet->getPool(Lease::TYPE_V4, addr, false); if (pool) { - const ClientClasses& to_add = pool->getRequiredClasses(); - for (auto const& cclass : to_add) { + const ClientClasses& pool_to_add = pool->getRequiredClasses(); + for (auto const& cclass : pool_to_add) { classes.insert(cclass); } } diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index dc0e8c086f..85338837e9 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -4435,8 +4435,8 @@ Dhcpv6Srv::requiredClassify(const Pkt6Ptr& pkt, AllocEngine::ClientContext6& ctx resource.getAddress(), false); if (pool) { - const ClientClasses& to_add = pool->getRequiredClasses(); - for (auto const& cclass : to_add) { + const ClientClasses& pool_to_add = pool->getRequiredClasses(); + for (auto const& cclass : pool_to_add) { classes.insert(cclass); } } diff --git a/src/bin/netconf/tests/control_socket_unittests.cc b/src/bin/netconf/tests/control_socket_unittests.cc index 3dedb20c56..f9f74f7a67 100644 --- a/src/bin/netconf/tests/control_socket_unittests.cc +++ b/src/bin/netconf/tests/control_socket_unittests.cc @@ -135,7 +135,7 @@ TEST(StdoutControlSocketTest, configSet) { //////////////////////////////// UNIX //////////////////////////////// /// @brief Test timeout in ms. -const long TEST_TIMEOUT = 10000; +const long TEST_TIMEOUT = 1500; /// @brief Test fixture class for unix control sockets. class UnixControlSocketTest : public ThreadedTest { @@ -231,7 +231,7 @@ UnixControlSocketTest::reflectServer() { timer.setup([&timeout]() { timeout = true; FAIL() << "timeout"; - }, 1500, IntervalTimer::ONE_SHOT); + }, TEST_TIMEOUT, IntervalTimer::ONE_SHOT); // Accept. bool accepted = false; diff --git a/src/bin/netconf/tests/netconf_unittests.cc b/src/bin/netconf/tests/netconf_unittests.cc index 6b602553f2..2efc99f74e 100644 --- a/src/bin/netconf/tests/netconf_unittests.cc +++ b/src/bin/netconf/tests/netconf_unittests.cc @@ -147,9 +147,9 @@ public: string socket_path; const char* env = getenv("KEA_SOCKET_TEST_DIR"); if (env) { - socket_path = string(env) + "/test-socket"; + socket_path = string(env) + "/" + TEST_SOCKET; } else { - socket_path = sandbox.join("test-socket"); + socket_path = sandbox.join(TEST_SOCKET); } return (socket_path); } diff --git a/src/hooks/dhcp/high_availability/ha_service.cc b/src/hooks/dhcp/high_availability/ha_service.cc index c295f91931..9b00b3fb99 100644 --- a/src/hooks/dhcp/high_availability/ha_service.cc +++ b/src/hooks/dhcp/high_availability/ha_service.cc @@ -1088,7 +1088,7 @@ HAService::adjustNetworkState() { (getCurrState() == HA_TERMINATED_ST)); if (!should_enable && network_state_->isServiceEnabled()) { - std::string current_state_name = getStateLabel(getCurrState()); + current_state_name = getStateLabel(getCurrState()); boost::to_upper(current_state_name); LOG_INFO(ha_logger, HA_LOCAL_DHCP_DISABLE) .arg(config_->getThisServerName()) @@ -1096,7 +1096,7 @@ HAService::adjustNetworkState() { network_state_->disableService(getLocalOrigin()); } else if (should_enable && !network_state_->isServiceEnabled()) { - std::string current_state_name = getStateLabel(getCurrState()); + current_state_name = getStateLabel(getCurrState()); boost::to_upper(current_state_name); LOG_INFO(ha_logger, HA_LOCAL_DHCP_ENABLE) .arg(config_->getThisServerName()) @@ -1677,7 +1677,7 @@ HAService::processStatusGet() const { try { role = config_->getFailoverPeerConfig()->getRole(); - std::string role_txt = HAConfig::PeerConfig::roleToString(role); + role_txt = HAConfig::PeerConfig::roleToString(role); remote->set("role", Element::create(role_txt)); } catch (...) { diff --git a/src/lib/asiolink/tests/interval_timer_unittest.cc b/src/lib/asiolink/tests/interval_timer_unittest.cc index c1390519b4..bd9376d5af 100644 --- a/src/lib/asiolink/tests/interval_timer_unittest.cc +++ b/src/lib/asiolink/tests/interval_timer_unittest.cc @@ -13,12 +13,6 @@ using namespace isc::asiolink; -namespace { -// TODO: Consider this margin -const boost::posix_time::time_duration TIMER_MARGIN_MSEC = - boost::posix_time::milliseconds(50); -} - // This fixture is for testing IntervalTimer. Some callback functors are // registered as callback function of the timer to test if they are called // or not. @@ -48,16 +42,13 @@ protected: }; class TimerCallBackCounter { public: - TimerCallBackCounter(IntervalTimerTest* test_obj) : - test_obj_(test_obj) { + TimerCallBackCounter(IntervalTimerTest* /* test_obj */) { counter_ = 0; } void operator()() { ++counter_; } int counter_; - private: - IntervalTimerTest* test_obj_; }; class TimerCallBackCancelDeleter { public: @@ -133,14 +124,13 @@ protected: }; class TimerCallBackAccumulator { public: - TimerCallBackAccumulator(IntervalTimerTest* test_obj, int &counter) : - test_obj_(test_obj), counter_(counter) { + TimerCallBackAccumulator(IntervalTimerTest* /* test_obj */, int &counter) : + counter_(counter) { } void operator()() { ++counter_; } private: - IntervalTimerTest* test_obj_; // Reference to integer accumulator int& counter_; }; diff --git a/src/lib/asiolink/tests/tls_unittest.cc b/src/lib/asiolink/tests/tls_unittest.cc index a045bc59dc..80eb0b2f7e 100644 --- a/src/lib/asiolink/tests/tls_unittest.cc +++ b/src/lib/asiolink/tests/tls_unittest.cc @@ -103,8 +103,7 @@ public: } /// @brief Destructor. - virtual ~TestCallback() { - } + virtual ~TestCallback() = default; /// @brief Callback function (one argument). /// diff --git a/src/lib/d2srv/testutils/nc_test_utils.cc b/src/lib/d2srv/testutils/nc_test_utils.cc index 066af32212..c9aaf0b1d0 100644 --- a/src/lib/d2srv/testutils/nc_test_utils.cc +++ b/src/lib/d2srv/testutils/nc_test_utils.cc @@ -142,12 +142,12 @@ FauxServer::requestHandler(const boost::system::error_code& error, // If context is not NULL, then we need to verify the message. if (context) { - dns::TSIGError error = context->verify(request.getTSIGRecord(), - receive_buffer_, - bytes_recvd); - if (error != dns::TSIGError::NOERROR()) { + dns::TSIGError tsig_error = context->verify(request.getTSIGRecord(), + receive_buffer_, + bytes_recvd); + if (tsig_error != dns::TSIGError::NOERROR()) { isc_throw(TSIGVerifyError, "TSIG verification failed: " - << error.toText()); + << tsig_error.toText()); } } } catch (const std::exception& ex) { diff --git a/src/lib/dhcpsrv/tests/d2_client_unittest.cc b/src/lib/dhcpsrv/tests/d2_client_unittest.cc index 0908950281..8dff72a7c0 100644 --- a/src/lib/dhcpsrv/tests/d2_client_unittest.cc +++ b/src/lib/dhcpsrv/tests/d2_client_unittest.cc @@ -1233,7 +1233,6 @@ TEST_F(D2ClientMgrParamsTest, sanitizeFqdnV6) { } }; - Option6ClientFqdnPtr response; for (auto const& scenario : scenarios) { SCOPED_TRACE(scenario.description_); { diff --git a/src/lib/http/tests/client_mt_unittests.cc b/src/lib/http/tests/client_mt_unittests.cc index 7a29ea37c3..73c09d4636 100644 --- a/src/lib/http/tests/client_mt_unittests.cc +++ b/src/lib/http/tests/client_mt_unittests.cc @@ -504,11 +504,11 @@ public: // Start the requisite number of requests: // batch * listeners * threads. - int sequence = 0; - for (auto b = 0; b < num_batches; ++b) { - for (auto l = 0; l < num_listeners_; ++l) { - for (auto t = 0; t < effective_threads; ++t) { - startRequest(++sequence, l); + int sequence_nr = 0; + for (size_t b = 0; b < num_batches; ++b) { + for (size_t l = 0; l < num_listeners_; ++l) { + for (size_t t = 0; t < effective_threads; ++t) { + startRequest(++sequence_nr, l); } } } @@ -662,11 +662,11 @@ public: // Start the requisite number of requests: // batch * listeners * threads. - int sequence = 0; - for (auto b = 0; b < num_batches; ++b) { - for (auto l = 0; l < num_listeners_; ++l) { - for (auto t = 0; t < num_threads_; ++t) { - startRequestSimple(++sequence, l); + int sequence_nr = 0; + for (size_t b = 0; b < num_batches; ++b) { + for (size_t l = 0; l < num_listeners_; ++l) { + for (size_t t = 0; t < num_threads_; ++t) { + startRequestSimple(++sequence_nr, l); } } } diff --git a/src/lib/http/tests/tls_server_unittests.cc b/src/lib/http/tests/tls_server_unittests.cc index 7d98a81e45..6984caa9ae 100644 --- a/src/lib/http/tests/tls_server_unittests.cc +++ b/src/lib/http/tests/tls_server_unittests.cc @@ -48,9 +48,6 @@ namespace { /// @brief IP address to which HTTP service is bound. const std::string SERVER_ADDRESS = "127.0.0.1"; -/// @brief IPv6 address to whch HTTP service is bound. -const std::string IPV6_SERVER_ADDRESS = "::1"; - /// @brief Port number to which HTTP service is bound. const unsigned short SERVER_PORT = 18123; @@ -60,10 +57,6 @@ const long REQUEST_TIMEOUT = 10000; /// @brief Persistent connection idle timeout used in most of the tests (ms). const long IDLE_TIMEOUT = 10000; -/// @brief Persistent connection idle timeout used in tests where idle connections -/// are tested (ms). -const long SHORT_IDLE_TIMEOUT = 200; - /// @brief Test timeout (ms). const long TEST_TIMEOUT = 10000; diff --git a/src/lib/tcp/tests/tcp_listener_unittests.cc b/src/lib/tcp/tests/tcp_listener_unittests.cc index 5396c8f8a4..92bfe8f7ea 100644 --- a/src/lib/tcp/tests/tcp_listener_unittests.cc +++ b/src/lib/tcp/tests/tcp_listener_unittests.cc @@ -33,19 +33,9 @@ namespace { /// @brief IP address to which service is bound. const std::string SERVER_ADDRESS = "127.0.0.1"; -/// @brief IPv6 address to whch service is bound. -const std::string IPV6_SERVER_ADDRESS = "::1"; - /// @brief Port number to which service is bound. const unsigned short SERVER_PORT = 18123; -/// @brief Request Timeout used in most of the tests (ms). -const long REQUEST_TIMEOUT = 10000; - -/// @brief Connection idle timeout used in tests where idle connections -/// are tested (ms). -const long SHORT_REQUEST_TIMEOUT = 200; - /// @brief Connection idle timeout used in most of the tests (ms). const long IDLE_TIMEOUT = 10000; diff --git a/src/lib/tcp/tests/tls_listener_unittests.cc b/src/lib/tcp/tests/tls_listener_unittests.cc index 583e223a10..9d1714117f 100644 --- a/src/lib/tcp/tests/tls_listener_unittests.cc +++ b/src/lib/tcp/tests/tls_listener_unittests.cc @@ -28,19 +28,9 @@ namespace { /// @brief IP address to which service is bound. const std::string SERVER_ADDRESS = "127.0.0.1"; -/// @brief IPv6 address to whch service is bound. -const std::string IPV6_SERVER_ADDRESS = "::1"; - /// @brief Port number to which service is bound. const unsigned short SERVER_PORT = 18123; -/// @brief Request Timeout used in most of the tests (ms). -const long REQUEST_TIMEOUT = 10000; - -/// @brief Connection idle timeout used in tests where idle connections -/// are tested (ms). -const long SHORT_REQUEST_TIMEOUT = 200; - /// @brief Connection idle timeout used in most of the tests (ms). const long IDLE_TIMEOUT = 10000;