Inserting string in linked list ends up in segmentation fault

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)

this is c++ - you should be using `std::list`, or at the very least, use `new` instead of `malloc`. — Red Alert, May 08 '14 at 21:05
Please do never 'malloc' a structure holding C++ data members! — , May 08 '14 at 21:07
that's for data structures class can't use list,but thanks for the help :D — Bezzi, May 08 '14 at 21:41
You are in a Data Structures class and they want you to write a linked list like this? — Brandon Buck, May 09 '14 at 01:49

Yirkha · Accepted Answer · 2014-05-08T21:17:46.780

4

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.

edited May 08 '14 at 21:17

answered May 08 '14 at 21:08

Yirkha

12,737
5
38
53

This does not matter in his case - he sets all the fields of the `var` before using them. – mark May 08 '14 at 21:10
It does -- `var->data = value;` invokes `std::string::operator=`, which sees e.g. `0xdeadbeef` as its buffer pointer and calls `delete[]` on it -> crash. – Yirkha May 08 '14 at 21:14
@mark It does - the string in the node will not get constructed using 'malloc' – May 08 '14 at 21:15
Oh, I missed that data was a C++ object. – mark May 08 '14 at 21:15

score -1 · Answer 2 · answered May 08 '14 at 21:07

-1

I would say that you forgot to initialize head to NULL

answered May 08 '14 at 21:07

mark

59,016
79
296
580

Global/static variables are always initialized to zeroes. – Yirkha May 08 '14 at 21:10
Are you sure? I thought it was only for the static. – mark May 08 '14 at 21:10
Yes -- "plain" globals also have "static storage duration", even though they don't have "static scope". Also e.g. http://stackoverflow.com/a/3553682/3543211 – Yirkha May 08 '14 at 21:15

Inserting string in linked list ends up in segmentation fault

2 Answers2