Inserting string in linked list ends up in segmentation fault [closed]

Question

Closed. This question is not reproducible or was caused by typos. It is not currently accepting answers.

---

This question was caused by a typo or a problem that can no longer be reproduced. While similar questions may be on-topic here, this one was resolved in a way less likely to help future readers.

Closed 9 years ago.

Improve this question

I am trying to create a linked-list with a string as the value inside each node, but every time I try to call the function that's supposed to add something to the list, it ends up giving me a segmentation fault error.

This is my code:

#include <iostream>
#include <vector>
#include <string>
#include <fstream>
#include <stdlib.h> 
#include <stdio.h>

using namespace std;

struct node
{
    string data;
    struct node *next;
}*head,*var,*trav;

void add(string value)
{
      struct node *temp; 
      temp=head;
      var=(struct node *)malloc(sizeof (struct node));
      var->data =  value;
      if(head==NULL)
      {
          head=var;
          head->next=NULL;
      }
      else
      {
          while(temp->next!=NULL)
          {     
               temp=temp->next;
          }
          var->next=NULL;
          temp->next=var;
      }
}

int main(int argc, char const *argv[])
{
    add("hello");
    return 0;
}

It seems that the problem is in the var->data = value;, but what exactly is causing it? And how could I fix that?

This is the error that shows up:

/bin/bash: line 1: 13056 Segmentation fault      (core dumped) 

Answer 1

malloc() does not call the constructor of the std::string data, it's just a C function which allocates a chunk memory of specified size and the contents is random junk.

Therefore when you subsequently assign to the string, it does not just set the value like a plain variable does -- it calls function std::string::operator=, which tries to release any previous contents first, but that's not correct and it crashes.

You need to use C++ operator new to create the node instead, like so:

var = new node;

As you can see, you also don't need the ugly casts anymore.

Answer 2

I would say that you forgot to initialize head to NULL