From 494e836aae6277a225116a8c40361fe3596ab923 Mon Sep 17 00:00:00 2001
From: Weaselbot <weaselbot@weaselab.dev>
Date: Mon, 13 Jul 2026 16:28:47 -0400
Subject: [PATCH] Address review: handle empty keys explicitly in operator<

Per review feedback on the i > 0 memcmp guard: handle empty keys up
front instead, so memcmp only runs when both keys are non-empty (and
therefore have valid data pointers). An empty key is a prefix of every
key, so it sorts before any non-empty key; when both are empty, fall
back to the extra ordering. This makes the empty-vs-non-empty case
explicit rather than relying on the length check after a defaulted c = 0.
---
 SkipList.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/SkipList.cpp b/SkipList.cpp
index 5dee515..e70a60c 100644
--- a/SkipList.cpp
+++ b/SkipList.cpp
@@ -99,8 +99,16 @@ force_inline bool getCharacter(const KeyInfo &ki, int character,
 }
 
 bool operator<(const KeyInfo &lhs, const KeyInfo &rhs) {
+  // An empty key is a prefix of every key, so it sorts before any non-empty
+  // key. Handle empty keys up front: an empty std::span may have
+  // data() == nullptr, which must never be passed to memcmp.
+  if (lhs.key.size() == 0 || rhs.key.size() == 0) {
+    if (lhs.key.size() != rhs.key.size())
+      return lhs.key.size() < rhs.key.size();
+    return extra_ordering(lhs) < extra_ordering(rhs);
+  }
   int i = std::min(lhs.key.size(), rhs.key.size());
-  int c = i > 0 ? memcmp(lhs.key.data(), rhs.key.data(), i) : 0;
+  int c = memcmp(lhs.key.data(), rhs.key.data(), i);
   if (c != 0)
     return c < 0;