How to prevent a temporary from going out of scope?

Question

In the following case my object goes out of scope and I access an invalid pointer:

struct Animal
{
    char* buffer;
    Animal() { buffer = new char[100]; }
    ~Animal() { delete[]buffer; }
};


int main()
{
    vector<Animal> list;

    {
        Animal dog;
        list.push_back(dog);
    }

    list[0].buffer[50] = 7;  // Buffer's been deleted, can't access it
 }

I guess the best way to prevent this would be to construct the Animal object in place in the vector, but I don't know how to do that. I thought about doing:

list.push_back(Dog());

But this still creates a temporary, unless it's optimised away, and I'd rather not rely on that because in another place (another compiler) it may not do the same thing.

Edit: Thanks to Remy Lebeau I've learned you can construct a vector element directly in the vector, no temporaries, no copying, with the function:

template< class... Args >
void emplace_back( Args&&... args );

I don't know how the variadic template arguments work, but the description is:

Appends a new element to the end of the container. The element is constructed through std::allocator_traits::construct, which typically uses placement-new to construct the element in-place at the location provided by the container. The arguments args... are forwarded to the constructor as std::forward(args)....

I don't understand the question. Are you asking you to put stuff that's not default constructible into a `std::vector`? — Baum mit Augen, Jan 02 '17 at 02:46
"Afterwards I use my vectorOfAnimals[0] and access an invalid pointer" What do you mean about the "invalid pointer"? Where is it? — songyuanyao, Jan 02 '17 at 02:48
@titone: good idea. My suggestion: ask a concrete question about real, compilable code. Those are much easier to address. — rici, Jan 02 '17 at 02:57
@TitoneMaurice You can't prevent `dog` from being destroyed. Note it will be copied into the vector when `push_back`ed, so you should apply the [rule of three](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). — songyuanyao, Jan 02 '17 at 03:04
@songyuanyao So you're saying to define copy constructor to copy the entire buffer over? That would be fine I guess, but if you wanted to avoid copying every single member over (some could be quite big), is there a way to avoid it? — Zebrafish, Jan 02 '17 at 03:08
@TitoneMaurice You can use move semantics (i.e. define move constructor and assignment operator) to avoid copy. — songyuanyao, Jan 02 '17 at 03:12
@TitoneMaurice You want to prevent these problems with using vector, but IMO you're going in the wrong direction by concentrating solely on vector. The vector just exposed the problems with your class. Either fix the copying issue (which is easily done by using `std::vector` or `std::string`), or make the class non-copyable by making the copy operations private and unimplemented, or use the `= delete` syntax of C++ 11. — PaulMcKenzie, Jan 02 '17 at 03:28

Remy Lebeau · Answer 1 · 2017-01-02T03:23:31.563

3

The problem is not that the temporary is going out of scope. The real problem is that Animal is violating the Rule of three by not implementing a copy constructor or copy assignment operator.

When you push the temporary into the vector, a copy of the object is made, but the compiler-generated copy constructor simply copies the pointer as-is, it does not allocate a copy of the memory. So, when the temporary gets destroyed, the memory is deallocated in the destructor, and the copy is left with a dangling pointer to invalid memory.

Add a copy constructor to allocate new memory:

struct Animal
{
    char* buffer;

    Animal() {
        buffer = new char[100];
    }

    Animal(const Animal &src) {
        buffer = new char[100];
        std::copy(src.buffer, src.buffer+100, buffer);
    }

    ~Animal() {
        delete[] buffer;
    }

    Animal& operator=(const Animal &rhs) {
        if (this != &rhs) {
            std::copy(rhs.buffer, rhs.buffer+100, buffer);
        }
        return *this;
    }
};

Alternatively, use a std::vector instead of a raw pointer, and let the compiler generate suitable copy constructor, copy assignment operator, and destructor for you:

struct Animal
{
    std::vector<char> buffer;
    Animal() : buffer(100) {}
};

Or, simply allocate the memory statically instead of dynamically:

struct Animal
{
    char buffer[100];
};

edited Jan 02 '17 at 03:23

answered Jan 02 '17 at 03:19

Remy Lebeau

454,445
28
366
620

Thanks. This is my fault for not being clear. I want to learn how to avoid the copy. I thought there are ways to construct an object in a vector. – Zebrafish Jan 02 '17 at 03:23
@TitoneMaurice nothing in your question even hints at your goal of avoiding a copy. Prior to C++11, you cannot avoid the copy. In C++11 and later, you can use `emplace_back()` instead of `push_back()`. But you should still follow the Rule of Three (and the Rule of Five in C++11 and later) whenever a class manages resources that cannot be shallow-copied. – Remy Lebeau Jan 02 '17 at 03:25
vector::emplace_back() isn't working. When I do myVector.emplace_back(myStruct(arg1, arg2)); it first constructs myStruct, and then copy constructs a new one in the vector. Grrrr, one thing at a time. If emplace_back() isn't working I guess I should ask a question about that. – Zebrafish Jan 02 '17 at 03:33
1

@TitoneMaurice remove the explicit temporary, just pass the constructor arguments by themselves: `myVector.emplace_back(arg1, arg2);` `emplace_back()` will forward the arguments to the type constructor when constructing the new object inplace. – Remy Lebeau Jan 02 '17 at 04:02

How to prevent a temporary from going out of scope?

1 Answers1