Clarify the new Red-Black post-order traversal code a bit.

Coverity complained about the for(;;) loop, because it never actually
iterated. It was used just to be able to use "break" to exit it early. I
agree with Coverity, that's a bit confusing, so refactor the code to
use if-else instead.

While we're at it, use a local variable to hold the "current" node. That's
shorter and clearer than referring to "iter->last_visited" all the time.
This commit is contained in:
Heikki Linnakangas 2016-09-04 15:02:06 +03:00
parent 6591f4226c
commit e21db14b8a
1 changed files with 24 additions and 22 deletions

View File

@ -781,27 +781,30 @@ static RBNode *
rb_inverted_iterator(RBTreeIterator *iter) rb_inverted_iterator(RBTreeIterator *iter)
{ {
RBNode *came_from; RBNode *came_from;
RBNode *current;
current = iter->last_visited;
loop: loop:
switch ((InvertedWalkNextStep) iter->next_step) switch ((InvertedWalkNextStep) iter->next_step)
{ {
/* First call, begin from root */ /* First call, begin from root */
case NextStepBegin: case NextStepBegin:
iter->last_visited = iter->rb->root; current = iter->rb->root;
iter->next_step = NextStepLeft; iter->next_step = NextStepLeft;
goto loop; goto loop;
case NextStepLeft: case NextStepLeft:
while (iter->last_visited->left != RBNIL) while (current->left != RBNIL)
iter->last_visited = iter->last_visited->left; current = current->left;
iter->next_step = NextStepRight; iter->next_step = NextStepRight;
goto loop; goto loop;
case NextStepRight: case NextStepRight:
if (iter->last_visited->right != RBNIL) if (current->right != RBNIL)
{ {
iter->last_visited = iter->last_visited->right; current = current->right;
iter->next_step = NextStepLeft; iter->next_step = NextStepLeft;
goto loop; goto loop;
} }
@ -810,30 +813,29 @@ loop:
break; break;
case NextStepUp: case NextStepUp:
for (;;) came_from = current;
current = current->parent;
if (current == NULL)
{
iter->is_over = true;
break; /* end of iteration */
}
else if (came_from == current->right)
{
/* return current, then continue to go up */
break;
}
else
{ {
came_from = iter->last_visited;
iter->last_visited = iter->last_visited->parent;
if (iter->last_visited == NULL)
{
iter->is_over = true;
break; /* end of iteration */
}
if (came_from == iter->last_visited->right)
{
/* return current, then continue to go up */
break;
}
/* otherwise we came from the left */ /* otherwise we came from the left */
Assert(came_from == current->left);
iter->next_step = NextStepRight; iter->next_step = NextStepRight;
goto loop; goto loop;
} }
break;
} }
return iter->last_visited; iter->last_visited = current;
return current;
} }
/* /*