Skip to content

Commit

Permalink
[#3190] fixed ASAN warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
Razvan Becheriu committed Mar 5, 2024
1 parent 8fc6b6f commit 133ccdf
Show file tree
Hide file tree
Showing 59 changed files with 941 additions and 588 deletions.
6 changes: 5 additions & 1 deletion src/bin/agent/ca_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ CtrlAgentProcess::CtrlAgentProcess(const char* name,
}

CtrlAgentProcess::~CtrlAgentProcess() {
garbageCollectListeners(0);
}

void
Expand Down Expand Up @@ -206,7 +207,10 @@ CtrlAgentProcess::garbageCollectListeners(size_t leaving) {
}
// We have stopped listeners but there may be some pending handlers
// related to these listeners. Need to invoke these handlers.
getIOService()->poll();
try {
getIOService()->poll();
} catch (...) {
}
// Finally, we're ready to remove no longer used listeners.
http_listeners_.erase(http_listeners_.begin(),
http_listeners_.end() - leaving);
Expand Down
3 changes: 0 additions & 3 deletions src/bin/agent/tests/ca_controller_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ TEST_F(CtrlAgentControllerTest, basicInstanceTesting) {
EXPECT_FALSE(checkProcess());
}


// Tests basic command line processing.
// Verifies that:
// 1. Standard command line options are supported.
Expand Down Expand Up @@ -680,7 +679,6 @@ TEST_F(CtrlAgentControllerTest, configReloadFileValid) {
answer = CtrlAgentCommandMgr::instance().handleCommand("config-reload",
params, cmd);


// Verify the reload was successful.
string expected = "{ \"result\": 0, \"text\": "
"\"Configuration applied successfully.\" }";
Expand Down Expand Up @@ -785,7 +783,6 @@ TEST_F(CtrlAgentControllerTest, shutdown) {
ctrl->deregisterCommands();
}


TEST_F(CtrlAgentControllerTest, shutdownExitValue) {
ASSERT_NO_THROW(initProcess());
EXPECT_TRUE(checkProcess());
Expand Down
8 changes: 7 additions & 1 deletion src/bin/agent/tests/ca_process_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,13 @@ TEST_F(CtrlAgentProcessTest, shutdown) {
time_duration elapsed = stop - start;
EXPECT_TRUE(elapsed.total_milliseconds() >= 100 &&
elapsed.total_milliseconds() <= 400);
}

timer.cancel();
getIOService()->restart();
try {
getIOService()->poll();
} catch (...) {
}
}

}
3 changes: 3 additions & 0 deletions src/bin/d2/d2_process.cc
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,9 @@ D2Process::reconfigureQueueMgr() {
}

D2Process::~D2Process() {
queue_mgr_->stopListening();
auto f = [](D2QueueMgrPtr) {};
getIOService()->post(std::bind(f, queue_mgr_));
}

D2CfgMgrPtr
Expand Down
2 changes: 0 additions & 2 deletions src/bin/d2/d2_queue_mgr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ D2QueueMgr::updateStopState() {
DHCP_DDNS_QUEUE_MGR_STOPPED);
}


void
D2QueueMgr::removeListener() {
// Force our managing layer(s) to stop us properly first.
Expand Down Expand Up @@ -224,7 +223,6 @@ D2QueueMgr::dequeueAt(const size_t index) {
ncr_queue_.erase(pos);
}


void
D2QueueMgr::dequeue() {
if (getQueueSize() == 0) {
Expand Down
7 changes: 2 additions & 5 deletions src/bin/d2/d2_queue_mgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class D2QueueMgrReceiveError : public isc::Exception {
isc::Exception(file, line, what) { };
};


/// @brief Thrown if the request queue is full when an enqueue is attempted.
/// @todo use or remove it.
class D2QueueMgrQueueFull : public isc::Exception {
Expand All @@ -63,7 +62,6 @@ class D2QueueMgrInvalidIndex : public isc::Exception {
isc::Exception(file, line, what) { };
};


/// @brief D2QueueMgr creates and manages a queue of DNS update requests.
///
/// D2QueueMgr is a class specifically designed as an integral part of DHCP-DDNS.
Expand Down Expand Up @@ -217,8 +215,8 @@ class D2QueueMgr : public dhcp_ddns::NameChangeListener::RequestReceiveHandler,
/// @param ncr is a pointer to the newly received NameChangeRequest if
/// result is NameChangeListener::SUCCESS. It is indeterminate other
/// wise.
virtual void operator ()(const dhcp_ddns::NameChangeListener::Result result,
dhcp_ddns::NameChangeRequestPtr& ncr);
virtual void operator()(const dhcp_ddns::NameChangeListener::Result result,
dhcp_ddns::NameChangeRequestPtr& ncr);

/// @brief Stops listening for requests.
///
Expand All @@ -234,7 +232,6 @@ class D2QueueMgr : public dhcp_ddns::NameChangeListener::RequestReceiveHandler,
/// @throw D2QueueMgrError if stop_state is a valid stop state.
void stopListening(const State target_stop_state = STOPPED);


/// @brief Deletes the current listener
///
/// This method will delete the current listener and returns the manager
Expand Down
18 changes: 16 additions & 2 deletions src/bin/d2/tests/d2_process_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ class D2ProcessTest : public D2Process, public ConfigParseTest {
const D2QueueMgrPtr& queue_mgr = getD2QueueMgr();

// If queue manager isn't in the RUNNING state, return failure.
if (D2QueueMgr::RUNNING != queue_mgr->getMgrState()) {
if (D2QueueMgr::RUNNING != queue_mgr->getMgrState()) {
return (::testing::AssertionFailure(::testing::Message() <<
"queue manager did not start"));
"queue manager did not start"));
}

// Good to go.
Expand Down Expand Up @@ -578,6 +578,13 @@ TEST_F(D2ProcessTest, normalShutdown) {
time_duration elapsed = stop - start;
EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
elapsed.total_milliseconds() <= 2200);

timer.cancel();
getIOService()->restart();
try {
getIOService()->poll();
} catch (...) {
}
}

/// @brief Verifies that an "uncaught" exception thrown during event loop
Expand All @@ -602,6 +609,13 @@ TEST_F(D2ProcessTest, fatalErrorShutdown) {
time_duration elapsed = stop - start;
EXPECT_TRUE(elapsed.total_milliseconds() >= 1900 &&
elapsed.total_milliseconds() <= 2200);

timer.cancel();
getIOService()->restart();
try {
getIOService()->poll();
} catch (...) {
}
}

/// @brief Used to permit visual inspection of logs to ensure
Expand Down
9 changes: 9 additions & 0 deletions src/bin/d2/tests/d2_queue_mgr_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,15 @@ class QueueMgrUDPTest : public virtual ::testing::Test, public D2StatTest,
TEST_TIMEOUT);
}

virtual ~QueueMgrUDPTest() {
test_timer_.cancel();
io_service_->restart();
try {
io_service_->poll();
} catch (...) {
}
}

void reset_results() {
sent_ncrs_.clear();
received_ncrs_.clear();
Expand Down
37 changes: 23 additions & 14 deletions src/bin/d2/tests/d2_update_mgr_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class D2UpdateMgrTest : public TimedIO, public ConfigParseTest {
D2UpdateMgrWrapperPtr update_mgr_;
std::vector<NameChangeRequestPtr> canned_ncrs_;
size_t canned_count_;
boost::shared_ptr<FauxServer> server_;

D2UpdateMgrTest() {
queue_mgr_.reset(new D2QueueMgr(io_service_));
Expand All @@ -82,6 +83,14 @@ class D2UpdateMgrTest : public TimedIO, public ConfigParseTest {
}

~D2UpdateMgrTest() {
if (server_) {
server_->stop();
}
io_service_->restart();
try {
io_service_->poll();
} catch (...) {
}
}

/// @brief Creates a list of valid NameChangeRequest.
Expand Down Expand Up @@ -184,11 +193,11 @@ class D2UpdateMgrTest : public TimedIO, public ConfigParseTest {
TransactionList::iterator it = update_mgr_->transactionListBegin();
while (it != update_mgr_->transactionListEnd()) {
if (((*it).second)->isModelWaiting()) {
return true;
return (true);
}
}

return false;
return (false);
}

/// @brief Process events until all requests have been completed.
Expand Down Expand Up @@ -692,8 +701,8 @@ TEST_F(D2UpdateMgrTest, addTransaction) {

// Create a server based on the transaction's current server, and
// start it listening.
FauxServer server(io_service_, *(trans->getCurrentServer()));
server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
server_.reset(new FauxServer(io_service_, *(trans->getCurrentServer())));
server_->receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());

// Run sweep and IO until everything is done.
processAll();
Expand Down Expand Up @@ -749,8 +758,8 @@ TEST_F(D2UpdateMgrTest, removeTransaction) {

// Create a server based on the transaction's current server, and
// start it listening.
FauxServer server(io_service_, *(trans->getCurrentServer()));
server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
server_.reset(new FauxServer(io_service_, *(trans->getCurrentServer())));
server_->receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());

// Run sweep and IO until everything is done.
processAll();
Expand Down Expand Up @@ -797,8 +806,8 @@ TEST_F(D2UpdateMgrTest, errorTransaction) {
ASSERT_TRUE(trans->getCurrentServer());

// Create a server and start it listening.
FauxServer server(io_service_, *(trans->getCurrentServer()));
server.receive(FauxServer::CORRUPT_RESP);
server_.reset(new FauxServer(io_service_, *(trans->getCurrentServer())));
server_->receive(FauxServer::CORRUPT_RESP);

// Run sweep and IO until everything is done.
processAll();
Expand Down Expand Up @@ -836,8 +845,8 @@ TEST_F(D2UpdateMgrTest, multiTransaction) {
// that all of configured servers have the same address.
// and start it listening.
asiolink::IOAddress server_ip("127.0.0.1");
FauxServer server(io_service_, server_ip, 5301);
server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
server_.reset(new FauxServer(io_service_, server_ip, 5301));
server_->receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());

// Run sweep and IO until everything is done.
processAll();
Expand Down Expand Up @@ -908,8 +917,8 @@ TEST_F(D2UpdateMgrTest, simpleAddTransaction) {

// Create a server based on the transaction's current server, and
// start it listening.
FauxServer server(io_service_, *(trans->getCurrentServer()));
server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
server_.reset(new FauxServer(io_service_, *(trans->getCurrentServer())));
server_->receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());

// Run sweep and IO until everything is done.
processAll();
Expand Down Expand Up @@ -966,8 +975,8 @@ TEST_F(D2UpdateMgrTest, simpleRemoveTransaction) {

// Create a server based on the transaction's current server, and
// start it listening.
FauxServer server(io_service_, *(trans->getCurrentServer()));
server.receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());
server_.reset(new FauxServer(io_service_, *(trans->getCurrentServer())));
server_->receive(FauxServer::USE_RCODE, dns::Rcode::NOERROR());

// Run sweep and IO until everything is done.
processAll();
Expand Down
5 changes: 5 additions & 0 deletions src/bin/dhcp4/dhcp4_srv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,11 @@ Dhcpv4Srv::~Dhcpv4Srv() {
}
LOG_ERROR(dhcp4_logger, DHCP4_SRV_UNLOAD_LIBRARIES_ERROR).arg(msg);
}
io_service_->restart();
try {
io_service_->poll();
} catch (...) {
}
}

void
Expand Down
9 changes: 0 additions & 9 deletions src/bin/dhcp4/tests/classify_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -521,7 +521,6 @@ class ClassifyTest : public Dhcpv4SrvTest {
IfaceMgrTestConfig iface_mgr_test_config_;
};


// This test checks that an incoming DISCOVER that does not match any classes
// will get the fixed fields empty.
TEST_F(ClassifyTest, fixedFieldsDiscoverNoClasses) {
Expand All @@ -538,7 +537,6 @@ TEST_F(ClassifyTest, fixedFieldsInformNoClasses) {
testFixedFields(CONFIGS[0], DHCPINFORM, OptionPtr(), "0.0.0.0", "", "");
}


// This test checks that an incoming DISCOVER that does match a class that has
// next-server specified will result in a response that has the next-server set.
TEST_F(ClassifyTest, fixedFieldsDiscoverNextServer) {
Expand All @@ -561,7 +559,6 @@ TEST_F(ClassifyTest, fixedFieldsInformNextServer) {
testFixedFields(CONFIGS[0], DHCPINFORM, pxe, "1.2.3.4", "", "");
}


// This test checks that an incoming DISCOVER that does match a class that has
// server-hostname specified will result in a response that has the sname field set.
TEST_F(ClassifyTest, fixedFieldsDiscoverHostname) {
Expand All @@ -584,7 +581,6 @@ TEST_F(ClassifyTest, fixedFieldsInformHostname) {
testFixedFields(CONFIGS[0], DHCPINFORM, pxe, "0.0.0.0", "deneb", "");
}


// This test checks that an incoming DISCOVER that does match a class that has
// boot-file-name specified will result in a response that has the filename field set.
TEST_F(ClassifyTest, fixedFieldsDiscoverFile1) {
Expand All @@ -607,7 +603,6 @@ TEST_F(ClassifyTest, fixedFieldsInformFile1) {
testFixedFields(CONFIGS[0], DHCPDISCOVER, pxe, "0.0.0.0", "", "pxelinux.0");
}


// This test checks that an incoming DISCOVER that does match a different class that has
// boot-file-name specified will result in a response that has the filename field set.
TEST_F(ClassifyTest, fixedFieldsDiscoverFile2) {
Expand Down Expand Up @@ -703,7 +698,6 @@ TEST_F(ClassifyTest, fixedFieldsInformNoClasses2) {
testFixedFields(CONFIGS[2], DHCPINFORM, OptionPtr(), "0.0.0.0", "", "");
}


// This test checks that an incoming DISCOVER that does match a class that has
// next-server specified will result in a response that has the next-server set.
TEST_F(ClassifyTest, fixedFieldsDiscoverNextServer2) {
Expand All @@ -726,7 +720,6 @@ TEST_F(ClassifyTest, fixedFieldsInformNextServer2) {
testFixedFields(CONFIGS[2], DHCPINFORM, pxe, "1.2.3.4", "", "");
}


// This test checks that an incoming DISCOVER that does match a class that has
// boot-file-name specified will result in a response that has the filename field set.
TEST_F(ClassifyTest, fixedFieldsDiscoverFile21) {
Expand All @@ -749,7 +742,6 @@ TEST_F(ClassifyTest, fixedFieldsInformFile21) {
testFixedFields(CONFIGS[2], DHCPDISCOVER, pxe, "0.0.0.0", "", "pxelinux.0");
}


// This test checks that an incoming DISCOVER that does match a different class that has
// boot-file-name specified will result in a response that has the filename field set.
TEST_F(ClassifyTest, fixedFieldsDiscoverFile22) {
Expand Down Expand Up @@ -800,7 +792,6 @@ TEST_F(ClassifyTest, fixedFieldsInformNextServer3) {
testFixedFields(CONFIGS[3], DHCPINFORM, pxe, "0.0.0.0", "", "");
}


// Class pxe2 is only-if-required but the subnet requires its evaluation
TEST_F(ClassifyTest, fixedFieldsDiscoverHostname3) {
OptionPtr pxe(new OptionInt<uint16_t>(Option::V4, 93, 0x0007));
Expand Down
7 changes: 5 additions & 2 deletions src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ class CtrlChannelDhcpv4SrvTest : public ::testing::Test {
CommandMgr::instance().setConnectionTimeout(TIMEOUT_DHCP_SERVER_RECEIVE_COMMAND);

server_.reset();
reset();
IfaceMgr::instance().setTestMode(false);
IfaceMgr::instance().setDetectCallback(std::bind(&IfaceMgr::checkDetectIfaces,
IfaceMgr::instancePtr().get(), ph::_1));
Expand Down Expand Up @@ -806,7 +807,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configSet) {
" \"name\": \"kea\", \n"
" \"severity\": \"FATAL\", \n"
" \"output-options\": [{ \n"
" \"output\": \"/dev/null\" \n"
" \"output\": \"/dev/null\", \n"
" \"maxsize\": 0"
" }] \n"
" }] \n";

Expand Down Expand Up @@ -1021,7 +1023,8 @@ TEST_F(CtrlChannelDhcpv4SrvTest, configTest) {
" \"name\": \"kea\", \n"
" \"severity\": \"FATAL\", \n"
" \"output-options\": [{ \n"
" \"output\": \"/dev/null\" \n"
" \"output\": \"/dev/null\", \n"
" \"maxsize\": 0"
" }] \n"
" }] \n";

Expand Down
Loading

0 comments on commit 133ccdf

Please sign in to comment.