Copying struct pointer element to a pointer seems to corrupt it

I'm still learning C and I must have misunderstood pointers at some point.

I was under the impression that the following lines would copy the memory address st开发者_开发知识库ored by l->first to temp. They are both struct list_el* pointers, so I can't see the problem.

struct list_elt * temp;
temp = l->first;

Running my sample code gives me an infinite loop:

user@machine:~$ gcc question.c 
user@machine:~$ ./a.out | head
append(): l->first->val: 30, l->first->next == NULL: 1
main()  : l->first->val: 30, l->first->next == NULL: 1
print() : l->first->val: 30, l->first->next == NULL: 1
print() : temp->val: 30, temp->next == NULL: 0
print() : temp->val: 30, temp->next == NULL: 0
print() : temp->val: 30, temp->next == NULL: 0
print() : temp->val: 30, temp->next == NULL: 0
print() : temp->val: 30, temp->next == NULL: 0
print() : temp->val: 30, temp->next == NULL: 0
print() : temp->val: 30, temp->next == NULL: 0

Here's question.c. I'm sorry I couldn't narrow it down further, every time I do the problem seems to magically go away.

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

struct list_elt {
    int val;
    struct list_elt * next;
};
struct linked_list {
    struct list_elt* first;
};

void print(const struct linked_list * l);
struct linked_list* new_list(void);
void append(struct linked_list* l, int value);


main()
{
    struct linked_list * l;
    l = new_list();
    append(l, 30);
    printf("main()  : l->first->val: %d, l->first->next == NULL: %d\n", l->first->val, l->first->next == NULL);
    print(l);
}

struct linked_list* new_list()
{
    struct linked_list* l;
    l = (struct linked_list*) malloc(sizeof(struct linked_list));
    l->first = NULL;
    return l;
}

void append(struct linked_list* l, int value)
{
    struct list_elt el = {0, NULL};
    el.val = value;
    el.next = NULL;
    if (l->first == NULL) {
        l->first = (struct list_elt*) &el;
        printf("append(): l->first->val: %d, l->first->next == NULL: %d\n", l->first->val, l->first->next == NULL);
    } else {
        printf("append(): Unimplemented\n");
    }
}


void print(const struct linked_list * l)
{
    printf("print() : l->first->val: %d, l->first->next == NULL: %d\n", l->first->val, l->first->next == NULL);
    struct list_elt * temp;
    temp = l->first;
    while (temp != NULL) {
        printf("print() : temp->val: %d, temp->next == NULL: %d\n", temp->val, temp->next == NULL);
        temp = temp->next;
    }
    printf("\n");
}

Thanks.

You are allocating your list_elt on the stack of the append function. this pointer is invalid once the function returns, and should no longer be referenced.

You should allocate your elements with malloc instead.

In append(), you are setting l->first to the address of a local stack variable, el. But el goes out of scope as soon as append() returns, so any attempt to dereference l->first afterwards is undefined behaviour.

(What follows is a very simple explanation, so simple that it's arguably unprecise. I went for simplicity, but you can learn more by googling about the stack and the heap in C).

There are basically two ways to create things and store them in memory: use the stack, or request memory to the Operating System.

The stack is temporary memory, used to keep track of the current execution context. Every time you call a function, a sector of the stack is used to store all the local variables. This is what you're doing in append(): using temporary memory located in the stack to store your new list node.

Anything in the stack (any local variables) is considered lost in time and space when you return from the function, since the current execution context is no longer relevant, and this memory is reused for another context (the next function call, for example). This is unavoidable, it's a consequence of how this memory is managed, and it occurs outside your direct reach in C (or rather, you shouldn't mess with it).

When you want to store something outside this local scope, relevant to your program in general and not just to the current execution context, you must request additional memory to the operating system -- this temporary stack won't do. There are numerous functions for this -- check out the simplest, called malloc().

Hope it helped. Cheers!

Your append function is wrong. It is allocating el on the stack, and using the pointer in your list. The problem with this is that the memory is will be overwritten with garbage as soon as another function is called after append, in this case, print. Instead, allocate with malloc, something like:

int append(struct linked_list* l, int value)
{
    struct list_elt *el = malloc(sizeof(struct list_elt));

    if (el)
    {
        el->val = value;
        el->next = NULL;
        if (l->first == NULL) {
            l->first = el;
            printf("append(): l->first->val: %d, l->first->next == NULL: %d\n",
                   l->first->val, l->first->next == NULL);
        } else {
            printf("append(): Unimplemented\n");
        }

        return 0;
    }
    else
    {
        return 1;
    }
}

In append, your element is created on stack and referenced later : this is wrong. When your function exits your el variable does not exist any more. You need to malloc your list_elt.