initialization of ints vs initialization of strings using pointers, c++

Question

I recently ran into an issue while experimenting with linked lists. When I use a function to link up a new node that has a string in its data field it doesn't work. By this I mean that when the function (linkin() see below) returns , the string (which was local to the function) is destroyed and so the string field appears to be uninitialized.

However, when I do this exact same operation with an int it seems to work just fine. The code I used is below (its the int version, but make val a string instead of an int to see the other version). Could someone explain to me what is happening?

Thanks!

struct testlist {
    int val;
    testlist *next;
};
void linkin ( testlist *a );

int main() {

    testlist test;

    linkin(&test);
    cout << test.next->val <<endl;
}


void linkin ( testlist *a )
{
  testlist b;
  b.val=1;
  a->next = &b;
  cout << a->next->val <<endl;
}

Oh, well, thank you for showing us the code that is working and hiding the one that is broken. We can certainly guess what is wrong with the code we haven't seen by looking at different code that works. — R. Martinho Fernandes, Oct 29 '13 at 15:35
Actually, nevermind. This code is broken too. It just works by accident. See http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope/6445794#6445794 — R. Martinho Fernandes, Oct 29 '13 at 15:35
`a->next = &b;` is wrong! `&b` will be invalid after the `linkin()` function exits! — πάντα ῥεῖ, Oct 29 '13 at 15:36
That code throws an automatic variable into a linked list, then attempts to reference it outside the scope of its demise. The term "working" is in the mind of the undefined-behavior-invoking beholder. — WhozCraig, Oct 29 '13 at 15:36
Use a constructor and as other point out a linked list uses the heap not stack allocation. — andre, Oct 29 '13 at 15:38
@andre it doesn't have to. Linked lists in automatic storage are not unheard of, but the usage of said automatic var by the OP in this case is most-certainly wrong. — WhozCraig, Oct 29 '13 at 15:39
@WhozCraig In that case, what's the best way to add a new node without actually writing everything into the main function? By passing a new node into the add function? Thanks for the help — Adam, Oct 29 '13 at 15:47
@Adam a static linked list can take on many forms. For example, an array of struct with the data element, and two indexes used as lookups in the same array. You could do as you mentioned, but more. An array of struct with `next` that point to different elements in the array. *None* of these I advise to anyone until they have a firm understanding of *dynamic* linked list management. — WhozCraig, Oct 29 '13 at 15:54

masoud · Accepted Answer · 2013-10-29T16:03:53.577

testlist b;
a->next = &b;

a->next is pointing to a local temporary object which will be destroyed right after returning from function. It invokes undefined behavior after dereferencing it out of the function.

It's undefined behavior, sometimes it works, sometimes not.

Also in C++ there is a linked-list: std::list. On the other hand, you can use smart-pointers such as std::unique_ptr instead of bare pointers. I've written a smart-pointer based of your container:

struct List
{
    int val;
    unique_ptr<List> next;
};

void linkin (List &a)
{
    unique_ptr<List> b(new List);
    b->val = 1;
    a.next = move(b); // After move you can't use b anymore
    cout << a.next->val << endl;
}

int main()
{
    List test;
    linkin(test);
    cout << test.next->val <<endl;
}

score 0 · Answer 2 · answered Oct 29 '13 at 15:40

0

Straight away, in linkin() I can see you're storing a pointer in a to an object that is going to go out of scope - ie: be destroyed.

There are reasons why this might appear to work with an int but not a complex type like a std::string. Despite appearances this is broken code - try rewriting linkin():

void linkin ( testlist& a )
{
   testlist* b = new testlist;
   b->val=1;
   a->next = b;
   cout << a->next->val <<endl;
}

answered Oct 29 '13 at 15:40

Grimm The Opiner

1,778
11
29

Not so sure if using `new` without a smart pointer is a really good advice! – πάντα ῥεῖ Oct 29 '13 at 16:06
@g-makulik You simply, absolutely, categorically, fundamentally, **cannot** be frickin' serious. – Grimm The Opiner Oct 29 '13 at 16:09
**I'am** serious about this! – πάντα ῥεῖ Oct 29 '13 at 16:10
Have a look at MM's answer to see what I mean. Again: **I am** serious about this point, and what you show in your answer is **not** good advice! – πάντα ῥεῖ Oct 29 '13 at 20:09
I'm never going to agree with the idea that good coding must involve assuming oneself incapable of managing heap memory. There are some occasions where use of various smart pointers can be very helpful - heap allocation inside a try block for example - but they are certainly not a necessity in a simple linked list. Further to that, my answer illustrated perfectly why his code didn't work and why, that he needed to use the heap, managing it from there is up to the questioner – Grimm The Opiner Oct 30 '13 at 08:33

initialization of ints vs initialization of strings using pointers, c++

2 Answers2