getting segmentation fault in searching an element in binary search tree in c++
node ** BST :: searchElement(node **tree, int item)
{
if( ((*tree)->data == item) || ( (*tree) == NULL) )
return tree;
else if( item < (*tree)->data)
return searchElement( &(*tree)->left, item);
else
return searchElement( &(*tree)->right, item);
}
int main(){
BST obj;
int choice;
int height=0,total=0,n,item;
node开发者_运维技巧 **tmp;
system("cls");
while(1){
//clrscr();
cout<<"*****BINARY SEARCH TREE OPERATIONS*****\n\n";
cout<<"1) Create Tree\n";
cout<<"2) Traversal\n";
cout<<"3) Insert Node\n";
cout<<"4) Search Node\n";
cout<<"5 Find Smallest Node\n";
cout<<"6) Find Largest Node\n";
cout<<"7) Exit\n";
cout<<"Enter your choice : ";
cin>>choice;
switch(choice){
case 1 : //Create Tree
cout<<"\n\n--Creating Tree--";
cout<<"\nHow many nodes u want to enter : ";
cin>>n;
for(int i=0;i<n;i++){
cout<<"Enter value : ";
cin>>item;
obj.createTree(&obj.tree,item);
}
break;
case 2 : //All Traversals
cout<<"\n\nInorder Traversal : ";
obj.inOrder(obj.tree);
cout<<"\n\nPre-order Traversal : ";
obj.preOrder(obj.tree);
cout<<"\n\nPost-order Traversal : ";
obj.postOrder(obj.tree);
getch();
break;
case 3 : //Inserting a node in a tree
cout<<"\n\n--Inserting Node in a tree--\n";
cout<<"Enter value : ";
cin>>item;
obj.createTree(&obj.tree,item);
cout<<"\nItem is inserted\n";
getch();
break;
case 4 : //Search element
cout<<"\n\n--Search Element--\n";
cout<<"Enter item to searched : ";
cin>>item;
&(*tmp) = obj.searchElement(&obj.tree,item);
if( (*tmp) == NULL)
cout<<"\nSearch Element was not Found";
else
cout<<"\nSearch Element was Found";
getch();
break;
case 5 : //Find Smallest Node
cout<<"\n\nSmallest Node is : ";
obj.findSmallestNode(obj.tree);
getch();
break;
case 6 : //Find Largest Node
cout<<"\n\nLargest Node is : ";
obj.findLargestNode(obj.tree);
getch();
break;
case 7: exit(1);
}//end of switch
}
}
In the above program, only case 4 is not working correctly when I try to find the particular element in tree. I have included search element member function on the top of the main program. When I was debugging program, I was getting segmentation fault in search element member function especially in if condition. I really don't know what I need to do to come out of this problem. Can anyone please help me to find out why segmentation fault is happening inside search element member function. Let me know if you have any questions regarding this program.
if( ((*tree)->data == item) || ( (*tree) == NULL) )
Should be
if ( ( (*tree) == NULL) || ((*tree)->data == item) )
If *tree
actually is null you're dereferencing a null pointer in the first check. Swapping them around will ensure that *tree
isn't NULL when you check (*tree)->data
- due to Short Circuit Evaluation
Additionally, &(*tmp)
should be written as just tmp
You're dereferencing an uninitialized pointer (tmp). You should either allocate memory for it or just skip it's usage (I really can't figure out why you need a temporary NODE** here.)
Here are a couple critiques:
Since you are only searching for a node, you don't need pointers-to-pointers. The only time you need pointers to pointers are when you actually need to modify the parameter. Also, since you are using C++, instead of passing a pp, you should pass a reference: node * &tree. This makes it so you can work with the tree variable without having to dereference it, since the compiler will take care of that for you.
In your if statements, you aren't checking if your left or right pointers are null pointers. I'm not sure if you have sentinel nodes for this, but I'm assuming you don't. With that, I would change your method to this:
node * BST :: searchElement(node *tree, int item)
{
if(tree->data == item)
return tree;
//short circuit if statements
else if( (tree->left != NULL) && (item < tree->data) )
return searchElement( tree->left, item );
else if( (tree->right != NULL) && (item > tree->data) ) //>= for duplicates
return searchElement( tree->right, item );
return NULL; //if it isn't found
}
Yes. Indeed as Erik already posted you MUST write
if ( ( (*tree) == NULL) || ((*tree)->data == item) )
instead of
if( ((*tree)->data == item) || ( (*tree) == NULL) )
Because if item
is not already in a tree your code will DEFINITELY lead to segfault when trying to dereference NULL pointer.
There is also another (not so obvious) problem - absolutely unnecessary recursion. If you are not doing careful balancing when insert or remove tree nodes you will have at most linear tree height and thus at most linear recursion depth that may easily lead to stack overflow. So you should transform searchElement
function to
node** BST::searchElement( node** tree, int item )
{
while( ( (*tree) != NULL) && ( (*tree)->data != item ) )
{
if( item < (*tree)->data )
{
tree = &(*tree)->left;
}
else
{
tree = &(*tree)->right;
}
}
return tree;
}
精彩评论