Skip to content

Commit 2cb3999

Browse files
author
Razvan Becheriu
committed
[#3281] addressed review comments
1 parent 3127326 commit 2cb3999

File tree

13 files changed

+31
-73
lines changed

13 files changed

+31
-73
lines changed

‎src/bin/dhcp4/dhcp4_srv.cc

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2474,7 +2474,6 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
24742474
bool fqdn_fwd = false;
24752475
bool fqdn_rev = false;
24762476

2477-
24782477
OptionStringPtr opt_hostname;
24792478
fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>(resp->getOption(DHO_FQDN));
24802479
if (fqdn) {
@@ -2556,8 +2555,7 @@ Dhcpv4Srv::processClientName(Dhcpv4Exchange& ex) {
25562555

25572556
// If there's an outbound FQDN option in the response we need
25582557
// to update it with the new host name.
2559-
Option4ClientFqdnPtr fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>
2560-
(resp->getOption(DHO_FQDN));
2558+
fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>(resp->getOption(DHO_FQDN));
25612559
if (fqdn) {
25622560
fqdn->setDomainName(hook_hostname, Option4ClientFqdn::FULL);
25632561
// Hook disabled updates, Set flags back to client accordingly.
@@ -2613,7 +2611,6 @@ Dhcpv4Srv::processClientFqdnOption(Dhcpv4Exchange& ex) {
26132611

26142612
if (ex.getContext()->currentHost() &&
26152613
!ex.getContext()->currentHost()->getHostname().empty()) {
2616-
D2ClientMgr& d2_mgr = CfgMgr::instance().getD2ClientMgr();
26172614
fqdn_resp->setDomainName(d2_mgr.qualifyName(ex.getContext()->currentHost()->getHostname(),
26182615
*(ex.getContext()->getDdnsParams()), true),
26192616
Option4ClientFqdn::FULL);
@@ -4669,8 +4666,8 @@ void Dhcpv4Srv::requiredClassify(Dhcpv4Exchange& ex) {
46694666
if (!addr.isV4Zero()) {
46704667
PoolPtr pool = subnet->getPool(Lease::TYPE_V4, addr, false);
46714668
if (pool) {
4672-
const ClientClasses& to_add = pool->getRequiredClasses();
4673-
for (auto const& cclass : to_add) {
4669+
const ClientClasses& pool_to_add = pool->getRequiredClasses();
4670+
for (auto const& cclass : pool_to_add) {
46744671
classes.insert(cclass);
46754672
}
46764673
}

‎src/bin/dhcp6/dhcp6_srv.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4435,8 +4435,8 @@ Dhcpv6Srv::requiredClassify(const Pkt6Ptr& pkt, AllocEngine::ClientContext6& ctx
44354435
resource.getAddress(),
44364436
false);
44374437
if (pool) {
4438-
const ClientClasses& to_add = pool->getRequiredClasses();
4439-
for (auto const& cclass : to_add) {
4438+
const ClientClasses& pool_to_add = pool->getRequiredClasses();
4439+
for (auto const& cclass : pool_to_add) {
44404440
classes.insert(cclass);
44414441
}
44424442
}

‎src/bin/netconf/tests/control_socket_unittests.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ TEST(StdoutControlSocketTest, configSet) {
135135
//////////////////////////////// UNIX ////////////////////////////////
136136

137137
/// @brief Test timeout in ms.
138-
const long TEST_TIMEOUT = 10000;
138+
const long TEST_TIMEOUT = 1500;
139139

140140
/// @brief Test fixture class for unix control sockets.
141141
class UnixControlSocketTest : public ThreadedTest {
@@ -231,7 +231,7 @@ UnixControlSocketTest::reflectServer() {
231231
timer.setup([&timeout]() {
232232
timeout = true;
233233
FAIL() << "timeout";
234-
}, 1500, IntervalTimer::ONE_SHOT);
234+
}, TEST_TIMEOUT, IntervalTimer::ONE_SHOT);
235235

236236
// Accept.
237237
bool accepted = false;

‎src/bin/netconf/tests/netconf_unittests.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,9 @@ class NetconfAgentTest : public ThreadedTest {
147147
string socket_path;
148148
const char* env = getenv("KEA_SOCKET_TEST_DIR");
149149
if (env) {
150-
socket_path = string(env) + "/test-socket";
150+
socket_path = string(env) + "/" + TEST_SOCKET;
151151
} else {
152-
socket_path = sandbox.join("test-socket");
152+
socket_path = sandbox.join(TEST_SOCKET);
153153
}
154154
return (socket_path);
155155
}

‎src/hooks/dhcp/high_availability/ha_service.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,15 +1088,15 @@ HAService::adjustNetworkState() {
10881088
(getCurrState() == HA_TERMINATED_ST));
10891089

10901090
if (!should_enable && network_state_->isServiceEnabled()) {
1091-
std::string current_state_name = getStateLabel(getCurrState());
1091+
current_state_name = getStateLabel(getCurrState());
10921092
boost::to_upper(current_state_name);
10931093
LOG_INFO(ha_logger, HA_LOCAL_DHCP_DISABLE)
10941094
.arg(config_->getThisServerName())
10951095
.arg(current_state_name);
10961096
network_state_->disableService(getLocalOrigin());
10971097

10981098
} else if (should_enable && !network_state_->isServiceEnabled()) {
1099-
std::string current_state_name = getStateLabel(getCurrState());
1099+
current_state_name = getStateLabel(getCurrState());
11001100
boost::to_upper(current_state_name);
11011101
LOG_INFO(ha_logger, HA_LOCAL_DHCP_ENABLE)
11021102
.arg(config_->getThisServerName())
@@ -1677,7 +1677,7 @@ HAService::processStatusGet() const {
16771677

16781678
try {
16791679
role = config_->getFailoverPeerConfig()->getRole();
1680-
std::string role_txt = HAConfig::PeerConfig::roleToString(role);
1680+
role_txt = HAConfig::PeerConfig::roleToString(role);
16811681
remote->set("role", Element::create(role_txt));
16821682

16831683
} catch (...) {

‎src/lib/asiolink/tests/interval_timer_unittest.cc

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@
1313

1414
using namespace isc::asiolink;
1515

16-
namespace {
17-
// TODO: Consider this margin
18-
const boost::posix_time::time_duration TIMER_MARGIN_MSEC =
19-
boost::posix_time::milliseconds(50);
20-
}
21-
2216
// This fixture is for testing IntervalTimer. Some callback functors are
2317
// registered as callback function of the timer to test if they are called
2418
// or not.
@@ -48,16 +42,13 @@ class IntervalTimerTest : public ::testing::Test {
4842
};
4943
class TimerCallBackCounter {
5044
public:
51-
TimerCallBackCounter(IntervalTimerTest* test_obj) :
52-
test_obj_(test_obj) {
45+
TimerCallBackCounter(IntervalTimerTest* /* test_obj */) {
5346
counter_ = 0;
5447
}
5548
void operator()() {
5649
++counter_;
5750
}
5851
int counter_;
59-
private:
60-
IntervalTimerTest* test_obj_;
6152
};
6253
class TimerCallBackCancelDeleter {
6354
public:
@@ -133,14 +124,13 @@ class IntervalTimerTest : public ::testing::Test {
133124
};
134125
class TimerCallBackAccumulator {
135126
public:
136-
TimerCallBackAccumulator(IntervalTimerTest* test_obj, int &counter) :
137-
test_obj_(test_obj), counter_(counter) {
127+
TimerCallBackAccumulator(IntervalTimerTest* /* test_obj */, int &counter) :
128+
counter_(counter) {
138129
}
139130
void operator()() {
140131
++counter_;
141132
}
142133
private:
143-
IntervalTimerTest* test_obj_;
144134
// Reference to integer accumulator
145135
int& counter_;
146136
};

‎src/lib/asiolink/tests/tls_unittest.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ class TestCallback {
103103
}
104104

105105
/// @brief Destructor.
106-
virtual ~TestCallback() {
107-
}
106+
virtual ~TestCallback() = default;
108107

109108
/// @brief Callback function (one argument).
110109
///

‎src/lib/d2srv/testutils/nc_test_utils.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,12 @@ FauxServer::requestHandler(const boost::system::error_code& error,
142142

143143
// If context is not NULL, then we need to verify the message.
144144
if (context) {
145-
dns::TSIGError error = context->verify(request.getTSIGRecord(),
146-
receive_buffer_,
147-
bytes_recvd);
148-
if (error != dns::TSIGError::NOERROR()) {
145+
dns::TSIGError tsig_error = context->verify(request.getTSIGRecord(),
146+
receive_buffer_,
147+
bytes_recvd);
148+
if (tsig_error != dns::TSIGError::NOERROR()) {
149149
isc_throw(TSIGVerifyError, "TSIG verification failed: "
150-
<< error.toText());
150+
<< tsig_error.toText());
151151
}
152152
}
153153
} catch (const std::exception& ex) {

‎src/lib/dhcpsrv/tests/d2_client_unittest.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1233,7 +1233,6 @@ TEST_F(D2ClientMgrParamsTest, sanitizeFqdnV6) {
12331233
}
12341234
};
12351235

1236-
Option6ClientFqdnPtr response;
12371236
for (auto const& scenario : scenarios) {
12381237
SCOPED_TRACE(scenario.description_);
12391238
{

‎src/lib/http/tests/client_mt_unittests.cc

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -504,11 +504,11 @@ class MultiThreadingHttpClientTest : public ::testing::Test {
504504

505505
// Start the requisite number of requests:
506506
// batch * listeners * threads.
507-
int sequence = 0;
508-
for (auto b = 0; b < num_batches; ++b) {
509-
for (auto l = 0; l < num_listeners_; ++l) {
510-
for (auto t = 0; t < effective_threads; ++t) {
511-
startRequest(++sequence, l);
507+
int sequence_nr = 0;
508+
for (size_t b = 0; b < num_batches; ++b) {
509+
for (size_t l = 0; l < num_listeners_; ++l) {
510+
for (size_t t = 0; t < effective_threads; ++t) {
511+
startRequest(++sequence_nr, l);
512512
}
513513
}
514514
}
@@ -662,11 +662,11 @@ class MultiThreadingHttpClientTest : public ::testing::Test {
662662

663663
// Start the requisite number of requests:
664664
// batch * listeners * threads.
665-
int sequence = 0;
666-
for (auto b = 0; b < num_batches; ++b) {
667-
for (auto l = 0; l < num_listeners_; ++l) {
668-
for (auto t = 0; t < num_threads_; ++t) {
669-
startRequestSimple(++sequence, l);
665+
int sequence_nr = 0;
666+
for (size_t b = 0; b < num_batches; ++b) {
667+
for (size_t l = 0; l < num_listeners_; ++l) {
668+
for (size_t t = 0; t < num_threads_; ++t) {
669+
startRequestSimple(++sequence_nr, l);
670670
}
671671
}
672672
}

0 commit comments

Comments
 (0)