table: NameTree::lookup return Entry& instead of shared_ptr

refs #3687

Change-Id: Ie86057337dd36ff2bc6ef3ea0c73fd4ffc4b13d0
diff --git a/daemon/table/fib.cpp b/daemon/table/fib.cpp
index 7cbcad8..4834783 100644
--- a/daemon/table/fib.cpp
+++ b/daemon/table/fib.cpp
@@ -95,7 +95,7 @@
 const Entry&
 Fib::findLongestPrefixMatch(const measurements::Entry& measurementsEntry) const
 {
-  name_tree::Entry* nte = m_nameTree.getEntry(measurementsEntry).get();
+  name_tree::Entry* nte = m_nameTree.getEntry(measurementsEntry);
   BOOST_ASSERT(nte != nullptr);
   return findLongestPrefixMatch(*nte);
 }
@@ -113,15 +113,15 @@
 std::pair<Entry*, bool>
 Fib::insert(const Name& prefix)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.lookup(prefix);
-  Entry* entry = nte->getFibEntry();
+  name_tree::Entry& nte = m_nameTree.lookup(prefix);
+  Entry* entry = nte.getFibEntry();
   if (entry != nullptr) {
     return std::make_pair(entry, false);
   }
 
-  nte->setFibEntry(make_unique<Entry>(prefix));
+  nte.setFibEntry(make_unique<Entry>(prefix));
   ++m_nItems;
-  return std::make_pair(nte->getFibEntry(), true);
+  return std::make_pair(nte.getFibEntry(), true);
 }
 
 void
@@ -148,10 +148,10 @@
 void
 Fib::erase(const Entry& entry)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.getEntry(entry);
+  name_tree::Entry* nte = m_nameTree.getEntry(entry);
   if (nte != nullptr) {
     // don't try to erase s_emptyEntry
-    this->erase(nte.get());
+    this->erase(nte);
   }
 }
 
@@ -161,8 +161,8 @@
   entry.removeNextHop(face);
 
   if (!entry.hasNextHops()) {
-    shared_ptr<name_tree::Entry> nte = m_nameTree.getEntry(entry);
-    this->erase(nte.get(), false);
+    name_tree::Entry* nte = m_nameTree.getEntry(entry);
+    this->erase(nte, false);
   }
 }
 
diff --git a/daemon/table/measurements.cpp b/daemon/table/measurements.cpp
index b2e8637..60011e2 100644
--- a/daemon/table/measurements.cpp
+++ b/daemon/table/measurements.cpp
@@ -59,25 +59,22 @@
 Entry&
 Measurements::get(const Name& name)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.lookup(name);
-  BOOST_ASSERT(nte != nullptr);
-  return this->get(*nte);
+  name_tree::Entry& nte = m_nameTree.lookup(name);
+  return this->get(nte);
 }
 
 Entry&
 Measurements::get(const fib::Entry& fibEntry)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.lookup(fibEntry);
-  BOOST_ASSERT(nte != nullptr);
-  return this->get(*nte);
+  name_tree::Entry& nte = m_nameTree.lookup(fibEntry);
+  return this->get(nte);
 }
 
 Entry&
 Measurements::get(const pit::Entry& pitEntry)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.lookup(pitEntry);
-  BOOST_ASSERT(nte != nullptr);
-  return this->get(*nte);
+  name_tree::Entry& nte = m_nameTree.lookup(pitEntry);
+  return this->get(nte);
 }
 
 Entry*
@@ -87,7 +84,7 @@
     return nullptr;
   }
 
-  shared_ptr<name_tree::Entry> nteChild = m_nameTree.getEntry(child);
+  name_tree::Entry* nteChild = m_nameTree.getEntry(child);
   name_tree::Entry* nte = nteChild->getParent();
   BOOST_ASSERT(nte != nullptr);
   return &this->get(*nte);
@@ -130,7 +127,7 @@
 void
 Measurements::extendLifetime(Entry& entry, const time::nanoseconds& lifetime)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.getEntry(entry);
+  name_tree::Entry* nte = m_nameTree.getEntry(entry);
   BOOST_ASSERT(nte != nullptr);
 
   time::steady_clock::TimePoint expiry = time::steady_clock::now() + lifetime;
@@ -147,11 +144,11 @@
 void
 Measurements::cleanup(Entry& entry)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.getEntry(entry);
+  name_tree::Entry* nte = m_nameTree.getEntry(entry);
   BOOST_ASSERT(nte != nullptr);
 
   nte->setMeasurementsEntry(nullptr);
-  m_nameTree.eraseIfEmpty(nte.get());
+  m_nameTree.eraseIfEmpty(nte);
   --m_nItems;
 }
 
diff --git a/daemon/table/name-tree.cpp b/daemon/table/name-tree.cpp
index fa3b558..90e953e 100644
--- a/daemon/table/name-tree.cpp
+++ b/daemon/table/name-tree.cpp
@@ -40,7 +40,7 @@
 {
 }
 
-shared_ptr<Entry>
+Entry&
 NameTree::lookup(const Name& name)
 {
   NFD_LOG_TRACE("lookup " << name);
@@ -58,13 +58,13 @@
     }
     parent = node->entry.get();
   }
-  return node->entry;
+  return *node->entry;
 }
 
-shared_ptr<Entry>
+Entry&
 NameTree::lookup(const fib::Entry& fibEntry)
 {
-  shared_ptr<Entry> nte = this->getEntry(fibEntry);
+  Entry* nte = this->getEntry(fibEntry);
   if (nte == nullptr) {
     // special case: Fib::s_emptyEntry is unattached
     BOOST_ASSERT(fibEntry.getPrefix().empty());
@@ -72,13 +72,13 @@
   }
 
   BOOST_ASSERT(nte->getFibEntry() == &fibEntry);
-  return nte;
+  return *nte;
 }
 
-shared_ptr<Entry>
+Entry&
 NameTree::lookup(const pit::Entry& pitEntry)
 {
-  shared_ptr<Entry> nte = this->getEntry(pitEntry);
+  Entry* nte = this->getEntry(pitEntry);
   BOOST_ASSERT(nte != nullptr);
 
   BOOST_ASSERT(std::count_if(nte->getPitEntries().begin(), nte->getPitEntries().end(),
@@ -87,7 +87,7 @@
     }) == 1);
 
   if (nte->getName().size() == pitEntry.getName().size()) {
-    return nte;
+    return *nte;
   }
 
   // special case: PIT entry whose Interest name ends with an implicit digest
@@ -97,24 +97,24 @@
   return this->lookup(pitEntry.getName());
 }
 
-shared_ptr<Entry>
+Entry&
 NameTree::lookup(const measurements::Entry& measurementsEntry)
 {
-  shared_ptr<Entry> nte = this->getEntry(measurementsEntry);
+  Entry* nte = this->getEntry(measurementsEntry);
   BOOST_ASSERT(nte != nullptr);
 
   BOOST_ASSERT(nte->getMeasurementsEntry() == &measurementsEntry);
-  return nte;
+  return *nte;
 }
 
-shared_ptr<Entry>
+Entry&
 NameTree::lookup(const strategy_choice::Entry& strategyChoiceEntry)
 {
-  shared_ptr<Entry> nte = this->getEntry(strategyChoiceEntry);
+  Entry* nte = this->getEntry(strategyChoiceEntry);
   BOOST_ASSERT(nte != nullptr);
 
   BOOST_ASSERT(nte->getStrategyChoiceEntry() == &strategyChoiceEntry);
-  return nte;
+  return *nte;
 }
 
 size_t
@@ -178,7 +178,7 @@
 Entry*
 NameTree::findLongestPrefixMatch(const pit::Entry& pitEntry, const EntrySelector& entrySelector) const
 {
-  const Entry* nte = this->getEntry(pitEntry).get();
+  const Entry* nte = this->getEntry(pitEntry);
   BOOST_ASSERT(nte != nullptr);
 
   if (nte->getName().size() < pitEntry.getName().size()) {
@@ -204,7 +204,7 @@
   // For trie-like design, it could be more efficient by walking down the
   // trie from the root node.
 
-  Entry* entry = findLongestPrefixMatch(name, entrySelector);
+  Entry* entry = this->findLongestPrefixMatch(name, entrySelector);
   return {Iterator(make_shared<PrefixMatchImpl>(*this, entrySelector), entry), end()};
 }
 
diff --git a/daemon/table/name-tree.hpp b/daemon/table/name-tree.hpp
index b534212..9c21f7e 100644
--- a/daemon/table/name-tree.hpp
+++ b/daemon/table/name-tree.hpp
@@ -62,10 +62,10 @@
    *          or nullptr if the table entry is detached
    */
   template<typename ENTRY>
-  shared_ptr<Entry>
+  Entry*
   getEntry(const ENTRY& tableEntry) const
   {
-    return tableEntry.m_nameTreeEntry.lock();
+    return tableEntry.m_nameTreeEntry.lock().get();
   }
 
 public: // mutation
@@ -75,35 +75,35 @@
    *  \post an entry with \p name and all ancestors are created
    *  \note Existing iterators are unaffected.
    */
-  shared_ptr<Entry>
+  Entry&
   lookup(const Name& name);
 
   /** \brief equivalent to .lookup(fibEntry.getPrefix())
    *  \param fibEntry a FIB entry attached to this name tree, or Fib::s_emptyEntry
    *  \note This overload is more efficient than .lookup(const Name&) in common cases.
    */
-  shared_ptr<Entry>
+  Entry&
   lookup(const fib::Entry& fibEntry);
 
   /** \brief equivalent to .lookup(pitEntry.getName()).
    *  \param pitEntry a PIT entry attached to this name tree
    *  \note This overload is more efficient than .lookup(const Name&) in common cases.
    */
-  shared_ptr<Entry>
+  Entry&
   lookup(const pit::Entry& pitEntry);
 
   /** \brief equivalent to .lookup(measurementsEntry.getName())
    *  \param measurementsEntry a Measurements entry attached to this name tree
    *  \note This overload is more efficient than .lookup(const Name&) in common cases.
    */
-  shared_ptr<Entry>
+  Entry&
   lookup(const measurements::Entry& measurementsEntry);
 
   /** \brief equivalent to .lookup(strategyChoiceEntry.getName())
    *  \param strategyChoiceEntry a StrategyChoice entry attached to this name tree
    *  \note This overload is more efficient than .lookup(const Name&) in common cases.
    */
-  shared_ptr<Entry>
+  Entry&
   lookup(const strategy_choice::Entry& strategyChoiceEntry);
 
   /** \brief delete the entry if it is empty
diff --git a/daemon/table/pit.cpp b/daemon/table/pit.cpp
index 06dab3e..6c378dc 100644
--- a/daemon/table/pit.cpp
+++ b/daemon/table/pit.cpp
@@ -68,8 +68,7 @@
   // ensure NameTree entry exists
   name_tree::Entry* nte = nullptr;
   if (allowInsert) {
-    nte = m_nameTree.lookup(nteName).get();
-    BOOST_ASSERT(nte != nullptr);
+    nte = &m_nameTree.lookup(nteName);
   }
   else {
     nte = m_nameTree.findExactMatch(nteName);
@@ -132,12 +131,12 @@
 void
 Pit::erase(shared_ptr<pit::Entry> entry, bool canDeleteNte)
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.getEntry(*entry);
+  name_tree::Entry* nte = m_nameTree.getEntry(*entry);
   BOOST_ASSERT(nte != nullptr);
 
   nte->erasePitEntry(entry);
   if (canDeleteNte) {
-    m_nameTree.eraseIfEmpty(nte.get());
+    m_nameTree.eraseIfEmpty(nte);
   }
   --m_nItems;
 }
diff --git a/daemon/table/strategy-choice.cpp b/daemon/table/strategy-choice.cpp
index 59ac463..76c19ab 100644
--- a/daemon/table/strategy-choice.cpp
+++ b/daemon/table/strategy-choice.cpp
@@ -103,8 +103,8 @@
     return false;
   }
 
-  shared_ptr<name_tree::Entry> nte = m_nameTree.lookup(prefix);
-  Entry* entry = nte->getStrategyChoiceEntry();
+  name_tree::Entry& nte = m_nameTree.lookup(prefix);
+  Entry* entry = nte.getStrategyChoiceEntry();
   Strategy* oldStrategy = nullptr;
   if (entry != nullptr) {
     if (entry->getStrategy().getName() == strategy->getName()) {
@@ -120,7 +120,7 @@
     oldStrategy = &this->findEffectiveStrategy(prefix);
     auto newEntry = make_unique<Entry>(prefix);
     entry = newEntry.get();
-    nte->setStrategyChoiceEntry(std::move(newEntry));
+    nte.setStrategyChoiceEntry(std::move(newEntry));
     ++m_nItems;
     NFD_LOG_TRACE("insert(" << prefix << ") new entry " << strategy->getName());
   }
@@ -202,7 +202,7 @@
 Strategy&
 StrategyChoice::findEffectiveStrategy(const measurements::Entry& measurementsEntry) const
 {
-  shared_ptr<name_tree::Entry> nte = m_nameTree.getEntry(measurementsEntry);
+  const name_tree::Entry* nte = m_nameTree.getEntry(measurementsEntry);
   BOOST_ASSERT(nte != nullptr);
   return this->findEffectiveStrategy(*nte);
 }
@@ -220,8 +220,8 @@
 
   // don't use .insert here, because it will invoke findEffectiveStrategy
   // which expects an existing root entry
-  shared_ptr<name_tree::Entry> nte = m_nameTree.lookup(Name());
-  nte->setStrategyChoiceEntry(std::move(entry));
+  name_tree::Entry& nte = m_nameTree.lookup(Name());
+  nte.setStrategyChoiceEntry(std::move(entry));
   ++m_nItems;
   NFD_LOG_INFO("setDefaultStrategy " << instance->getName());
 }
@@ -258,7 +258,7 @@
 
   // reset StrategyInfo on a portion of NameTree,
   // where entry's effective strategy is covered by the changing StrategyChoice entry
-  const name_tree::Entry* rootNte = m_nameTree.getEntry(entry).get();
+  const name_tree::Entry* rootNte = m_nameTree.getEntry(entry);
   BOOST_ASSERT(rootNte != nullptr);
   auto&& ntChanged = m_nameTree.partialEnumerate(entry.getPrefix(),
     [&rootNte] (const name_tree::Entry& nte) -> std::pair<bool, bool> {
diff --git a/tests/daemon/table/name-tree.t.cpp b/tests/daemon/table/name-tree.t.cpp
index b5d37d0..bf46022 100644
--- a/tests/daemon/table/name-tree.t.cpp
+++ b/tests/daemon/table/name-tree.t.cpp
@@ -245,33 +245,35 @@
   BOOST_CHECK_EQUAL(nt.size(), 0);
   BOOST_CHECK_EQUAL(nt.getNBuckets(), nBuckets);
 
+  // lookup
+
   Name nameABC("ndn:/a/b/c");
-  shared_ptr<Entry> npeABC = nt.lookup(nameABC);
+  Entry& npeABC = nt.lookup(nameABC);
   BOOST_CHECK_EQUAL(nt.size(), 4);
 
   Name nameABD("/a/b/d");
-  shared_ptr<Entry> npeABD = nt.lookup(nameABD);
+  Entry& npeABD = nt.lookup(nameABD);
   BOOST_CHECK_EQUAL(nt.size(), 5);
 
   Name nameAE("/a/e/");
-  shared_ptr<Entry> npeAE = nt.lookup(nameAE);
+  Entry& npeAE = nt.lookup(nameAE);
   BOOST_CHECK_EQUAL(nt.size(), 6);
 
   Name nameF("/f");
-  shared_ptr<Entry> npeF = nt.lookup(nameF);
+  Entry& npeF = nt.lookup(nameF);
   BOOST_CHECK_EQUAL(nt.size(), 7);
 
-  // validate lookup() and findExactMatch()
+  // getParent and findExactMatch
 
   Name nameAB("/a/b");
-  BOOST_CHECK_EQUAL(npeABC->getParent(), nt.findExactMatch(nameAB));
-  BOOST_CHECK_EQUAL(npeABD->getParent(), nt.findExactMatch(nameAB));
+  BOOST_CHECK_EQUAL(npeABC.getParent(), nt.findExactMatch(nameAB));
+  BOOST_CHECK_EQUAL(npeABD.getParent(), nt.findExactMatch(nameAB));
 
-  Name nameA ("/a");
-  BOOST_CHECK_EQUAL(npeAE->getParent(), nt.findExactMatch(nameA));
+  Name nameA("/a");
+  BOOST_CHECK_EQUAL(npeAE.getParent(), nt.findExactMatch(nameA));
 
-  Name nameRoot ("/");
-  BOOST_CHECK_EQUAL(npeF->getParent(), nt.findExactMatch(nameRoot));
+  Name nameRoot("/");
+  BOOST_CHECK_EQUAL(npeF.getParent(), nt.findExactMatch(nameRoot));
   BOOST_CHECK_EQUAL(nt.size(), 7);
 
   Name name0("/does/not/exist");
@@ -279,7 +281,8 @@
   BOOST_CHECK(npe0 == nullptr);
 
 
-  // Longest Prefix Matching
+  // findLongestPrefixMatch
+
   Entry* temp = nullptr;
   Name nameABCLPM("/a/b/c/def/asdf/nlf");
   temp = nt.findLongestPrefixMatch(nameABCLPM);
@@ -639,11 +642,11 @@
 
   Name prefix("/a/b/c/d/e/f/g/h"); // requires 9 buckets
 
-  shared_ptr<Entry> entry = nameTree.lookup(prefix);
+  Entry& entry = nameTree.lookup(prefix);
   BOOST_CHECK_EQUAL(nameTree.size(), 9);
   BOOST_CHECK_EQUAL(nameTree.getNBuckets(), 32);
 
-  nameTree.eraseIfEmpty(entry.get());
+  nameTree.eraseIfEmpty(&entry);
   BOOST_CHECK_EQUAL(nameTree.size(), 0);
   BOOST_CHECK_EQUAL(nameTree.getNBuckets(), 16);
 }