[#3281] addressed review comments

diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc
index 088b4b022203c591550b300ed42301ec5cfd38b5..db0c632be8ea0a7fafa203e350f653e800e85a53 100644 (file)
--- 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<Option4ClientFqdn>(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<Option4ClientFqdn>
-                                            (resp->getOption(DHO_FQDN));
+                fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>(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 dc0e8c086f39cc1ea179eaec2f5f9cedf4809b63..85338837e9904510bf7c599229cb599d65ce64a2 100644 (file)
--- 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 3dedb20c5607384515cbcc5b6b3efa178b154c72..f9f74f7a6707435ecaecb2a38ba61a06aad223d1 100644 (file)
--- 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 6b602553f2484bcf257662b8140f144366815a3d..2efc99f74ecdea7fc0168b92abdcf836fc4ba81b 100644 (file)
--- 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 c295f919313ebd0ed9d7020bb94314fa25f48511..9b00b3fb9910d38b8e75f1a544275d11d29a6bac 100644 (file)
--- 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 c1390519b414ef18765771bd617b085e4e16e4e8..bd9376d5afa3adbd6e9aa72f0b5af52e047e738e 100644 (file)
--- 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 a045bc59dc0aeec78698406350198ca9b4d442db..80eb0b2f7e630a4c3c7e7807e00e6987226c5c6e 100644 (file)
--- 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 066af32212626648a3106cd878b6853ce9762677..c9aaf0b1d045755ee6d46b72706e732fee013b94 100644 (file)
--- 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 0908950281355d0fb70552c43bd008ce4f357c02..8dff72a7c0bbfd76bfd8764d398911c46d369bfe 100644 (file)
--- 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 7a29ea37c3ab2554e0539c136a03c877da86886e..73c09d4636137fb1704841a0e890f12f2640e9ff 100644 (file)
--- 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 7d98a81e45a9ba84dc3a0967e04e0c5dc259d503..6984caa9aeae8c926937d4f2e708796daade241c 100644 (file)
--- 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 5396c8f8a4d0abb1c865ad6497cd7e7ee65864a2..92bfe8f7ea090def5ab722f3c3577163c234995b 100644 (file)
--- 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 583e223a1071d9d6fa720f1c5410a070f0930f3f..9d1714117fc8815e2a808ef0993bbe2ac08e532f 100644 (file)
--- 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;