[asterisk-commits] rmudgett: branch rmudgett/ao2_red_black r371819 - /team/rmudgett/ao2_red_blac...

[SVN commits to the Asterisk project]

Author: rmudgett
Date: Tue Aug 28 10:45:18 2012
New Revision: 371819

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=371819
Log:
Clean-up some comments and assertions dealing with rbtree node deletion.

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=371819&r1=371818&r2=371819
==============================================================================
--- team/rmudgett/ao2_red_black/main/astobj2.c (original)
+++ team/rmudgett/ao2_red_black/main/astobj2.c Tue Aug 28 10:45:18 2012
@@ -3602,17 +3602,31 @@
 				sibling = child->parent->right;
 				ast_assert(sibling != NULL);
 			}
+			/*
+			 * The sibling is black.  A black node must have two children,
+			 * or one red child, or no children.
+			 */
 			if ((!sibling->left || !sibling->left->is_red)
 				&& (!sibling->right || !sibling->right->is_red)) {
-				/* Case 2: The sibling is black and both of its children are black. */
+				/*
+				 * Case 2: The sibling is black and both of its children are black.
+				 *
+				 * This case handles the two black children or no children
+				 * possibilities of a black node.
+				 */
 				AO2_DEVMODE_STAT(++self->stats.fixup_delete_left[1]);
 				sibling->is_red = 1;
 				child = child->parent;
 			} else {
-				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. */
+				/* At this point the sibling has at least one red child. */
+				if (!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]);
+					ast_assert(sibling->left != NULL);
+					ast_assert(sibling->left->is_red);
 					sibling->left->is_red = 0;
 					sibling->is_red = 1;
 					rb_rotate_right(self, sibling);
@@ -3642,17 +3656,31 @@
 				sibling = child->parent->left;
 				ast_assert(sibling != NULL);
 			}
+			/*
+			 * The sibling is black.  A black node must have two children,
+			 * or one red child, or no children.
+			 */
 			if ((!sibling->right || !sibling->right->is_red)
 				&& (!sibling->left || !sibling->left->is_red)) {
-				/* Case 2: The sibling is black and both of its children are black. */
+				/*
+				 * Case 2: The sibling is black and both of its children are black.
+				 *
+				 * This case handles the two black children or no children
+				 * possibilities of a black node.
+				 */
 				AO2_DEVMODE_STAT(++self->stats.fixup_delete_right[1]);
 				sibling->is_red = 1;
 				child = child->parent;
 			} else {
-				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. */
+				/* At this point the sibling has at least one red child. */
+				if (!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]);
+					ast_assert(sibling->right != NULL);
+					ast_assert(sibling->right->is_red);
 					sibling->right->is_red = 0;
 					sibling->is_red = 1;
 					rb_rotate_left(self, sibling);
@@ -3675,6 +3703,9 @@
 	/*
 	 * Case 2 could leave the child node red and it needs to leave
 	 * with it black.
+	 *
+	 * Case 4 sets the child node to the root which of course must
+	 * be black.
 	 */
 	child->is_red = 0;
 }
@@ -3734,21 +3765,20 @@
 			doomed->parent->left = doomed;
 		}
 
+		/* The doomed node has no left child now. */
+		ast_assert(doomed->left == NULL);
+
 		/*
-		 * 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]);