Why my simply linked list doesn't work after first node?

Question

I write this code to automatically gets values and put them in a linked list but it just maintain first one and replace any new one in second node and doesn't make third, forth or ... nodes.

    #include <iostream>

    using namespace std;

    struct node
    {
        int a;
        struct node *next;
    }zero;

    node *first = NULL;


    int main()
    {
        int t;
        for (int i = 0;i < 10;i++)
        {
            node *n = first;
            cin >> t;
            if (first == NULL)
            {
                node temp;
                temp.a = t;
                first = &temp;
                temp.next = NULL;
            }
            else
            {
                while ((*n).next != NULL)
                {
                    n = (*n).next;
                }
                node tt;
                tt.a = t;
                (*n).next = &tt;
                tt.next = NULL;
            }
        }
    }

I inserted 28. my first node data=28. I inserted 57. my second node data=57. I inserted 120. my second node data=120. ...

What did you observe when you start your program in the debugger and step through the code line by line? — πάντα ῥεῖ, Dec 16 '18 at 16:03
I know it comes up in every code review, but [don't use `using namespace std`](https://stackoverflow.com/q/1452721/1813169) — MTCoster, Dec 16 '18 at 16:06
Undefined behavior due to the dereferencing of dangling pointers (due to storing pointers to local variables). — Algirdas Preidžius, Dec 16 '18 at 16:08
It’s usually a bad idea to store what `&` gives for later use. This is no exception. The web is full of examples of linked lists, and so are books. — molbdnilo, Dec 16 '18 at 16:08
You use pointers to local variables. You never allocate new memory. — Dmytro Dadyka, Dec 16 '18 at 16:12

eerorika · Accepted Answer · 2018-12-16T16:44:55.670

2

node temp; within a compound statement declares an automatic variable. first = &temp; assigns first to point to the automatic variable.

Automatic variables are destroyed automatically at the end of the scope - the compound sub-statement of the if-statement in this case. After that, first no longer points to a valid object - it becomes a dangling pointer.

When in the next iteration you do node *n = first and then *n in the else-branch, you indirect a dangling pointer. The behaviour of the program is undefined.

edited Dec 16 '18 at 16:44

answered Dec 16 '18 at 16:17

eerorika

232,697
12
197
326

I don't have this problem (I checked it line by line) but my problem is that after making first node every time the temp is a special memory space which doesn't change. – P7928Z Dec 16 '18 at 16:29
1

@P7928Z of course you have. In line `node temp;` you allocate memory. In line `first = &temp;` you store the address of this memory. After this the memory is deallocated. In line `while ((*n).next != NULL)` you dereference this memory. That's undefined behaviour – Thomas Sablik Dec 16 '18 at 16:38

Why my simply linked list doesn't work after first node?

1 Answers1