From 64a98c529c93fce9cce344d157cf05dccb898b36 Mon Sep 17 00:00:00 2001
From: Andrew Noyes <andrew@weaselab.dev>
Date: Wed, 17 Apr 2024 14:11:26 -0700
Subject: [PATCH] Fix conflict_set.py bug, and add full inner words test

Apparently all bytes were 0
---
 CMakeLists.txt       |  7 ++++---
 SkipList.cpp         |  4 ++++
 conflict_set.py      | 29 ++++++++++++++---------------
 test_conflict_set.py | 23 +++++++++++++++++++++--
 4 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e3ef795..ce04877 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -267,9 +267,10 @@ if(BUILD_TESTING)
       COMMAND ${Python3_EXECUTABLE} ${CMAKE_SOURCE_DIR}/test_conflict_set.py
               list OUTPUT_VARIABLE SCRIPT_TESTS)
     foreach(TEST ${SCRIPT_TESTS})
-      add_test(NAME script_test_${TEST}
-               COMMAND ${Python3_EXECUTABLE}
-                       ${CMAKE_SOURCE_DIR}/test_conflict_set.py test ${TEST})
+      add_test(
+        NAME script_test_${TEST}
+        COMMAND ${Python3_EXECUTABLE} ${CMAKE_SOURCE_DIR}/test_conflict_set.py
+                test ${TEST} --build-dir ${CMAKE_BINARY_DIR})
     endforeach()
   endif()
 
diff --git a/SkipList.cpp b/SkipList.cpp
index 99b002b..2c99555 100644
--- a/SkipList.cpp
+++ b/SkipList.cpp
@@ -613,6 +613,10 @@ struct __attribute__((visibility("hidden"))) ConflictSet::Impl {
     for (int s = stripes - 1; s >= 0; s--) {
       for (int i = 0; i * 2 < ss; ++i) {
         const auto &w = writes[s * stripeSize / 2 + i];
+#if DEBUG_VERBOSE
+        printf("Write begin: %s\n", printable(w.begin).c_str());
+        fflush(stdout);
+#endif
         values[i * 2] = {w.begin.p, size_t(w.begin.len)};
         values[i * 2 + 1] = w.end.len > 0
                                 ? StringRef{w.end.p, size_t(w.end.len)}
diff --git a/conflict_set.py b/conflict_set.py
index 54e40d2..04cfb54 100644
--- a/conflict_set.py
+++ b/conflict_set.py
@@ -28,39 +28,38 @@ class Result(enum.Enum):
 
 
 def write(begin: bytes, end: Optional[bytes] = None) -> WriteRange:
-    b = (ctypes.c_ubyte * len(begin))()
-    b.value = begin
+    b = (ctypes.c_ubyte * len(begin)).from_buffer(bytearray(begin))
+
     if end is None:
         e = (ctypes.c_ubyte * 0)()
-        e.value = b""
     else:
-        e = (ctypes.c_ubyte * len(end))()
-        e.value = end
+        e = (ctypes.c_ubyte * len(end)).from_buffer(bytearray(end))
     return WriteRange(_Key(b, len(b)), _Key(e, len(e)))
 
 
 def read(version: int, begin: bytes, end: Optional[bytes] = None) -> ReadRange:
-    b = (ctypes.c_ubyte * len(begin))()
-    b.value = begin
+    b = (ctypes.c_ubyte * len(begin)).from_buffer(bytearray(begin))
     if end is None:
         e = (ctypes.c_ubyte * 0)()
-        e.value = b""
     else:
-        e = (ctypes.c_ubyte * len(end))()
-        e.value = end
+        e = (ctypes.c_ubyte * len(end)).from_buffer(bytearray(end))
     return ReadRange(_Key(b, len(b)), _Key(e, len(e)), version)
 
 
 class ConflictSet:
-    def __init__(self, version: int = 0, implementation: Optional[str] = None) -> None:
+    def __init__(
+        self,
+        version: int = 0,
+        build_dir: Optional[str] = None,
+        implementation: Optional[str] = None,
+    ) -> None:
         self._lib = None
+        if build_dir is None:
+            build_dir = os.path.dirname(__file__) + "/build"
         if implementation is None:
             implementation = "radix_tree"
         for f in (
-            os.path.dirname(__file__)
-            + "/build/"
-            + implementation
-            + "/libconflict-set.so.0",
+            build_dir + "/" + implementation + "/libconflict-set.so.0",
             os.path.dirname(__file__)
             + "/build/"
             + implementation
diff --git a/test_conflict_set.py b/test_conflict_set.py
index b324316..262911e 100644
--- a/test_conflict_set.py
+++ b/test_conflict_set.py
@@ -1,14 +1,21 @@
 from conflict_set import *
 
 
+build_dir = None
+
+
 class DebugConflictSet:
     """
     Bisimulates the skip list and radix tree conflict sets for testing purposes
     """
 
     def __init__(self, version: int = 0) -> None:
-        self.skip_list = ConflictSet(version, implementation="skip_list")
-        self.radix_tree = ConflictSet(version, implementation="radix_tree")
+        self.skip_list = ConflictSet(
+            version, build_dir=build_dir, implementation="skip_list"
+        )
+        self.radix_tree = ConflictSet(
+            version, build_dir=build_dir, implementation="radix_tree"
+        )
 
     def addWrites(self, version: int, *writes: WriteRange):
         self.skip_list.addWrites(version, *writes)
@@ -49,6 +56,16 @@ def test_conflict_set():
         assert cs.check(read(0, key)) == [Result.TOO_OLD]
 
 
+def test_inner_full_words():
+    with DebugConflictSet() as cs:
+        cs.addWrites(1, write(b"\x3f\x61"), write(b"\x81\x61"))
+        writes = []
+        for i in range(0x40, 0x81):
+            writes.append(write(bytes([i, 0x61])))
+        cs.addWrites(2, *writes)
+        cs.check(read(1, b"\x21", b"\xc2"))
+
+
 if __name__ == "__main__":
     # budget "pytest" for ctest integration without pulling in a dependency. You can of course still use pytest in local development.
     import argparse
@@ -61,6 +78,7 @@ if __name__ == "__main__":
     list_parser = subparsers.add_parser("list")
     test_parser = subparsers.add_parser("test")
     test_parser.add_argument("test")
+    test_parser.add_argument("--build-dir")
 
     args = parser.parse_args()
 
@@ -74,4 +92,5 @@ if __name__ == "__main__":
             )
         )
     elif args.command == "test":
+        build_dir = args.build_dir
         globals()["test_" + args.test]()