Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove redundant if statements in avl tree delete_generic() #1894

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

mvandenhoek
Copy link
Contributor

This was discovered whilst trying to improve code coverage. The issue is with the following code:

if (node->cs[0] == NULL)
{
if (node->cs[1]) {
node->cs[1]->parent = node->parent;
}
*pnode = node->cs[1];
whence = node->parent;
}
else if (node->cs[1] == NULL)
{
node->cs[0]->parent = node->parent;
*pnode = node->cs[0];
whence = node->parent;
}
else
{
ddsrt_avl_node_t *subst;
assert (node->cs[0] != NULL);
assert (node->cs[1] != NULL);
/* Use predecessor as substitute */
if (path) {
path->p.pnode[++path->p.depth] = &node->cs[0];
}
subst = node->cs[0];
if (subst->cs[1] == NULL) {
whence = subst;
} else {
do {
if (path) {
path->p.pnode[++path->p.depth] = &subst->cs[1];
}
subst = subst->cs[1];
} while (subst->cs[1]);
whence = subst->parent;
whence->cs[1] = subst->cs[0];
if (whence->cs[1]) {
whence->cs[1]->parent = whence;
}
subst->cs[0] = node->cs[0];
if (subst->cs[0]) {
subst->cs[0]->parent = subst;
}
if (path) {
path->p.pnode[path->p.pnodeidx+1] = &subst->cs[0];
}
}
subst->parent = node->parent;
subst->height = node->height;
subst->cs[1] = node->cs[1];
if (subst->cs[1]) {
subst->cs[1]->parent = subst;
}
*pnode = subst;
}

If you go into the else of line 485, then proceed past line 506, you have a situation like in the depicted below:

             node <-- delete target
            /    \
           /      \
        whence     subtree
        /   \
 subtree    subst
           /
          ? 

The if and if else preceding the else of line 485 guarantee node->cs[0] != NULL and node->cs[1] != NULL.
Thus if (subst->cs[0]) { can be scrapped.
Likewise, if (subst->cs[1]) { can also be scrapped.

Signed-off-by: Michel van den Hoek <michel.vandenhoek@zettascale.tech>
Copy link
Contributor

@eboasson eboasson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, all nodes are unique and so node, subst and whence are do not alias. Thus the value of node->cs[{0,1}] does not changed and the new values of subst->cs[{0,1}] cannot be null pointers.

@eboasson eboasson merged commit b829fa9 into eclipse-cyclonedds:master Dec 4, 2023
20 of 23 checks passed
@mvandenhoek mvandenhoek deleted the avl_fix_branch branch December 4, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants