tags:

views:

117

answers:

4

Let's say I have a structure named vertex with a method that adds two vertices:

struct vertex {
    float x, y, z;

    // constructs the vertex with initial values
    vertex(float ix, float iy, float iz);

    // returns the value c = this + b
    vertex operator+(vertex b);
};

vertex::vertex(float ix, float iy, float iz){
    this->x = ix; this->y = iy; this->z = iz;
}

vertex vertex::operator+(vertex b){
    vertex c;
    c.x = this->x + b.x;
    c.y = this->y + b.y;
    c.z = this->z + b.z;
    return c;
}

In another calling function I want to add two vertices together and add the result to a vector<vertex*>. When is it safe to use the returned value to add to the given vector? If never, how would I implement it?

For example,

vector<vertex*> v;
vertex a(1, 2, 3);
vertex b(4, 5, 6);
v.push_back(&(a + b));
+2  A: 

This is not safe, since you are storing a pointer to an automatic or temporary variable, which will be reclaimed when the current function terminates.

Mixing dynamically allocated objects with automatically-allocated ones is a serious risk. Sometimes the best strategy is to completely disallow automatic allocation (e.g., by making the constructor private and using a factory method to create new instance). You would then be responsible for eliminating these at some point.

A second option (not necessarily what you want) is to do everything by value. Have a vector of Vertex rather than Vertex*, and just have the vertices copied when they are stored. The way your class is written, all the fields are primitives, so this might be good enough and you wouldn't run into performance or deep-copy semantic problems.

Uri
That clarifies a bit, but given that, whats a safe way to do this without returning pointers from the operator function? I want to be able to chain different operators together - ie a + b + c.
Asuah
You could still do this if you use pass-by-value and return-by-value semantics. It will be safe. Just do a vector of vertex objects rather than a vector of pointers to vertices.
Uri
Use smart pointer is another option here.
J.W.
Yes, of course. Good point.
Uri
+1  A: 

It's never save, since you add a pointer to a temporary object to the vector. That temporary object will be destroyed after the line is executed, leaving the vector with an invalid pointer.

You have two possibilities. Either you don't store pointers in the vector and use a vector<vertex> instead, or you explicitly allocate a new copy of the temporary object when you add it:

v.push_back(new vertex(a+b));
sth
+1  A: 

Another alternative is insert smart-pointer into std container, such as boost:shared_ptr, since the shared pointer will take care of the memory management for you. However, you need return a shared pointer from Shared_Ptr vertex::operator+(vertex b), that means the return value will still be on the heap.

If you are not familiar with boost:shared_ptr, then just to do everything by value as suggested by the other posts. This is actually stl standard practice.

J.W.
A: 

This is not safe, because the vertex values that you are adding to the vector are not on the heap. The returned vertex objects are on the stack, so their memory locations may be overwritten once the current function terminates. If the vector (or a copy of it) persists after the current function ends, there is no guarantee that its pointers will still be referring valid vertex objects.

The most dangerous part of this situation is that those vertex objects may survive in memory long after the function in which they were created ends. I saw an example of this once, where a student filled a vector with pointers to value objects in a very long constructor. The vector was a member field in the object, so it still existed after the constructor ended. Because the constructor was so long, the value objects that the vector pointed to were not overwritten until halfway through a different function. Watching the vector's contents in a debugger created the perplexing illusion that the objects were spontaneously corrupting in memory.

Unless you really need to store those vertices as pointers, the safest strategy is to store them as value objects in the vector: vector<vertex> instead of vector<vertex*>. You will still be able to chain the different operators together; in fact, it should be easier to change them together as value objects because you will not constantly need to dereference pointers.

Eric L