CODEWITHSUNDEEP
c++segmentation-fault
Jeremy
Jeremy

Reputation: 201

segmentation fault only after accessing struct in linked list for a second time

Sorry for the unclear title, I really don't know how to describe this issue. I'm in my first year of computer science so I really don't know much about C++ yet. However, trying to look up this issue did not help.

The issue: In the main function, the "printRawData" friend function is called twice. The function is supposed to print each element of the linked list stored by the the class "LinkedList". It works the first time, but the second time I get a segmentation fault. I really have no idea what I'm doing wrong. My T.A. said he thinks that the struct's string variable "element_name" is being corrupted when accessed.

Sorry for the messy code, if I'm not explaining my issue well, or if I'm breaking any kind of stackoverflow etiquette. I appreciate any help I get. 

//Note: C++ 11 is needed, due to to_string use
#include <iostream>
#include <string>

using namespace std;

struct Node {
  string element_name;
  int element_count;
  Node* next;
};

class LinkedList{
  private:
    Node* first;
  public:
    LinkedList();
    ~LinkedList();
    bool isEmpty();
    void AddData(string name, int count);
    friend void printRawData(LinkedList l);
};

//where the error occurs
void printRawData(LinkedList l){
  Node* n = l.first;
  while (n != NULL) { //iterates through the linked list and prints each element
    cout << n->element_name << " : " << n->element_count << endl;
    n = n->next;
  }
}

LinkedList::LinkedList(){
  first = NULL;
}

LinkedList::~LinkedList(){
  Node* n = first;
  while (n != NULL) {
    Node* temp = n;
    n = temp->next;
    delete temp;
  }
}

bool LinkedList::isEmpty(){
  return first == NULL;
}

void LinkedList::AddData(string name, int count){
  Node* newnode = new Node;
  newnode->element_name = name;
  newnode->element_count = count;
  newnode->next = NULL;

  Node* n = first;

  //if the linked list is empty
  if(n == NULL){
    first = newnode;
    return;
  }

  //if there's only one element in the linked list,
  //if the name of first element comes before the name of new element,
  //first element's pointer is to the new element.
  //otherwise, the new node becomes the first and points to the previous first
  //element.
  if (n->next == NULL){
    if (n->element_name < newnode->element_name){
      n->next = newnode;
      return;
    } else {
      newnode->next = first;
      first = newnode;
      return;
    }
  }

  //if the first element's name comes after the new element's name,
  //have the new element replace the first and point to it.
  if (n->element_name > newnode->element_name){
    newnode->next = first;
    first = newnode;
    return;
  }

  //iterating through linked list until the next element's name comes after
  //the one we're inserting, then inserting before it.
  while (n->next != NULL) {
    if (n->next->element_name > newnode->element_name){
      newnode->next = n->next;
      n->next = newnode;
      return;
    }
    n = n->next;
  }

  //since no element name in the linked list comes after the new element,
  //the node is put at the back of the linked list
  n->next = newnode;
}



main(){
  LinkedList stack;

  stack.AddData("Fish", 12);
  stack.AddData("Dog", 18);
  stack.AddData("Cat", 6);

  printRawData(stack);
  printRawData(stack);
}

Upvotes: 1

Views: 42

Answers (1)

Bo Persson
Bo Persson

Reputation: 92371

The function void printRawData(LinkedList l) passes the parameter by value, so it gets a copy of the LinkedList object.

However, the copy contains a copy of the first pointer, but doesn't copy any of the nodes. So when this copy is destroyed, the LinkedList destructor will delete all the nodes.

And then the original is damaged.

You might want to pass a reference instead of creating a copy.

This is also the reason why the std::list has copy constructors and assignment operators that perform a "deep copy", where the nodes are also copied (not just the list head).

Upvotes: 2

Related Questions