Variable as reference not staying

Question

So i read this thread and many others: Function does not change passed pointer C++

Yet i still can't solve my issue. I have a function declared like this:

void test(list<int*> *listNodes){
    int v=5;
    (*listNodes).push_back(&v);
    (*listNodes).push_back(&v);
    (*listNodes).push_back(&v);
    for(int a = 0; a < (*listNodes).size(); a ++){
        std::list<int*>::iterator i = (*listNodes).begin();
        advance(i, a);
        int *totry = *i;
        cout << *totry;
        cout << ",";
    }
}

Wich works, and prints fine, by the i mean: the listNodes variable has 3 elements, all 5's. However, when this functions returns, the values are not updated. By that, i mean that the variable has trash. I call this function in another one like this:

void create(list<int*> listNodes){
    test(&listNodes);
    for(list<int*>::const_iterator it=listNodes.begin();
     it!=listNodes.end(); it++){
        int *show=*it;
        cout << *show << '\n';
    }
}

Again, in this function, the cout will output memory garbage instead of outputting the 3 fives. Any ideas on how should i proceed to, when the function test comes back, i have the list populated?

That call won't even compile. Don't lie to us: present your [MCVE]. — Lightness Races in Orbit, Sep 18 '16 at 00:57
Define "values are not updated". By the caller, do you mean `create()`, or whatever called `create()`. Since `create()` receives this list by value, all that `test()` is doing is modifying a copy of the original list, and `create()`'s caller will not see any changes. — Sam Varshavchik, Sep 18 '16 at 00:58
Which values are not updated? And when you say "when it goes back to the calle", you mean when `test` returns to `listnodes`? It would really help to see real code. (If you mean when `create` returns, then the answer is obvious -- `create` takes `listNodes` by value.) — David Schwartz, Sep 18 '16 at 01:05

score 1 · Accepted Answer · answered Sep 18 '16 at 03:12

The problem I believe you're thinking about (as opposed to other problems in this code) is not actually what you're thinking. The list DOES maintain its values, the problem is that the values it has are pointing to garbage memory.

When you do this:

int v=5;
(*listNodes).push_back(&v);
(*listNodes).push_back(&v);
(*listNodes).push_back(&v);

You are putting three copies of the address of v into the list. You have declared v as a stack variable that only exists for the duration of this function. When you print the values pointed to by the elements of listNodes inside function test, that variable still exists in that memory location.

When you later print out the values pointed to by the elements of listNodes in function create, that variable has gone out of scope and has been used by something else, hence the garbage.

Here are two possible solutions to consider:

Use list<int> instead of list<int *>. If all you want to do is store a list of integers, this is the way to go.

If, on the other hand, you really need to store pointers to those integers, you'll need to allocate memory off the heap:

int* v = new int(); // allocate an int on the heap
*v = 5;             // store 5 in that int
(*listNodes).push_back(v);  // save the pointer to the allocated
                            // memory in *listNodes
etc

This is not very good in terms of modern c++, however, as you generally don't want to be handling raw pointers at all, but it illustrates the point I think you are struggling with.

score 0 · Answer 2 · answered Sep 18 '16 at 01:14

In this code,

void create(list<int*> listNodes){
    listNodes=teste(&listNodes);

… the formal argument listNodes is passed by value. That means that the function receives a copy of whatever was passed as actual argument in a call siste. Changes to this copy will not be reflected in the actual argument.

The call to teste won't call the test function, since it's a different name.

In a way that's good, because test is declared as a void function so it can't return anything.

But it's also bad, because it means that a very crucial piece of your code, the teste function that's actually called, isn't shown at all in your question.

The test function,

void test(list<int*> *listNodes){
    int v=5;
    (*listNodes).push_back(&v);
    for(int a = 0; a < (*listNodes).size(); a ++){
        std::list<int*>::iterator i = (*listNodes).begin();
        advance(i, a);
        int *totry = *i;
        cout << *totry;
        cout << ",";
    }
    printf("\n");
}

… has a lot wrong with it.

Starting at the top, in C++ the pointer argument

void test(list<int*> *listNodes){

… should better be a pass-by-reference argument. A pointer can be null. That doesn't make sense for this function, and the code is not prepared to handle that.

Next, in

    int v=5;
    (*listNodes).push_back(&v);

… the address of a local variable is pushed on a list that's returned. But at that point the local variable ceases to exist, and you have a dangling pointer, one that used to point to something, but doesn't anymore. If the caller uses that pointer then you have Undefined Behavior.

Next, this loop,

for(int a = 0; a < (*listNodes).size(); a ++){
    std::list<int*>::iterator i = (*listNodes).begin();
    advance(i, a);

… will work, but it needlessly has O(n²) complexity, i.e. execution time.

Just iterate with the iterator. That's what iterators are for. Iterating.

Summing up, the garbage you see is due to the undefined behavior.

Just, don't do that.

… should better be a pass-by-reference argument. A pointer can be null. But what if i garantee in another part of the program that that pointer won't be NULL? In that case, the variable still exists right? What i still don't get is, why does the variable loses it's values after being sent by reference? In test(&listNodes); i'm sending a list (supose not Null), by reference, am i not? — Joao Carlos, Sep 18 '16 at 01:20

Variable as reference not staying

2 Answers2