Add missing test coverage for fixupMaxVersion for Node256

We're already doing this in checkRangeStartsWith

ConflictSet.cpp

@@ -902,8 +902,7 @@ Node *&getChildExists(Node *self, uint8_t index) {
 }
 
 InternalVersionT maxVersion(Node *n, ConflictSet::Impl *);
-InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax,
-                                    ConflictSet::Impl *);
+InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax);
 
 void setMaxVersion(Node *n, ConflictSet::Impl *, InternalVersionT maxVersion);
 
@@ -2796,38 +2795,13 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
     }
     auto [child, v] = getChildAndMaxVersion(n, remaining[0]);
     if (child == nullptr) {
-      auto c = getChildGeq(n, remaining[0]);
-      if (c != nullptr) {
-        n = c;
-        goto downLeftSpine;
-      } else {
-        n = nextSibling(n);
-        if (n == nullptr) {
-          return true;
-        }
-        goto downLeftSpine;
-      }
+      break;
     }
 
     if (child->partialKeyLen > 0) {
       int cl = std::min<int>(child->partialKeyLen, remaining.size() - 1);
       int i =
           longestCommonPrefix(child->partialKey(), remaining.data() + 1, cl);
-      if (i < cl) {
-        auto c = child->partialKey()[i] <=> remaining[1 + i];
-        if (c > 0) {
-          n = child;
-          goto downLeftSpine;
-        } else {
-          n = nextSibling(child);
-          if (n == nullptr) {
-            return true;
-          }
-          goto downLeftSpine;
-        }
-      }
-      // If the partial key is longer than the common prefix (remaining), we
-      // still need to keep searching
       if (i != child->partialKeyLen) {
         break;
       }
@@ -2843,12 +2817,6 @@ bool checkRangeRead(Node *n, std::span<const uint8_t> begin,
   }
   assert(getSearchPath(arena, n) <=> begin.subspan(0, lcp - remaining.size()) ==
          0);
-  if (0) {
-  downLeftSpine:
-    for (; !n->entryPresent; n = getFirstChildExists(n)) {
-    }
-    return n->entry.rangeVersion <= readVersion;
-  }
 
   const int consumed = lcp - remaining.size();
   assume(consumed >= 0);
@@ -2916,8 +2884,7 @@ checkMaxBetweenExclusiveImpl<true>(Node *n, int begin, int end,
 // Consume the partial key of `self` (which must exist), and update `self` and
 // `key` such that `self` is along the search path of `key`
 void consumePartialKey(Node *&self, std::span<const uint8_t> &key,
-                       InternalVersionT writeVersion, WriteContext *tls,
-                       ConflictSet::Impl *impl) {
+                       InternalVersionT writeVersion, WriteContext *tls) {
   assert(self->partialKeyLen > 0);
   // Handle an existing partial key
   int commonLen = std::min<int>(self->partialKeyLen, key.size());
@@ -2925,8 +2892,9 @@ void consumePartialKey(Node *&self, std::span<const uint8_t> &key,
       longestCommonPrefix(self->partialKey(), key.data(), commonLen);
   if (partialKeyIndex < self->partialKeyLen) {
     auto *old = self;
-    InternalVersionT oldMaxVersion =
-        exchangeMaxVersion(old, writeVersion, impl);
+    // Since root cannot have a partial key
+    assert(old->parent != nullptr);
+    InternalVersionT oldMaxVersion = exchangeMaxVersion(old, writeVersion);
 
     // *self will have one child (old)
     auto *newSelf = tls->allocate<Node3>(partialKeyIndex);
@@ -2968,7 +2936,7 @@ Node *insert(Node **self, std::span<const uint8_t> key,
              ConflictSet::Impl *impl) {
 
   if ((*self)->partialKeyLen > 0) {
-    consumePartialKey(*self, key, writeVersion, tls, impl);
+    consumePartialKey(*self, key, writeVersion, tls);
   }
   assert(maxVersion(*self, impl) <= writeVersion);
   setMaxVersion(*self, impl, writeVersion);
@@ -2996,7 +2964,7 @@ Node *insert(Node **self, std::span<const uint8_t> key,
     key = key.subspan(1, key.size() - 1);
 
     if ((*self)->partialKeyLen > 0) {
-      consumePartialKey(*self, key, writeVersion, tls, impl);
+      consumePartialKey(*self, key, writeVersion, tls);
       assert(maxVersion(*self, impl) <= writeVersion);
       setMaxVersion(*self, impl, writeVersion);
     }
@@ -3049,13 +3017,11 @@ void addPointWrite(Node *&root, std::span<const uint8_t> key,
   }
 }
 
+// Precondition: `node->entryPresent`
 void fixupMaxVersion(Node *node, ConflictSet::Impl *impl, WriteContext *tls) {
   InternalVersionT max;
-  if (node->entryPresent) {
-    max = std::max(node->entry.pointVersion, tls->zero);
-  } else {
-    max = tls->zero;
-  }
+  assert(node->entryPresent);
+  max = std::max(node->entry.pointVersion, tls->zero);
   switch (node->getType()) {
   case Type_Node0:
     break;
@@ -3596,13 +3562,11 @@ InternalVersionT maxVersion(Node *n, ConflictSet::Impl *impl) {
   }
 }
 
-InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax,
-                                    ConflictSet::Impl *impl) {
+// Precondition `n` is not the root
+InternalVersionT exchangeMaxVersion(Node *n, InternalVersionT newMax) {
   int index = n->parentsIndex;
   n = n->parent;
-  if (n == nullptr) {
-    return std::exchange(impl->rootMaxVersion, newMax);
-  }
+  assert(n != nullptr);
   switch (n->getType()) {
   case Type_Node0:           // GCOVR_EXCL_LINE
     __builtin_unreachable(); // GCOVR_EXCL_LINE
@@ -4141,6 +4105,10 @@ checkMaxVersion(Node *root, Node *node, InternalVersionT oldestVersion,
                                        ConflictSet::Impl *impl) {
   bool success = true;
 
+  if (node->partialKeyLen > 0) {
+    fprintf(stderr, "Root cannot have a partial key");
+    success = false;
+  }
   checkParentPointers(node, success);
   checkMaxVersion(node, node, oldestVersion, success, impl);
   checkEntriesExist(node, success);

corpus/0f5c5f28c31e44ee0ec085fec09adf0981065771

@@ -1,5 +0,0 @@
-
-<EFBFBD><EFBFBD>
-
-
-2

corpus/12bdd00fd4038756cbcf8ecdad1b0cd862603cd8

@@ -1 +0,0 @@
-<EFBFBD>