adjacency-list: fix crash in equals operator
some minor code improvements
refs: #4705
Change-Id: I28805981a88a94c79f699956bb7e62a268d9ba49
diff --git a/.jenkins.d/20-tests.sh b/.jenkins.d/20-tests.sh
index b172b2a..f3208b3 100755
--- a/.jenkins.d/20-tests.sh
+++ b/.jenkins.d/20-tests.sh
@@ -10,6 +10,7 @@
}
ASAN_OPTIONS="color=always"
+ASAN_OPTIONS+=":detect_leaks=false" #4682
ASAN_OPTIONS+=":detect_stack_use_after_return=true"
ASAN_OPTIONS+=":check_initialization_order=true"
ASAN_OPTIONS+=":strict_init_order=true"
diff --git a/src/adjacency-list.cpp b/src/adjacency-list.cpp
index 7fa3e6e..0cf8ff3 100644
--- a/src/adjacency-list.cpp
+++ b/src/adjacency-list.cpp
@@ -73,14 +73,15 @@
bool
AdjacencyList::operator==(const AdjacencyList& adl) const
{
- if (m_adjList.size() != adl.getAdjList().size()) {
+ auto theirList = adl.getAdjList();
+ if (m_adjList.size() != theirList.size()) {
return false;
}
- std::set<Adjacent> ourList(m_adjList.cbegin(), m_adjList.cend());
- std::set<Adjacent> theirList(adl.getAdjList().cbegin(), adl.getAdjList().cend());
+ std::set<Adjacent> ourSet(m_adjList.cbegin(), m_adjList.cend());
+ std::set<Adjacent> theirSet(theirList.cbegin(), theirList.cend());
- return ourList == theirList;
+ return ourSet == theirSet;
}
bool
diff --git a/src/adjacent.cpp b/src/adjacent.cpp
index 183dcef..950a7ae 100644
--- a/src/adjacent.cpp
+++ b/src/adjacent.cpp
@@ -75,8 +75,9 @@
bool
Adjacent::operator<(const Adjacent& adjacent) const
{
- return (m_name < adjacent.getName()) ||
- (m_linkCost < adjacent.getLinkCost());
+ auto linkCost = adjacent.getLinkCost();
+ return std::tie(m_name, m_linkCost) <
+ std::tie(adjacent.getName(), linkCost);
}
std::ostream&
diff --git a/src/lsa-segment-storage.cpp b/src/lsa-segment-storage.cpp
index c384a53..dfe92d7 100644
--- a/src/lsa-segment-storage.cpp
+++ b/src/lsa-segment-storage.cpp
@@ -85,8 +85,8 @@
// No need to store same LSA multiple time
if (m_lsaSegments.find(lsaSegmentsKey) == m_lsaSegments.end()) {
- NLSR_LOG_TRACE("Received LSA segment is new. Storing it in the storage.\n"
- << " LSA data name: " << lsaSegmentName);
+ NLSR_LOG_TRACE("Received LSA segment is new. Storing it in the storage.");
+ NLSR_LOG_TRACE("LSA data segment's name: " << lsaSegmentName);
// Delete the same LSA with lower sequence number
deleteOldLsas(lsaSegmentName);
diff --git a/src/lsdb.cpp b/src/lsdb.cpp
index a38dd55..ce77683 100644
--- a/src/lsdb.cpp
+++ b/src/lsdb.cpp
@@ -232,11 +232,9 @@
// prefixes to the NPT.
m_nlsr.getNamePrefixTable().addEntry(nlsa.getOrigRouter(),
nlsa.getOrigRouter());
- std::list<ndn::Name> nameList = nlsa.getNpl().getNames();
- for (std::list<ndn::Name>::iterator it = nameList.begin(); it != nameList.end();
- it++) {
- if ((*it) != m_nlsr.getConfParameter().getRouterPrefix()) {
- m_nlsr.getNamePrefixTable().addEntry((*it), nlsa.getOrigRouter());
+ for (const auto& name : nlsa.getNpl().getNames()) {
+ if (name != m_nlsr.getConfParameter().getRouterPrefix()) {
+ m_nlsr.getNamePrefixTable().addEntry(name, nlsa.getOrigRouter());
}
}
}
@@ -269,12 +267,11 @@
std::list<ndn::Name> namesToAdd;
std::set_difference(newNames.begin(), newNames.end(), oldNames.begin(), oldNames.end(),
std::inserter(namesToAdd, namesToAdd.begin()));
- for (std::list<ndn::Name>::iterator it = namesToAdd.begin();
- it != namesToAdd.end(); ++it) {
- chkNameLsa->addName((*it));
+ for (const auto& name : namesToAdd) {
+ chkNameLsa->addName(name);
if (nlsa.getOrigRouter() != m_nlsr.getConfParameter().getRouterPrefix()) {
- if ((*it) != m_nlsr.getConfParameter().getRouterPrefix()) {
- m_nlsr.getNamePrefixTable().addEntry((*it), nlsa.getOrigRouter());
+ if (name != m_nlsr.getConfParameter().getRouterPrefix()) {
+ m_nlsr.getNamePrefixTable().addEntry(name, nlsa.getOrigRouter());
}
}
}
@@ -285,13 +282,12 @@
std::list<ndn::Name> namesToRemove;
std::set_difference(oldNames.begin(), oldNames.end(), newNames.begin(), newNames.end(),
std::inserter(namesToRemove, namesToRemove.begin()));
- for (std::list<ndn::Name>::iterator it = namesToRemove.begin();
- it != namesToRemove.end(); ++it) {
- NLSR_LOG_DEBUG("Removing name LSA no longer advertised: " << (*it).toUri());
- chkNameLsa->removeName((*it));
+ for (const auto& name : namesToRemove) {
+ NLSR_LOG_DEBUG("Removing name LSA no longer advertised: " << name);
+ chkNameLsa->removeName(name);
if (nlsa.getOrigRouter() != m_nlsr.getConfParameter().getRouterPrefix()) {
- if ((*it) != m_nlsr.getConfParameter().getRouterPrefix()) {
- m_nlsr.getNamePrefixTable().removeEntry((*it), nlsa.getOrigRouter());
+ if (name != m_nlsr.getConfParameter().getRouterPrefix()) {
+ m_nlsr.getNamePrefixTable().removeEntry(name, nlsa.getOrigRouter());
}
}
}
@@ -369,9 +365,8 @@
Lsdb::writeNameLsdbLog()
{
NLSR_LOG_DEBUG("---------------Name LSDB-------------------");
- for (std::list<NameLsa>::iterator it = m_nameLsdb.begin();
- it != m_nameLsdb.end() ; it++) {
- (*it).writeLog();
+ for (const auto& nlsa : m_nameLsdb) {
+ nlsa.writeLog();
}
}
@@ -570,9 +565,8 @@
Lsdb::writeCorLsdbLog()
{
NLSR_LOG_DEBUG("---------------Cor LSDB-------------------");
- for (std::list<CoordinateLsa>::iterator it = m_corLsdb.begin();
- it != m_corLsdb.end() ; it++) {
- (*it).writeLog();
+ for (const auto& corLsa : m_corLsdb) {
+ corLsa.writeLog();
}
}
@@ -1322,9 +1316,8 @@
Lsdb::writeAdjLsdbLog()
{
NLSR_LOG_DEBUG("---------------Adj LSDB-------------------");
- for (std::list<AdjLsa>::iterator it = m_adjLsdb.begin();
- it != m_adjLsdb.end() ; it++) {
- (*it).writeLog();
+ for (const auto& adj : m_adjLsdb) {
+ adj.writeLog();
}
}
diff --git a/src/route/fib.cpp b/src/route/fib.cpp
index d472920..82978e2 100644
--- a/src/route/fib.cpp
+++ b/src/route/fib.cpp
@@ -56,11 +56,11 @@
}
void
-Fib::addNextHopsToFibEntryAndNfd(FibEntry& entry, NexthopList& hopsToAdd)
+Fib::addNextHopsToFibEntryAndNfd(FibEntry& entry, const NexthopList& hopsToAdd)
{
const ndn::Name& name = entry.getName();
- for (NexthopList::iterator it = hopsToAdd.begin(); it != hopsToAdd.end(); ++it)
+ for (NexthopList::iterator it = hopsToAdd.cbegin(); it != hopsToAdd.cend(); ++it)
{
// Add nexthop to FIB entry
entry.getNexthopList().addNextHop(*it);
@@ -76,7 +76,7 @@
}
void
-Fib::update(const ndn::Name& name, NexthopList& allHops)
+Fib::update(const ndn::Name& name, const NexthopList& allHops)
{
NLSR_LOG_DEBUG("Fib::update called");
@@ -88,7 +88,7 @@
unsigned int nFaces = 0;
// Create a list of next hops to be installed with length == maxFaces
- for (NexthopList::iterator it = allHops.begin(); it != allHops.end() && nFaces < maxFaces;
+ for (NexthopList::iterator it = allHops.cbegin(); it != allHops.cend() && nFaces < maxFaces;
++it, ++nFaces) {
hopsToAdd.addNextHop(*it);
}
@@ -172,7 +172,7 @@
}
unsigned int
-Fib::getNumberOfFacesForName(NexthopList& nextHopList)
+Fib::getNumberOfFacesForName(const NexthopList& nextHopList)
{
uint32_t nNextHops = static_cast<uint32_t>(nextHopList.getNextHops().size());
uint32_t nMaxFaces = m_confParameter.getMaxFacesPerPrefix();
diff --git a/src/route/fib.hpp b/src/route/fib.hpp
index 0c561ab..27e5967 100644
--- a/src/route/fib.hpp
+++ b/src/route/fib.hpp
@@ -89,7 +89,7 @@
* \param allHops A complete list of next-hops to associate with name.
*/
VIRTUAL_WITH_TESTS void
- update(const ndn::Name& name, NexthopList& allHops);
+ update(const ndn::Name& name, const NexthopList& allHops);
/*! \brief Remove all entries from the FIB.
*
@@ -160,10 +160,10 @@
* \sa Fib::removeOldNextHopsFromFibEntryAndNfd
*/
void
- addNextHopsToFibEntryAndNfd(FibEntry& entry, NexthopList& hopsToAdd);
+ addNextHopsToFibEntryAndNfd(FibEntry& entry, const NexthopList& hopsToAdd);
unsigned int
- getNumberOfFacesForName(NexthopList& nextHopList);
+ getNumberOfFacesForName(const NexthopList& nextHopList);
/*! \brief Unregisters a prefix from NFD's RIB.
*
diff --git a/src/route/name-prefix-table-entry.hpp b/src/route/name-prefix-table-entry.hpp
index b345231..828d64a 100644
--- a/src/route/name-prefix-table-entry.hpp
+++ b/src/route/name-prefix-table-entry.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/**
- * Copyright (c) 2014-2017, The University of Memphis,
+ * Copyright (c) 2014-2018, The University of Memphis,
* Regents of the University of California,
* Arizona Board of Regents.
*
@@ -75,8 +75,8 @@
return m_rteList.size();
}
- NexthopList&
- getNexthopList()
+ const NexthopList&
+ getNexthopList() const
{
return m_nexthopList;
}
diff --git a/src/route/name-prefix-table.cpp b/src/route/name-prefix-table.cpp
index dee3630..7fb9941 100644
--- a/src/route/name-prefix-table.cpp
+++ b/src/route/name-prefix-table.cpp
@@ -101,7 +101,7 @@
m_table.push_back(npte);
// If this entry has next hops, we need to inform the FIB
if (npte->getNexthopList().size() > 0) {
- NLSR_LOG_TRACE("Updating FIB with next hops for " << npte);
+ NLSR_LOG_TRACE("Updating FIB with next hops for " << npte->getNamePrefix());
m_nlsr.getFib().update(name, npte->getNexthopList());
}
// The routing table may recalculate and add a routing table entry
@@ -111,7 +111,7 @@
// remain in the Name Prefix Table as a future routing table
// calculation may add next hops.
else {
- NLSR_LOG_TRACE(*npte << " has no next hops; removing from FIB");
+ NLSR_LOG_TRACE(npte->getNamePrefix() << " has no next hops; removing from FIB");
m_nlsr.getFib().remove(name);
}
}
@@ -127,7 +127,7 @@
m_nlsr.getFib().update(name, (*nameItr)->getNexthopList());
}
else {
- NLSR_LOG_TRACE((*nameItr) << " has no next hops; removing from FIB");
+ NLSR_LOG_TRACE(npte->getNamePrefix() << " has no next hops; removing from FIB");
m_nlsr.getFib().remove(name);
}
}
diff --git a/src/route/routing-table-calculator.cpp b/src/route/routing-table-calculator.cpp
index 7dbf821..eeaf4d2 100644
--- a/src/route/routing-table-calculator.cpp
+++ b/src/route/routing-table-calculator.cpp
@@ -615,6 +615,7 @@
if (deltaTheta <= 0.0 || rI <= 0.0 || rJ <= 0.0) {
NLSR_LOG_ERROR("Delta theta or rI or rJ is <= 0");
+ NLSR_LOG_ERROR("Please make sure that no two nodes have the exact same HR coordinates");
return UNKNOWN_DISTANCE;
}
diff --git a/tests/test-nlsr.cpp b/tests/test-nlsr.cpp
index efab71f..c4ca308 100644
--- a/tests/test-nlsr.cpp
+++ b/tests/test-nlsr.cpp
@@ -581,8 +581,6 @@
this->advanceClocks(ndn::time::seconds(fetchInterval));
// Elapse the default timeout on the interest.
this->advanceClocks(defaultTimeout);
- // Plus a little more to let the events process.
- this->advanceClocks(ndn::time::seconds(1));
// Check that we now have two interests
nNameMatches = 0;