Redblack: fixed special deleting cases and missing parent pointer initialization

lib/redblack.h

@@ -246,7 +246,7 @@
 
 #define REDBLACK_INSERT(type, name, key, compare, root, what) do { \
   type **where = &(root); \
-  what->name[1] = what->name[2] = NULL; \
+  what->name[0] = what->name[1] = what->name[2] = NULL; \
   REDBLACK_FIND_POINTER(name, key, compare, root, key(what), where) \
     REDBLACK_PARENT_POINTER(name, what) = REDBLACK_PTR_RED(type, *where); \
   ASSERT(!*where); \
@@ -326,13 +326,28 @@
     cr = REDBLACK_RIGHT_CHILD(name, what); \
   } \
   type *p = REDBLACK_PARENT(type, name, what); \
+  if (!p) { \
+    /* Deleting root in almost empty tree */ \
+    if (cl) { \
+      root = cl; \
+      REDBLACK_PARENT_POINTER(name, cl) = NULL; \
+      break; \
+    } \
+    if (cr) { \
+      root = cr; \
+      REDBLACK_PARENT_POINTER(name, cr) = NULL; \
+      break; \
+    } \
+    root = NULL; \
+    break; \
+  } \
+  int ps = REDBLACK_PARENT_SIDE(name, p, what); \
   if (REDBLACK_NODE_COLOR(name, what) == REDBLACK_RED) { \
-    ASSERT((cl == NULL) && (cl == NULL)); \
-    REDBLACK_CHILD(name, p, REDBLACK_PARENT_SIDE(name, p, what)) = NULL; \
+    ASSERT((cl == NULL) && (cr == NULL)); \
+    REDBLACK_CHILD(name, p, ps) = NULL; \
     break; \
   } \
   /* The only child now must be red */ \
-  int ps = REDBLACK_PARENT_SIDE(name, p, what); \
   if (cl) { \
     REDBLACK_CONNECT_NODE_SET_COLOR(type, name, p, ps, cl, REDBLACK_BLACK); \
     break; \
@@ -344,7 +359,8 @@
   type *drop = what; \
   while (1) { /* Invariant: what is black */ \
     if (what == root) { /* Case 1 */ \
-      root = NULL; \
+      if (drop) \
+	root = NULL; \
       break; \
     } \
     type *p = REDBLACK_PARENT(type, name, what); \
@@ -355,6 +371,7 @@
       REDBLACK_ROTATE(type, name, root, p, ws); \
       REDBLACK_SET_COLOR(type, name, p, REDBLACK_RED); \
       REDBLACK_SET_COLOR(type, name, s, REDBLACK_BLACK); \
+      /* Now what's sibling is also black, let's try once again */ \
       continue; \
     } \
     if (drop) drop = REDBLACK_CHILD(name, p, ws) = NULL; \

lib/redblack_test.c

@@ -10,9 +10,6 @@
 #include "test/birdtest.h"
 #include "lib/redblack.h"
 
-#define N 4096
-#define MUL 16
-
 struct rb_test {
   REDBLACK_NODE(struct rb_test, rb_);
   uint value;
@@ -70,7 +67,8 @@ static void rb_dump(struct rb_test *root) {
 
 #define RB_INSERT(root, val) do { \
   struct rb_test *new = xmalloc(sizeof(struct rb_test)); \
-  *new = (struct rb_test) { .value = val }; \
+  memset(new, 42, sizeof(struct rb_test)); \
+  new->value = val; \
   REDBLACK_INSERT(struct rb_test, rb_, RBT_KEY, RBT_COMPARE, root, new); \
 } while (0)
 
@@ -79,16 +77,70 @@ static void rb_dump(struct rb_test *root) {
   REDBLACK_DELETE(struct rb_test, rb_, root, old); \
 } while (0)
 
+struct rb_test_args {
+  uint N, MUL;
+};
+
+#define RB_INSERT_SIMPLE(root, val) do { \
+  RB_CHECK(root, bits, total); \
+  if (bt_verbose >= BT_VERBOSE_ABSOLUTELY_ALL) \
+    rb_dump(root); \
+  SIT(i); \
+  total++; \
+  RB_INSERT(root, i); \
+} while (0)
+
+#define RB_DELETE_SIMPLE(root, val) do { \
+  RB_CHECK(root, bits, total); \
+  if (bt_verbose >= BT_VERBOSE_ABSOLUTELY_ALL) \
+    rb_dump(root); \
+  CIT(i); \
+  total--; \
+  RB_DELETE(root, i); \
+} while (0)
+
 static int
-rb_insert(void)
+rb_insert(const void *_args)
 {
+  const struct rb_test_args *args = _args;
+  uint N = args->N;
+  uint MUL = args->MUL;
+
   struct rb_test *root = NULL;
 
 #define BIT(x) ((bits[(x) / 64] >> ((x) % 64)) & 1)
 #define SIT(x) (bits[(x) / 64] |= (1ULL << ((x) % 64)))
 #define CIT(x) (bits[(x) / 64] &= ~(1ULL << ((x) % 64)))
   uint total = 0;
-  u64 bits[N / 64] = {};
+  u64 *bits = alloca(sizeof(u64) * ((N+63) / 64));
+  memset(bits, 0, sizeof(u64) * ((N+63) / 64));
+
+  bt_debug("Inserting full tree");
+  for (uint i=0; i<N; i++)
+    RB_INSERT_SIMPLE(root, i);
+
+  bt_debug("Deleting full tree");
+  for (uint i=0; i<N; i++)
+    RB_DELETE_SIMPLE(root, i);
+
+  bt_debug("Inserting full tree backwards");
+  for (uint i=0; i<N; i++)
+    RB_INSERT_SIMPLE(root, N-i-1);
+
+  bt_debug("Deleting full tree");
+  for (uint i=0; i<N; i++)
+    RB_DELETE_SIMPLE(root, i);
+
+  bt_debug("Inserting full tree");
+  for (uint i=0; i<N; i++)
+    RB_INSERT_SIMPLE(root, i);
+
+  bt_debug("Deleting full tree backwards");
+  for (uint i=0; i<N; i++)
+    RB_DELETE_SIMPLE(root, i);
+
+  bt_debug("Running random test");
+
   for (uint i=0; i<N * MUL; i++) {
     RB_CHECK(root, bits, total);
     if (bt_verbose >= BT_VERBOSE_ABSOLUTELY_ALL)
@@ -111,13 +163,30 @@ rb_insert(void)
       RB_INSERT(root, tv);
     }
   }
+
+  for (uint i=0; i<N; i++) {
+    if (!BIT(i))
+      continue;
+
+    RB_DELETE_SIMPLE(root, i);
+  }
  
   return 1;  
 }
 
+#define RUNTEST(n, mul) do { \
+  const struct rb_test_args rbta = { .N = n, .MUL = mul }; \
+  bt_test_suite_arg_extra(rb_insert, &rbta, BT_FORKING, 30, "redblack insertion test: N=%u, MUL=%u", n, mul); \
+} while (0)
+
 int
 main(int argc, char **argv)
 {
   bt_init(argc, argv);
-  bt_test_suite_extra(rb_insert, BT_FORKING, 30, "redblack insertion test");
+  RUNTEST(3, 31);
+  RUNTEST(7, 17);
+  RUNTEST(127, 11);
+  RUNTEST(8191, 3);
+
+  return bt_exit_value();
 }