-1

I have started learning C++ and as a first project, I am trying to make a linked list application. I have not had any real problems with the linked list concept so far as I have previously worked with it in a different language.

So far, I already have working functions to add items to the list and to print it. In the main() class, I can add items one by one, but when I try to add multiple items via a for or while loop.

I have separate functions to create an item and to add it to (the beginning of) the list:

ListItem createItem(int value)
{
    ListItem x;
    std::cout << &x;
    x.value = value;

    return x;
}

void addBeg(ListItem* &start, ListItem &item)
{
    // Adding to an empty list
    if (start == nullptr)
    {
        item.prev = &item;
        item.next = &item;
    }
    else
    {
        std::cout << std::endl <<"X" &item;
        item.prev = (*start).prev;
        (*item.prev).next = &item;
        item.next = start;
        (*start).prev = &item;

    }
    start = &item;
}

My main() function looks like this:

int main(void)
{
    using namespace std;

    // List is empty at the beginning.
    ListItem* start = nullptr;

    printList(start);

    // This doesn't work:
    int i = 0;
    while (i < 10)
    {
        ListItem a = createItem(i);
        addBeg(start, a);
        i++;
    }

    // This works:
    addBeg(start, createItem(12));
    addBeg(start, createItem(13));
    addBeg(start, createItem(-42));
    addBeg(start, createItem(1));
    addBeg(start, createItem(-85));

    printList(start);

    return 0;
}

I cannot seem to graps why it doesn't work. One reason I have thought of is that ListItem a does not reset in each iteration, but that does not make any sense whatsoever to me. Any help or idea is appreciated.

  • Linked lists work with *pointers*. In the loop you are adding the address of a locally scoped variable, and that address will not be valid outside the scope of that loop. – crashmstr May 01 '15 at 15:33
  • Possible duplicate of [Can a local variable's memory be accessed outside its scope?](http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – πάντα ῥεῖ May 01 '15 at 15:39
  • I am not adding the address - createItem returns a ListItem object. But even if I was, why is it that it works just fine in a single addBeg() statement (shown below the loop)? What is getting done differently in the for loop? – Kristián Leško May 01 '15 at 15:40
  • @KristiánLeško um, yes you are. `&item` in `addBeg` gets the *address* of the referenced object. The scope of `a` is limited to the for loop, and the `a` will be destructed at the end *of each loop*. – crashmstr May 01 '15 at 15:48
  • But you *do* add the address, what do you think the ampersand `&` does in e.g. `&item`? It returns the *address* to where `item` is, and it `item` is a temporary variable that will disappear then you have problems. – Some programmer dude May 01 '15 at 15:48
  • OK, I have noticed a solution in Joachim's answer. Thank you! – Kristián Leško May 01 '15 at 15:53

1 Answers1

1

The createItem function returns by value, and when passing that along directly to another function (like you do with e.g. addBeg(start, createItem(12)) the returned value is temporary. Taking and saving an address of a temporary value will lead to undefined behavior.

The simple solution is to have createItem create a node dynamically of the heap using new, and return a pointer.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Should *createItem* return a ListItem object, as is specified in its definition? Also, if you are right, I do not know why the error you mention only demonstrates when adding items via a for loop. The single line addBeg(...) you quote works just fine. – Kristián Leško May 01 '15 at 15:35
  • @KristiánLeško One problem with undefined behavior is that sometimes it might *seem* to work, in reality is really doesn't. Undefined behavior makes your whole program ill-formed. – Some programmer dude May 01 '15 at 15:39