0

I have the Segmentation fault (core dumped) error.

main.c

#include "header1.h"

#include <stdlib.h>
#include <stdio.h>

int main(int argc, char** argv) {
        struct t_list *list = NULL;
        doFill(list);
        printf("%s\n", list->name);
        free(list);
        return 0;
}

header1.h

#ifndef HEADER1_H
#define HEADER1_H

struct t_list {
  char *name;
  struct t_list *next;
};

void doFill(struct t_list *list);

#endif

worker.c

#include "header1.h"
#include <stdlib.h>

void doFill(struct t_list *list) {
    list = (struct t_list *) malloc(sizeof(struct t_list));
    char *tmp = "somename";
    list->name = tmp;
    list->next = NULL;
}

When I run this (gcc -g main.c worker.c -o test) I get (on the line with printf in main.c):

Segmentation fault (core dumped)

In gdb I see:

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffddf8) at main.c:8
8       struct t_list *list = NULL;
(gdb) next
9       doFill(list);
(gdb) step
doFill (list=0x0) at worker.c:6
6       list = (struct t_list *) malloc(sizeof(struct t_list));
(gdb) p list
$1 = (struct t_list *) 0x0
(gdb) next
7       char *tmp = "somename";
(gdb) p list
$2 = (struct t_list *) 0x0

As you can see malloc in worker.c doesn't allocate memory for the list variable (the pointer before and after malloc points at 0x0).

If I move the code from the doFill procedure in main.c it works correctly:

main.c

#include "header1.h"

#include <stdlib.h>
#include <stdio.h>

int main(int argc, char** argv) {
    struct t_list *list;
    list = (struct t_list *) malloc(sizeof(struct t_list));
    char *tmp = "somename";
    list->name = tmp;
    list->next = NULL;
    printf("%s\n", list->name);
    free(list);
    return 0;
}

$ gcc -g main.c -o test
$ ./test
somename

How is it possible? What do I do wrong?

gcc version 4.8.2 (Ubuntu 4.8.2-19ubuntu1)
neshkeev
  • 6,280
  • 3
  • 26
  • 47
  • 3
    `void doFill(struct t_list *list)` You need `**` here to make allocation visible by caller. – Alex F Nov 09 '14 at 14:17
  • [Don't cast the result of malloc (and friends)](http://stackoverflow.com/q/605845). – Deduplicator Nov 09 '14 at 14:23
  • @Deduplicator Thank you for the link, I read the `The C Programming Language` book that was written by the creators of the c language. They casted results of malloc, I do cast too. But the book was written in 1988 and I agree that it might have been obsolete behavior already. – neshkeev Nov 09 '14 at 14:38

2 Answers2

2

You aren't receiving back the new value of list. In fact, passing list in is totally useless. Better to pass in the name for this node.

typedef struct t_list List;

List *newListNode(char *name) {
    List *list = malloc(sizeof(*list));
    if (!list) return NULL;
    list->name = strdup(name);
    if (!list->name) { free(list); return NULL; }
    list->next = NULL;
    return list;
}

char *strdup(char *src) {   // if strdup doesn't already exist.
    char *dst = malloc(strlen(src) + 1);
    if (!dst) return NULL;
    strcpy(dst, src);
    return dst;
}

To add nodes to the front of the list:

List *listAdd(List *list, char *name) {
    List *newnode = newListNode(name);
    if (!newnode) return NULL;
    if (list) newnode->next = list;
    return newnode;
}

To delete the list, remember to delete the malloced strings:

void deleteList(List *list) {
  for (List *next; list; list = next) {
    next = list->next;
    free(list->name);
    free(list);
  }
}
ooga
  • 15,423
  • 2
  • 20
  • 21
1

Parameters in C are passed by copy. The changes you make to list inside of doFill() are not propagated back to main(), which means that list is always NULL in main(). Try passing a pointer to pointer instead:

#include "header1.h"

#include <stdlib.h>
#include <stdio.h>

int main(int argc, char** argv) {
        struct t_list *list = NULL;
        doFill(&list);
        printf("%s\n", list->name);
        free(list);
        return 0;
}

And then change doFill() accordingly:

#include "header1.h"
#include <stdlib.h>

void doFill(struct t_list **list) {
    *list = malloc(sizeof(**list));
    char *tmp = "somename";
    (*list)->name = tmp;
    (*list)->next = NULL;
}
Filipe Gonçalves
  • 20,783
  • 6
  • 53
  • 70