C- Trying to add a node to the end of a linked list causes crash

Question

I am trying to implement a linked list in C. Here is my definition of a Node structure, followed by a function for adding a Node to the end of the linked list.

struct Node{
    char *name;
    struct Node *next;
};
typedef struct Node Node;

void addNode(Node *head, char n[100]){
    if(head->next == NULL){
        Node new;
        new.name = n;
        new.next = NULL;
        head->next = &new;
    }
    else{
        addNode(head->next, n);
    }
}

I can create a head node and pass a pointer for it into the addNode function just fine to add a second node to the linked list. (Also worth mentioning that when I make a head node, I set its "next" pointer to NULL to represent the end of the linked list) The problem seems to be with the recursive call to addNode in the else branch of the addNode function, as my program works without crashing when I comment it out. Why is this causing my program to crash, and how can I fix it?

You are using local variable inside `addNode`. It goes out of scope when you return from the function. So the next time when you call the function it might cause crash. And further you need to allocate seperate memory for `name` and use strcpy to copy `n` to `name` — Karthick, Sep 20 '17 at 04:36
`Node new; --> Node * new = malloc(sizeof * new);` (change other assignments accordingly), also need to check if `head` itself is `NULL`. — Ajay Brahmakshatriya, Sep 20 '17 at 04:38

score 1 · Accepted Answer · answered Sep 20 '17 at 04:41

if(head->next == NULL){
    Node new;
    new.name = n;
    new.next = NULL;
    head->next = &new;
}

Your problem is actually here. The variable new has automatic storage duration, which means that it ceases to exist at the end of the block in which it was declared - in this case, at the closing brace at the end of this if().

The pointer you have saved in head->next now points at a no-longer-existing object - this is called a "dangling pointer". When you later use the dangling pointer, your program's behaviour is no longer defined.

You instead want to allocate a Node that will live beyond the invocation of the function that created it - that's what malloc() is used for:

if(head->next == NULL) {
    Node *new = malloc(sizeof *new);

    if (new) {
        new->name = n;
        new->next = NULL;
        head->next = new;
    } else {
        /* malloc failed */
    }
}

The object created with malloc() has a lifetime that extends until you pass a pointer to it to free(), which you would do if you were removing the Node from the linked list and discarding it.

C- Trying to add a node to the end of a linked list causes crash

1 Answers1