I gave a glance at the code around the region you mentioned. I believe you are correct as this is a bug.
void BinarySearchTree::remove(int d)
{
...
tree_node* curr;
tree_node* parent;
curr = root;
...
//Node with 2 children
// replace node with smallest value in right subtree
if (curr->left != NULL && curr->right != NULL)
{
tree_node* chkr;
chkr = curr->right;
if((chkr->left == NULL) && (chkr->right == NULL))
{
curr = chkr;
delete chkr;
curr->right = NULL;
}
In this code section, curr and chkr are both declared as a pointer to a tree_node instance. While running curr = chkr
, pointer value for chkr is wirtten to curr and thus point curr to instance chkr is pointing to. By delete chkr
, the instance is destroyed and garbage collected. Thus curr is now pointing to a non-existed object who is out of life. This IS a dangling pointer by definition, if I remember correctly.
If I'm all correct above and the understanding for that particular chunk, the following is the fix for this:
curr->data = chkr->data;
to replace
curr = chkr;
Regarding memory leak. Sorry, I did not read the whole code. It appears msram's code should be merely for displaying the correct logic. That is a lot more extra information for that purpose though.