Copying values from one vector to another (from book)

Question

Consider this piece of code.

#include <iostream>
#include <vector>

using namespace std;


int main()
{
    vector <int *> test;
    vector <int *> v;
    int *a = new int;
    int *b = new int;
    *a = 1;
    *b = 2;
    v.push_back (a);
    v.push_back (b);
    for (int i = 0; i < 2; ++i)
    {
        int n = *v[i];
        test.push_back (&n);
    }
    cout << *test[0] << " " << *test[1] << endl;
    delete a;
    delete b;
    return 0;
}

The problem's statement is:

"Given this code, answer the following questions:

1. Why does "test" vector contain only 2's?

2. How can we change for loop to copy properly (only code inside for loop)?"

I couldn't answer any of these questions, so a little bit of help will be appreciated.

Thanks in advance.

Answer 1

That code introduces dangling pointers. The body of the loop looks like this:

{
    int n = *v[i];
    test.push_back (&n);
}

The local variable n loses scope as soon as the loop body ends, so the pointer &n is now a dangling pointer. If it happens that test contains only 2's, that's just what randomly came out of what is undefined behavior.

If you want to "properly" copy the data over to test, you can change the for loop body to this:

{
    int* n = new int;
    *n = *v[i];
    test.push_back (n);
}

Please take the "properly" with a grain of salt...

Answer 2

The first question is a trick question: The vector contains pointers to a variable that no longer exists, and dereferencing that could cause pretty much any output. I imagine on some machines and compilers it prints all 2s however.

I can't understand what the exercise is trying to do (why does it use vectors of pointers for example) so I can't really help with how to solve the problem.

One way you could do it is by making test store by value:

First change the test vector to vector <int> test;

Then change the push_back to something like test.push_back (n); and finally the print statements to remove the now-unneeded * operators.

EDIT for comment:

First, I'm suspect of this book: It shouldn't be demonstrating undefined behavior or raw pointers to single builtin types. But you can change your loop body if you want:

 for (int i = 0; i < 2; ++i)
 {
     int* n = new int;
    *n = *v[i];
     test.push_back (&n);
 }

Note that both this will cause a memory leak unless you later delete those pointers, a problem that storing by value eliminates.

Answer 3

You push two the same pointers to n into test array. n equals the last element of your first array. Note that after control flow exited the loop, all pointers to n become invalid. So, in fact your test array contains invalid pointers, not pointers to 2s.

You should create a copy of each integer:

int* n = new int(*v[i]);
test.push_back (n);

Note also that you have memory leak here. Each int created using new should be later destroyed using delete.

Answer 4

1) I think that the premise of the question is faulty. The loop adds two elements to test, each contains the address of the automatic variable n, the scope of which is limited to the body of the loop. It's not guaranteed that n will be allocated the same memory location in both passes through the loop, but I suppose that it's likely that most compilers will reuse the same location in both passes.

Moreover, n is out of scope at the output statement. So referencing the pointers in test to those memory locations is undefined. Again, there's a good chance that they will still contain the values assigned in the loop.

So, only if the same location gets reused for n in the second pass of the loop and that location has not been overwritten at the time the output statement is executed, will the output be "2 2". There is no guarantee of either of these premises.

2) To get the output "1 2" without changing anything outside the loop, one could change the definition of n to int& n = *v[i], which would be a single character change from the given code, though the end result is rather strange.

A simpler solution would be to eliminate the temporary n and simply test.push_back(v[i]).