Normalize SHAMap visit functions

src/ripple/app/misc/SHAMapStoreImp.cpp

@@ -282,10 +282,10 @@ SHAMapStoreImp::copyNode (std::uint64_t& nodeCount,
     if (! (++nodeCount % checkHealthInterval_))
     {
         if (health())
-            return true;
+            return false;
     }
 
-    return false;
+    return true;
 }
 
 void

src/ripple/shamap/SHAMap.h

@@ -177,10 +177,29 @@ public:
     // traverse functions
     const_iterator upper_bound(uint256 const& id) const;
 
-    void visitNodes (std::function<bool (SHAMapAbstractNode&)> const&) const;
-    void
-        visitLeaves(
-            std::function<void(std::shared_ptr<SHAMapItem const> const&)> const&) const;
+    /**  Visit every node in this SHAMap
+
+         @param function called with every node visited.
+         If function returns false, visitNodes exits.
+    */
+    void visitNodes (std::function<bool (
+        SHAMapAbstractNode&)> const& function) const;
+
+    /**  Visit every node in this SHAMap that
+         is not present in the specified SHAMap
+
+         @param function called with every node visited.
+         If function returns false, visitDifferences exits.
+    */
+    void visitDifferences(SHAMap const* have,
+        std::function<bool (SHAMapAbstractNode&)>) const;
+
+    /**  Visit every leaf node in this SHAMap
+
+         @param function called with every non inner node visited.
+    */
+    void visitLeaves(std::function<void (
+        std::shared_ptr<SHAMapItem const> const&)> const&) const;
 
     // comparison/sync functions
 
@@ -247,8 +266,6 @@ private:
     using DeltaRef = std::pair<std::shared_ptr<SHAMapItem const> const&,
                                std::shared_ptr<SHAMapItem const> const&>;
 
-    void visitDifferences(SHAMap const* have, std::function<bool(SHAMapAbstractNode&)>) const;
-
      // tree node cache operations
     std::shared_ptr<SHAMapAbstractNode> getCache (SHAMapHash const& hash) const;
     void canonicalize (SHAMapHash const& hash, std::shared_ptr<SHAMapAbstractNode>&) const;

src/ripple/shamap/impl/SHAMapSync.cpp

@@ -25,19 +25,21 @@
 namespace ripple {
 
 void
-SHAMap::visitLeaves(
-    std::function<void(std::shared_ptr<SHAMapItem const> const& item)> const& leafFunction) const
+SHAMap::visitLeaves(std::function<void (
+    std::shared_ptr<SHAMapItem const> const& item)> const& leafFunction) const
 {
     visitNodes(
         [&leafFunction](SHAMapAbstractNode& node)
         {
-            if (!node.isInner())
+            if (! node.isInner())
                 leafFunction(static_cast<SHAMapTreeNode&>(node).peekItem());
-            return false;
+            return true;
         });
 }
 
-void SHAMap::visitNodes(std::function<bool (SHAMapAbstractNode&)> const& function) const
+void
+SHAMap::visitNodes(std::function<bool (
+    SHAMapAbstractNode&)> const& function) const
 {
     // Visit every node in a SHAMap
     assert (root_->isValid ());
@@ -64,7 +66,7 @@ void SHAMap::visitNodes(std::function<bool (SHAMapAbstractNode&)> const& functio
             if (!node->isEmptyBranch (pos))
             {
                 std::shared_ptr<SHAMapAbstractNode> child = descendNoStore (node, pos);
-                if (function (*child))
+                if (! function (*child))
                     return;
 
                 if (child->isLeaf ())
@@ -100,6 +102,74 @@ void SHAMap::visitNodes(std::function<bool (SHAMapAbstractNode&)> const& functio
     }
 }
 
+void
+SHAMap::visitDifferences(SHAMap const* have,
+    std::function<bool (SHAMapAbstractNode&)> function) const
+{
+    // Visit every node in this SHAMap that is not present
+    // in the specified SHAMap
+    assert (root_->isValid ());
+
+    if (! root_)
+        return;
+
+    if (root_->getNodeHash ().isZero ())
+        return;
+
+    if (have && (root_->getNodeHash () == have->root_->getNodeHash ()))
+        return;
+
+    if (root_->isLeaf ())
+    {
+        auto leaf = std::static_pointer_cast<SHAMapTreeNode>(root_);
+        if (! have || ! have->hasLeafNode(leaf->peekItem()->key(), leaf->getNodeHash()))
+            function(*root_);
+
+        return;
+    }
+    // contains unexplored non-matching inner node entries
+    using StackEntry = std::pair <SHAMapInnerNode*, SHAMapNodeID>;
+    std::stack <StackEntry, std::vector<StackEntry>> stack;
+
+    stack.push ({static_cast<SHAMapInnerNode*>(root_.get()), SHAMapNodeID{}});
+
+    while (! stack.empty())
+    {
+        SHAMapInnerNode* node;
+        SHAMapNodeID nodeID;
+        std::tie (node, nodeID) = stack.top ();
+        stack.pop ();
+
+        // 1) Add this node to the pack
+        if (! function(*node))
+            return;
+
+        // 2) push non-matching child inner nodes
+        for (int i = 0; i < 16; ++i)
+        {
+            if (! node->isEmptyBranch (i))
+            {
+                auto const& childHash = node->getChildHash (i);
+                SHAMapNodeID childID = nodeID.getChildNodeID (i);
+                auto next = descendThrow(node, i);
+
+                if (next->isInner ())
+                {
+                    if (! have || ! have->hasInnerNode(childID, childHash))
+                        stack.push ({static_cast<SHAMapInnerNode*>(next), childID});
+                }
+                else if (! have || ! have->hasLeafNode(
+                         static_cast<SHAMapTreeNode*>(next)->peekItem()->key(),
+                         childHash))
+                {
+                    if (! function(*next))
+                        return;
+                }
+            }
+        }
+    }
+}
+
 // Starting at the position referred to by the specfied
 // StackEntry, process that node and its first resident
 // children, descending the SHAMap until we complete the
@@ -753,68 +823,4 @@ void SHAMap::getFetchPack (SHAMap const* have, bool includeLeaves, int max,
         });
 }
 
-void
-SHAMap::visitDifferences(SHAMap const* have,
-                         std::function<bool (SHAMapAbstractNode&)> func) const
-{
-    // Visit every node in this SHAMap that is not present
-    // in the specified SHAMap
-
-    if (root_->getNodeHash ().isZero ())
-        return;
-
-    if (have && (root_->getNodeHash () == have->root_->getNodeHash ()))
-        return;
-
-    if (root_->isLeaf ())
-    {
-        auto leaf = std::static_pointer_cast<SHAMapTreeNode>(root_);
-        if (!have || !have->hasLeafNode(leaf->peekItem()->key(), leaf->getNodeHash()))
-            func (*root_);
-
-        return;
-    }
-    // contains unexplored non-matching inner node entries
-    using StackEntry = std::pair <SHAMapInnerNode*, SHAMapNodeID>;
-    std::stack <StackEntry, std::vector<StackEntry>> stack;
-
-    stack.push ({static_cast<SHAMapInnerNode*>(root_.get()), SHAMapNodeID{}});
-
-    while (!stack.empty())
-    {
-        SHAMapInnerNode* node;
-        SHAMapNodeID nodeID;
-        std::tie (node, nodeID) = stack.top ();
-        stack.pop ();
-
-        // 1) Add this node to the pack
-        if (!func (*node))
-            return;
-
-        // 2) push non-matching child inner nodes
-        for (int i = 0; i < 16; ++i)
-        {
-            if (!node->isEmptyBranch (i))
-            {
-                auto const& childHash = node->getChildHash (i);
-                SHAMapNodeID childID = nodeID.getChildNodeID (i);
-                auto next = descendThrow(node, i);
-
-                if (next->isInner ())
-                {
-                    if (!have || !have->hasInnerNode(childID, childHash))
-                        stack.push ({static_cast<SHAMapInnerNode*>(next), childID});
-                }
-                else if (!have || !have->hasLeafNode(
-                         static_cast<SHAMapTreeNode*>(next)->peekItem()->key(),
-                         childHash))
-                {
-                    if (! func (*next))
-                        return;
-                }
-            }
-        }
-    }
-}
-
 } // ripple