Accesing struct value, changes struct value

Question

I was trying to understand how pointers work in c when i came across this weird problem.

Now, I wanted to build a linked list. The first thing I did was adding the add function. Once the function add a node to the last node of the list(which it does successfully)

typedef struct linkedLists{
    int x;
    struct linkedLists *next;
    //int (*add)(int) = add;
}linkedList;

void addF(linkedList *l, int y){
    linkedList adder = {.x=y};
    l->next = &adder;
    return;
}

int main(int argc, char** argv) {
    linkedList list = {.x=2,.next=NULL};
    printf("%d\n",list.x);

    addF(&list,3);

    printf("%d\n",list.x);
    // If you comment this line the result changes to what it's  
    //expected

    printf("%d\n",(*list.next).x);

    return (EXIT_SUCCESS);
}

If I run

printf("%d\n",(*list.next).x);

I get 3, which is desired. However, if I run

printf("%d\n",list.x);
printf("%d\n",(*list.next).x);

I get: 2 Random number

`l->next = &adder;` overwrites the `->next` node each time with the same `linkedList adder` node which is declared local to `addF()` and ceases to exist when `addF()` returns. — David C. Rankin, Mar 25 '19 at 00:21

chux - Reinstate Monica · Answer 1 · 2019-03-25T01:13:02.250

1

l->next is assigned a pointer value that will soon become invalid once addF() ends. Result: Undefined behavior.

void addF(linkedList *l, int y){
    linkedList adder = {.x=y};  // adder is local
    l->next = &adder;  
    return;
}

What makes more sense is to walk the linked list (assuming it has at least one node) and append a new node.

void addF_alt(linkedList *l, int y) {
  while (l->next) {
    l = l->next;
  }
  l->next = malloc(sizeof *(l->next));
  if (l->next) {
    l = l->next;
    l->next = NULL;
    l->x = y;
  }
}

What is more common is to do all node appending via a function.

TBD code

edited Mar 25 '19 at 01:13

answered Mar 25 '19 at 00:22

chux - Reinstate Monica

143,097
13
135
256

1

That makes 3 answers in 15 seconds all pointing out the same issue -- here's to consistency `:)` – David C. Rankin Mar 25 '19 at 00:23
Thank you very much, this was very helpful! – miraunpajaro Mar 27 '19 at 21:58

score 0 · Accepted Answer · answered Mar 25 '19 at 00:21

0

In this function

void addF(linkedList *l, int y){
    linkedList adder = {.x=y};
    l->next = &adder;
    return;
}

You leak a pointer (&adder) to an object, which lifetime ends at return of the function. You have to either use the static keyword

static linkedList adder = {.x=y};

or allocate space dynamically with malloc() & co.

Howeber, list.x should always evaluate to 2 in both cases.

answered Mar 25 '19 at 00:21

Ctx

18,090
24
36
51

2

How would making it `static` help. There will only be one `static linkedList adder = {.x=y};` so future additions would overwrite `l->next` each time `addF()` is called. Your 2nd option would be fine. – David C. Rankin Mar 25 '19 at 00:25
@DavidC.Rankin `addF()` is only called once, so in the case of the program shown, it would help. If it is called more than once, then this option isn't viable of course. – Ctx Mar 25 '19 at 00:26
1

Yes, you are correct there. In the case the list is limited to a 2-node list, `static` will work for the 2nd node. – David C. Rankin Mar 25 '19 at 00:58

Accesing struct value, changes struct value

2 Answers2