How to delete root node as part of full tree deletion

Question

I am making a binary search tree that has a function that is deleting all nodes in the tree. When called later on it seems all nodes are being deleted but the root node. Before adding the conditionals to the code below there were also other nodes that weren't getting deleted. That's fixed at this time, but the root node is not getting deleted. Wondering what conditionals should be added or if there's something I'm not understanding about root deletion.

I've tried a simpler solution where no conditionals were used. Program ran fine but after calling a traversal again at the end it seems not everything is actually getting deleted.

TreeNodePtr deleteTree(TreeNodePtr node)
{


    if(node -> left)
    {
        deleteTree(node -> left);
        printf("Deleting node %s \n", node -> left -> data.word);
        free(node -> left);
        node -> left = NULL;
    }

    if(node -> right)
    {
        deleteTree(node -> right);
        printf("Deleting node %s \n", node -> right -> data.word);
        free(node -> right);
        node -> right = NULL;
    }


    if(allocation_count == 1)
    {
        printf("Deleting node %s \n", node -> data.word);
        free(node);
        node = NULL;
    }


    //whenever a node is deleted this decreases by one, when at one  
    //attempt to delete root node
    allocation_count--;

    return node;

}

All of the deletion instances are being printed out, but the root is not actually getting removed from the tree. One node value remains and gets printed out when a traversal is called after the deletion process.

Are you sure that the root node is not deleted? Please, note: `free()` releases the memory the passed pointer points to (if not `NULL`). However, the pointer (variable) isn't changed. You even might be able to access the pointee afterwards. Releasing the memory doesn't change it necessarily - it might be still intact (but you cannot count on this). After `free()`, the passed pointer may not be used anymore. In case, you may reset it to `NULL` to remark it as invalid. — Scheff's Cat, Oct 27 '19 at 12:46
But *the function* cannot reset the node pointer to null if it receives it by value, as this function does. It can, however, return `NULL`, with the expectation that the caller will update the root pointer with the function's return value. As it looks, on first glance, as if the OP's function does. — John Bollinger, Oct 27 '19 at 12:49
Concerning `free(node); node = NULL;`: That resets the local variable (actually argument) `node`. In C, arguments are passed by value exclusively. Hence, the originally passed pointer on caller side is unchanged (and dangling). In C, reference arguments are "emulated" by pointers to pointers. So, with `TreeNodePtr *node` you could pass the address of the pointer variable (on caller side) and the function could modify it. This might be an alternative to what @JohnBollinger suggested. — Scheff's Cat, Oct 27 '19 at 12:50
I figured it out actually, it had to do with something outside the function. — roberts4, Oct 27 '19 at 14:26

Deduplicator · Answer 1 · 2019-10-27T13:04:43.167

2

The code you show is unneccessarily convoluted, hiding subtle issues.
Anyway, it cannot modify the argument passed, as in C all arguments are passed by value.
Condiering you return a TreeNode*, the caller is probably responsible for assigning it to the root-pointer.

Also, unless you only ever have at most one tree, using a global for allocationCount is wrong, and leads to a bug.

And finally, what if your tree is empty?

Simplified fixed code:

TreeNode* deleteTree(TreeNode* node) {
    if (!node)
        return 0;
    deleteTree(node->left);
    deleteTree(node->right);
    printf("Deleting node %s\n", node->data.word);
    free(node);
    --allocationCount; // Whatever for. Statistics maybe?
    return 0;
}

edited Oct 27 '19 at 13:04

answered Oct 27 '19 at 12:54

Deduplicator

44,692
7
66
118

Tried this, not working its now printing even more nodes when traversal is called afterwards – roberts4 Oct 27 '19 at 13:18
Based off of what I've tried I think I need those conditionals in there, the first two anyways – roberts4 Oct 27 '19 at 13:29
1

Did you null the pointer to the root-node after deleting the tree? – Deduplicator Oct 27 '19 at 14:23
I just figured it out actually- thanks for the responses though – roberts4 Oct 27 '19 at 14:26

Emily · Answer 2 · 2019-10-27T12:57:00.020

1

Your algorithm immediately starts by deleting the left and right subtrees, and does not delete the root node.

Remember that trees are a recursive structure, and each subtree has its own root node. Hence, you should delete node and then recursively call deleteTree on the left and right subtrees.

This should work:

void deleteTree(TreeNodePtr node)
{
    if(node -> left)
    {
        deleteTree(node -> left);
        node -> left = NULL;
    }

    if(node -> right)
    {
        deleteTree(node -> right);
        node -> right = NULL;
    }

    free(node);
}

edited Oct 27 '19 at 12:57

answered Oct 27 '19 at 12:54

Emily

1,030
1
12
20

Unfortunately this is not working, I think I also tried something like this earlier – roberts4 Oct 27 '19 at 13:14

How to delete root node as part of full tree deletion

2 Answers2