How to prevent a temporary from going out of scope?

2.2k views Asked by At

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)....

1

There are 1 answers

4
Remy Lebeau On

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];
};