2

I am getting a warning in my code:

warning: taking address of temporary

I have seen similar questions, but they do not answer my specific problem.

Here is what my code is doing:

vector<A*>* ex_A;

ex_A->push_back( &A());  //I get this warning taking address of temporary

Is this undefined behavior?

I did have this before, which was fine, but i didn't want to worry about deleting memory from the heap.

vector<A*>* ex_A;

ex_A->push_back( new A());

Could some one explain to me what the warning means?

NPE
  • 486,780
  • 108
  • 951
  • 1,012
MWright
  • 1,681
  • 3
  • 19
  • 31

6 Answers6

7

&A() is creating a temporary object which gets destructed on exit of the full expression automagically, while new A() creates a new object on the heap which will live until you manually destroy it using delete.

I should add that if you store objects allocated with new in your vector<A*> and the vector gets destructed, the objects stored inside will not get deleted automatically, thus you will have a memory leak. You can verify this by using valgrind --leak-check=full my_compiled_program on your program, which generally is a good idea.

hochl
  • 12,524
  • 10
  • 53
  • 87
  • Thanks! So when it goes out of scope the vector of pointers will be pointing to a dangling pointer? – MWright Nov 25 '11 at 13:58
  • 1
    A couple of problems with this answer: `A()` doesn't get deleted, it gets destructed. And that doesn't happen on exit of the block, but at the end of the full expression. And of course, the compiler isn't right to issue a warning; it should be a full error (according to the standard). – James Kanze Nov 25 '11 at 15:59
2

Yes, this is undefined behavior. This code:

ex_A->push_back( &A());

will construct a temporary instance of A, then add its address to a vector, then destroy the instance and reclaim the storage occupied by A for reuse. The pointer inside vector will become dangling - there's no guarantee what will be allocated at that address further and using that address is undefined behavior.

sharptooth
  • 167,383
  • 100
  • 513
  • 979
  • Well, it's not undefined behaviour *yet*. For example, you could use the pointers in the collection to seed a random number generator... *dereferencing* any of the pointers would be bad. – Kerrek SB Nov 25 '11 at 14:02
  • @Kerrek SB: No, even reading those pointers values is already UB. – sharptooth Nov 25 '11 at 14:04
  • Oh, interesting. Where's that from? I suppose that allows the compiler to omit the creation of temporaries entirely. – Kerrek SB Nov 25 '11 at 14:06
  • 2
    @KerrekSB The motivation for this rule is in some ancient processors with separate registers for addresses and integers. Some of them would validate the address when it was loaded. In this particular scenario, I don't think there's really a chance of the memory being invalidated, but in others, the possibility existed, and the authors of the standard wanted to support these. Today, it's still undefined behavior, but it's not something I'd worry about too much. – James Kanze Nov 25 '11 at 16:02
1

A() is a temporary variable that will cease to exist at the end of the statement. When you take its address, you get a pointer that will soon become invalid, and the next time you'll try to dereference that pointer, you'll get undefined behavior.

Etienne de Martel
  • 34,692
  • 8
  • 91
  • 111
1

Think about the scope of the data you created. In the first case, you create an object on the stack, store its address and right after, when it gets out of scope, the data pointed by the stored pointer is deleted.

In the second case, where you allocate on the heap, the data is still "alive" after you exit the block and the stored pointer points to valid data.

Barth
  • 15,135
  • 20
  • 70
  • 105
0

ex_A->push_back( &A());

In this line by using the & you are taking an adress of a pointer on the stack, once you will be leaving the scope of your fonction this adress won't be valable so you have to use (new A()) that allocate memory.

Kurt
  • 57
  • 7
-1

As long as your working in C++ you are going to have to look after memory... if you want to avoid that move to sharp!
I think you need to make a desission about who is going to own the items that are in your vector - if its the vector then 

ex_A->push_back(new A());

is fine - but you just have to look after deleting that object when its removed/finished with.
You can't 

ex_A->push_back(&A());

because A() is on heap and will go out of scope and be deleted when the current scope ends. What you want is to keep an object alive and not have to worry about whos going delete it -> you want C# or Java for that.

Ricibob
  • 7,505
  • 5
  • 46
  • 65