Solved
Problem ultimately stemmed from the design of the data structure. To remove the root element there had to be a heap allocated (new) pointer to it, which wasn't possible in the original case where the data was held directly by the tree. Now there is a node
struct holding all the data, with the tree
class containing all the methods and the root pointer.
I have a double pointer target_address
initialized to NULL
and I have to check whether *target_address
is NULL later. In between, a function guarantees that *target_address
points to an uninitialized value, but Valgrind keeps complaining that I'm reading an uninitialized value only when I remove the root element.
template <typename T>
int tree<T>::remove(T target) {
if (!this) // root is empty
return EMPTY;
tree<T>** target_address = NULL;
if (tree_find(target, &target_address) == SUCCESS) {
// expect there to be something in target_address
if (*target_address == NULL) return EMPTY; // <---- error happens
tree_delete(target_address);
return SUCCESS;
}
else return EMPTY;
}
tree_find that operates on target_address
template <typename T>
int tree<T>::tree_find(T key, tree<T>*** target_address_handle) {
// find tree matched by key, or NULL pointer in correct location
// give tree pointer address back
tree<T>* root = this; // <---- ensures *target_address points to valid value, maybe this is problematic?
tree<T>** target_address = &root;
while(*target_address) {
tree<T>* current = *target_address;
if(typeid(key) == typeid(current->data)) {
// assume comparison operator exists
if(key == current->data)
break;
else if(key < current->data)
target_address = ¤t->left;
else
target_address = ¤t->right;
}
else return FAIL;
}
// if loop exited without breaking, will insert into an empty NULL position
// else loop exited by matching/breaking, will delete non-NULL tree
*target_address_handle = target_address;
return SUCCESS;
}
tree_delete that also complains of uninitialized value; I'm pretty sure it's also complaining of *target_address
void tree<T>::tree_delete(tree<T>** target_address) {
tree<T>* target = *target_address;
// first case: no left subtree, replace with right subtree (or NULL for a leaf)
if (!target->left)
*target_address = target->right;
// second case: no right subtree, replace with left subtree
else if (!target->right)
*target_address = target->left;
// third case: both subtrees, find second largest by taking rightmost left tree
else {
tree<T>** second_largest_address = &target->left; // start at target's left tree
while ((*second_largest_address)->right) // keep going right
second_largest_address = &((*second_largest_address)->right);
// reached the rightmost left
tree<T>* second_largest = *second_largest_address;
*target_address = second_largest; // delete target by replacing it with second largest
// second largest guranteed to not have a right subtree, so can treat as case 2 by shifting
*second_largest_address = second_largest->left;
second_largest->left = target->left;
second_largest->right = target->right;
}
delete target;
}
P.S. Tips for getting Valgrind to display line numbers would be appreciated, compilation flags = -g -Wall -Werror -std=c++11
and Valgrind's run under -q --track-origins=yes
I tried static linking -static
as suggested in someone else's question, but that introduced many more problems and solved none...
Error messages
==25887== Conditional jump or move depends on uninitialised value(s)
==25887== at 0x401764: tree<int>::remove(int) (in /home/johnson/Code/tree/treetest)
==25887== by 0x4013AA: main (in /home/johnson/Code/tree/treetest)
==25887== Uninitialised value was created by a stack allocation
==25887== at 0x40175A: tree<int>::remove(int) (in /home/johnson/Code/tree/treetest)
==25887==
After reading *target tree address
Before read
==25887== Use of uninitialised value of size 8
==25887== at 0x401A75: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887== by 0x40178E: tree<int>::remove(int) (in /home/johnson/Code/tree/treetest)
==25887== by 0x4013AA: main (in /home/johnson/Code/tree/treetest)
==25887== Uninitialised value was created by a stack allocation
==25887== at 0x401A46: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887==
==25887== Use of uninitialised value of size 8
==25887== at 0x401AA5: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887== by 0x40178E: tree<int>::remove(int) (in /home/johnson/Code/tree/treetest)
==25887== by 0x4013AA: main (in /home/johnson/Code/tree/treetest)
==25887== Uninitialised value was created by a stack allocation
==25887== at 0x401A46: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887==
==25887== Use of uninitialised value of size 8
==25887== at 0x401AF2: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887== by 0x40178E: tree<int>::remove(int) (in /home/johnson/Code/tree/treetest)
==25887== by 0x4013AA: main (in /home/johnson/Code/tree/treetest)
==25887== Uninitialised value was created by a stack allocation
==25887== at 0x401A46: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887==
==25887== Invalid read of size 8
==25887== at 0x401AF5: tree<int>::tree_delete(tree<int>**) (in /home/johnson/Code/tree/treetest)
==25887== by 0x40178E: tree<int>::remove(int) (in /home/johnson/Code/tree/treetest)
==25887== by 0x4013AA: main (in /home/johnson/Code/tree/treetest)
==25887== Address 0x801f0fc36d is not stack'd, malloc'd or (recently) free'd