Segmentation fault while inserting in Binary search tree

Question

I am trying to build a basic Binary search tree in C++ and running into some problems, specifically when I try to insert a node via function and read, it gives me segmentation fault. But the same node struct works perfectly fine when I manually insert it. The code for BST insert is as follows, and is most likely the culprit:

void BST::insert(Node* temproot,int val){
  // std::cout << root->value <<std::endl;
  if(!temproot){  
    Node* newNode = new Node;
    newNode->value = val;
    temproot = newNode;
    std::cout << "Added Node with Value: " << val << std::endl;
    return;
  }
  if(val<(temproot->value)){
    std::cout << "LEFT" << std::endl;
    insert(temproot->left, val);
  }else{
    std::cout << "RIGHT" << std::endl;
    insert(temproot->right, val);
  } 
}

The node structure looks like this:

struct Node{
  int value;
  Node* left = nullptr, *right = nullptr;
};

And the BST class looks something like below:

  class BST{
  public:
  Node* root= new Node;
    BST(int val){      

      root->value = val;
    }
    void insert(Node*,int val);
    void insertStart(int vasl){
      Node* temproot = root;
      insert(temproot, vasl);
    }
    void print(Node*);
    void _print(){
      print(root);
    }
};

When I try to print it as follow, it gives me segmentation fault:

void BST::print(Node* temp){
  std::cout << temp->value << std::endl;
  temp = temp->left;
  std::cout << (temp->value) << std::endl;
}

I am a bit new to C++ and am having struggle pin pointing it for couple of days. Can someone help me figure out what I am doing wrong here?

Are you aware that `temproot` passed to `insert` is *a copy* of the original pointer, and the original pointer is not modified? — Yksisarvinen, Jan 31 '20 at 14:10

NutCracker · Answer 1 · 2020-01-31T14:19:07.597

1

Basically, SEGFAULT comes from print function which should look like this:

void BST::print(Node* temp){
    if (nullptr == temp) {
        return;
    }

    print(temp->left);
    std::cout << temp->value << std::endl;
    print(temp->right);
}

And your insert function should look like this:

void BST::insert(Node *&temproot,int val){
    if(nullptr == temproot){  
        Node* newNode = new Node;
        newNode->value = val;
        temproot = newNode;
        return;
    }
    if(val < (temproot->value)){
        insert(temproot->left, val);
    }else{
        insert(temproot->right, val);
    } 
}

Check it out live

edited Jan 31 '20 at 14:19

answered Jan 31 '20 at 14:11

NutCracker

11,485
4
44
68

I try to do it manually cause it was causing errors, the actually print function is just like the one you have described above. The one mention here is just cause I have been trying to debug the issue. – Priyank Trivedi Jan 31 '20 at 14:14
@PriyankTrivedi anyway, the `print` function you showed is incorrect – NutCracker Jan 31 '20 at 14:22

Vlad from Moscow · Accepted Answer · 2020-01-31T14:29:49.600

The function deals with a copy of the passed to it pointer to node. So changing a copy does not influence on the original pointer.

You have to pass a pointer to node to the function by reference. The function declaration can look the following way

void insert(Node* &temproot,int val);

and call it like

void insertStart(int vasl){
  insert( root, vasl );
}

without an intermediate pointer.

And you should declare this function as a provate static function of the class.

And initialize the data member root by nullptr.

For example

  class BST{
  public:
  Node* root = nullptr;

    BST(int val){      

      insert( root, val );
    }
    void insertStart(int vasl){
      insert( root, vasl);
    }
    void print(Node*);
    void _print(){
      print(root);
    }

private:
    static void insert(Node* &,int val);
};

Segmentation fault while inserting in Binary search tree

2 Answers2