Segmentation fault on strcpy even though I am using malloc

Question

I am trying to make a Doubly Linked List in C.

And I get a Segmentation Fault even though I use malloc.

Here is my code so far.

list.h

#ifndef _LIST_
#define _LIST_

typedef struct listnode
{
    char * data;
    struct listnode * next;
    struct listnode * prev;
}listnode;

typedef struct list
{
    listnode * firstnode; // it will point to the first element in the list
    int size; // the size of the list
}list;

list create_list();
void insert_first_element(char *, list);

#endif

list.c

#include "list.h"
#include <stdlib.h>
#include <string.h>

list create_list()
{
    list L;
    L.firstnode= NULL;
    L.size = 0;

    return L;
}

void incert_first_element(char * d, list L)
{
    listnode * N= (listnode *)malloc(sizeof(listnode));
    
    strcpy(N->data, d); // <-- I get Segmentation Fault Here

    if(L.firstnode != NULL)
    {
        N->next=L.firstnode;
        N->prev=L.firstnode->prev;
        L.firstnode->prev=N;
        L.firstnode=N;
    }
    else
    {
        N->next=NULL;
        N->prev=N;
        L.firstnode=N;
    }
    L.size++;
    return 0;
}

main.c

#include <stdio.h>
#include "list.h"

int main(void)
{
   list L = create_list();
   incert_first_element("test",L);
  
   return 0;
}

Any idea what is causing the Segmentation Fault?

Because any problems I found when googling were caused by the lack of malloc, but here I do implement it.

Just for your information, your include guard should probably not start with an underscore: https://stackoverflow.com/a/49688652/2770331 — T S, Mar 06 '21 at 20:55

score 1 · Accepted Answer · answered Mar 06 '21 at 20:02

1

this code

listnode * N= (listnode *)malloc(sizeof(listnode));

strcpy(N->data, d); // <-- I get Segmentation Fault Here

allocates a listnode structure but the data field is a pointer on a char, so it's not initialized by the malloc call.

The second line should be replaced for instance by a strdup call

N->data = strdup(d);

deallocation should also be done in 2 passes. First free(N->data) then free(N)

answered Mar 06 '21 at 20:02

Jean-François Fabre

137,073
23
153
219

1

OP is passing the list by value so the insert function will have no effect on return. Arg should be `List *L` with `L.` --> `L->` in the body. – Craig Estey Mar 06 '21 at 20:15

chqrlie · Answer 2 · 2021-03-06T21:19:08.130

There are multiple problems in your code:

incert_first_element receives a copy of the list structure and modifies it. This has no effect on the list object of the caller. You should instead pass a pointer to the caller's list object.
the incert_first_element function allocates a new listnode object, but not for the string. The member data is a pointer, not an array, malloc() does not initialize it so strcpy(N->data, d); copies the characters into an uninitialized pointer, invoking undefined behavior (a segmentation fault stooping the program). You should allocate a copy of the string with N-strcpy(N->data, d);
you insert the node at the beginning of the doubly linked list, thus it is incorrect to set N->prev = N; the previous node should be set to NULL in both cases.
in list.c, you should #include "list.h" after the standard headers.

Here is a modified version:

list.h

#ifndef LIST_H
#define LIST_H

typedef struct listnode {
    char *data;
    struct listnode *next;
    struct listnode *prev;
} listnode;

typedef struct list {
    listnode *firstnode; // it will point to the first element in the list
    int size; // the size of the list
} list;

list create_list(void);
void insert_first_element(list *, const char *);
#endif

list.c

#include <stdlib.h>
#include <string.h>
#include "list.h"

list create_list(void) {
    list L = { NULL, 0 };
    return L;
}

void insert_first_element(list *L, const char *d) {
    listnode *N = malloc(sizeof(*N));
    if (N == NULL) {
        return -1;
    }
    N->data = strdup(d);
    N->prev = NULL;
    N->next = L->firstnode;

    if (L->firstnode != NULL) {
        L->firstnode->prev = N;
    }
    L->firstnode = N;
    L->size++;
    return 0;
}

main.c

#include <stdio.h>
#include "list.h"

int main(void) {
   list L = create_list();
   insert_first_element(&L, "test");
  
   return 0;
}

Segmentation fault on strcpy even though I am using malloc

2 Answers2