[svn-commits] rmudgett: branch rmudgett/ao2_red_black r371818 - /team/rmudgett/ao2_red_blac...

[SVN commits to the Digium repositories]

Author: rmudgett
Date: Tue Aug 28 02:20:45 2012
New Revision: 371818

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=371818
Log:
Fix rbtree deletion.  Added debug code.

Modified:
    team/rmudgett/ao2_red_black/main/astobj2.c

Modified: team/rmudgett/ao2_red_black/main/astobj2.c
URL: http://svnview.digium.com/svn/asterisk/team/rmudgett/ao2_red_black/main/astobj2.c?view=diff&rev=371818&r1=371817&r2=371818
==============================================================================
--- team/rmudgett/ao2_red_black/main/astobj2.c (original)
+++ team/rmudgett/ao2_red_black/main/astobj2.c Tue Aug 28 02:20:45 2012
@@ -1039,6 +1039,9 @@
 	res = 0;
 	node = self->v_table->new_node(self, obj_new, tag, file, line, func);
 	if (node) {
+		if (ao2_container_check(self, OBJ_NOLOCK)) {/* BUGBUG */
+			ast_log(LOG_NOTICE, "BUGBUG integrity failed before insert.\n");
+		}
 		/* Insert the new node. */
 		switch (self->v_table->insert(self, node)) {
 		case AO2_CONTAINER_INSERT_NODE_INSERTED:
@@ -1063,6 +1066,9 @@
 		case AO2_CONTAINER_INSERT_NODE_REJECTED:
 			__ao2_ref(node, -1);
 			break;
+		}
+		if (ao2_container_check(self, OBJ_NOLOCK)) {/* BUGBUG */
+			ast_log(LOG_NOTICE, "BUGBUG integrity failed after insert.\n");
 		}
 	}
 
@@ -3482,6 +3488,30 @@
 	child->right = node;
 }
 
+#if 0
+/*!
+ * \internal
+ * \brief Dump the tree structure.
+ * \since 12.0.0
+ *
+ * \param self Container to operate upon.
+ * \param header What is being dumped.
+ *
+ * \return Nothing
+ */
+static void rb_dump(struct ao2_container_rbtree *self, const char *header)
+{
+	struct rbtree_node *node;
+
+	printf("%s\n", header);
+	printf("%16s, %16s, %16s, %16s, %c, %16s\n", "Node", "Parent", "Left", "Right", 'T', "Obj");
+	for (node = self->root; node; node = rb_node_pre(node)) {
+		printf("%16p, %16p, %16p, %16p, %c, %16p\n",
+			node, node->parent, node->left, node->right, node->is_red ? 'R' : 'B', node->common.obj);
+	}
+}
+#endif
+
 /*!
  * \internal
  * \brief Create an empty copy of this container.
@@ -3579,12 +3609,11 @@
 				sibling->is_red = 1;
 				child = child->parent;
 			} else {
-				if (!sibling->right || !sibling->right->is_red) {
+				if ((sibling->left && sibling->left->is_red)
+					&& (!sibling->right || !sibling->right->is_red)) {
 					/* Case 3: The sibling is black, its left child is red, and its right child is black. */
 					AO2_DEVMODE_STAT(++self->stats.fixup_delete_left[2]);
-					if (sibling->left) {
-						sibling->left->is_red = 0;
-					}
+					sibling->left->is_red = 0;
 					sibling->is_red = 1;
 					rb_rotate_right(self, sibling);
 					sibling = child->parent->right;
@@ -3620,12 +3649,11 @@
 				sibling->is_red = 1;
 				child = child->parent;
 			} else {
-				if (!sibling->left || !sibling->left->is_red) {
+				if ((sibling->right && sibling->right->is_red)
+					&& (!sibling->left || !sibling->left->is_red)) {
 					/* Case 3: The sibling is black, its right child is red, and its left child is black. */
 					AO2_DEVMODE_STAT(++self->stats.fixup_delete_right[2]);
-					if (sibling->right) {
-						sibling->right->is_red = 0;
-					}
+					sibling->right->is_red = 0;
 					sibling->is_red = 1;
 					rb_rotate_left(self, sibling);
 					sibling = child->parent->left;
@@ -3643,6 +3671,11 @@
 			}
 		}
 	}
+
+	/*
+	 * Case 2 could leave the child node red and it needs to leave
+	 * with it black.
+	 */
 	child->is_red = 0;
 }
 
@@ -3692,25 +3725,30 @@
 			next->parent->right = next;
 		}
 		next->left->parent = next;
-		next->right->parent = next;
-
-		/* Link back in the doomed node.  (It has no left child) */
-		if (doomed->parent->left == next) {
-			/* The next node was not the right child of doomed. */
+		if (next->right == next) {
+			/* The next node was the right child of doomed. */
+			next->right = doomed;
+			doomed->parent = next;
+		} else {
+			next->right->parent = next;
 			doomed->parent->left = doomed;
 		}
 
 		/*
+		 * The doomed node has no left child now.
+		 *
 		 * We don't have to link the right child back in with doomed
 		 * since we are going to link it with doomed's parent anyway.
 		 */
 		child = doomed->right;
+		ast_assert(doomed->parent != doomed);/* BUGBUG */
 	} else {
 		/* Doomed has at most one child. */
 		child = doomed->left;
 		if (!child) {
 			child = doomed->right;
 		}
+		ast_assert(doomed->parent != doomed);/* BUGBUG */
 	}
 	if (child) {
 		AO2_DEVMODE_STAT(++self->stats.delete_children[1]);
@@ -3793,7 +3831,17 @@
 		my_container = (struct ao2_container_rbtree *) doomed->common.my_container;
 		adjust_lock(my_container, AO2_LOCK_REQ_WRLOCK, 1);
 
+		if (!my_container->common.destroying
+			&& ao2_container_check(doomed->common.my_container, OBJ_NOLOCK)) {/* BUGBUG */
+			ast_log(LOG_NOTICE, "BUGBUG integrity failed before delete.\n");
+		}
+//		rb_dump(my_container, "Pre deletion");
+//		printf("%16p Deleting node\n", doomed);
 		rb_delete_node(my_container, doomed);
+		if (!my_container->common.destroying
+			&& ao2_container_check(doomed->common.my_container, OBJ_NOLOCK)) {/* BUGBUG */
+			ast_log(LOG_NOTICE, "BUGBUG integrity failed after delete.\n");
+		}
 	}
 
 	/*