[#3281] addressed review comments

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);
                 }
             }

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);
                 }
             }

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;

src/bin/netconf/tests/netconf_unittests.cc

@@ -147,9 +147,9 @@ class NetconfAgentTest : public ThreadedTest {
         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);
     }

src/hooks/dhcp/high_availability/ha_service.cc

@@ -1088,15 +1088,15 @@ 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())
             .arg(current_state_name);
         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 (...) {

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 @@ class IntervalTimerTest : public ::testing::Test {
     };
     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 @@ class IntervalTimerTest : public ::testing::Test {
     };
     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_;
     };

src/lib/asiolink/tests/tls_unittest.cc

@@ -103,8 +103,7 @@ class TestCallback {
     }
 
     /// @brief Destructor.
-    virtual ~TestCallback() {
-    }
+    virtual ~TestCallback() = default;
 
     /// @brief Callback function (one argument).
     ///

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) {

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_);
         {

src/lib/http/tests/client_mt_unittests.cc

@@ -504,11 +504,11 @@ class MultiThreadingHttpClientTest : public ::testing::Test {
 
         // 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 @@ class MultiThreadingHttpClientTest : public ::testing::Test {
 
         // 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);
                 }
             }
         }

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;
 

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;
 

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;