Does delete need to be used for new'd arrays created in vector?

Question

I'm trying to create some dynamic arrays for a void** data array.

std::vector<void*> data;
data.push_back(new double[1024]);  // array of doubles
data.push_back(new short[1024]);   // array of shorts

For clean up should I just use

data.clear();

Or do each of the new(s) need to be deleted and if so, how is that done?

Answer 1

The rule of thumb is delete everything you have allocated with new As you pass a reference for an array allocated with new[], you'll have to delete it with delete[]. But think twice, you are making bad use of the allocated reference.... because you don't save it anywhere, and vector doesn't also, it just can navigate another container (if iterable) to make copies of all the other container elements (that's a convenience method to add several items in a bunch) so you are loosing the array reference and creating a memory leak. **and you are using an array<void*> which is not compatible with double type (void* is not the same type as double) vector doesn't have a constructor to pass it an array of base type elements. Should it have a constructor like

vector::vector<T>(const T v[], const size_t sz);

then you could initialize a vector<double> with:

const double initial[] = { 109.8, 98.5, 87.5 };
const size_t initial_sz = sizeof initial / sizeof initial[0];
vector<double> v(initial, initial_sz);

but you don't have this constructor, so you'll have to use another approach, like:

vector<double> v;
v.push_back(109.8);
...
v.push_back(87.5);

of if you have another iterable container, to get the iterator and pass it to it:

vector<double> v(another_double_container.begin(), another_double_container.end());

but a double [] array has not such iterators, so you cannot follow this approach with an array of doubles.

Another problem is that you try to include different types in your vector, and it only allows same type instances. You cannot (and the compiler doesn`t allow you to do) store different types into a vector (as it stores elements as in an array, contiguously)

Answer 2

For clean up should I just use

data.clear();

That will remove all pointers from the vector. If any of those are the only copies that point to their respective dynamic objects, then those objects can no longer be destroyed nor can their memory be released. This is called a memory leak.

Or do each of the new(s) need to be deleted

Yes. For each time a new[] expression is executed, there must be exactly one delete[]. If there is no delete[], then there is a leak.

and if so, how is that done?

You simply delete each pointer. You have to be extra careful to not accidentally overwrite or remove an element of that vector without deleting it first. You must also be careful to not delete same element (or copies of it) more than once.

Also, it is not possible to delete a pointer to void. A pointer to void can be converted to any data pointer type, but you must know the type of each element. A working example

delete[] static_cast<double*>(data[0]);
delete[] static_cast<short* >(data[1]);

It is usually a very bad idea to store bare pointers that own memory - even more so to use pointers to void for that purpose. A better design is to store the dynamic arrays in RAII container such as std::vector:

std::vector<double> doubles(1024);
std::vector<short>  shorts (1024);
std::vector<void*>  data{doubles.data(), shorts.data()};

This way the arrays are owned by vectors, and memory is managed by the code of std::vector. You do have to mind that doubles and shorts must not be destroyed as long as elements of data still point to them.

Answer 3

No, this won't work

Sorry to be direct, but the use of void* as you did is a very bad practice. This C like shortcut will not work correctly in C++, at least not when you use array of anything else that fundamental types !

Because casting a pointer to an object to a void pointer, doesn't allow the compiler to generate the code required for the C++ object lifecycle.

So let's use a helper class to demonstrate what's wrong:

class C1 {
public: 
    C1() { cout<<"  C1 object constructed"<<endl; } 
    ~C1() { cout<<"  C1 object destroyed"<<endl; }
};

Here an online demo of the problem that I'll explain step by step hereafter.

Case 1: manual allocation with a pointer

Every C1 allocated in the array, will be created and destroyed, with the following simple code:

C1 *p = new C1[2];   // every new []
delete[] p;          // requires delete[]

Case 2: manual allocation pushed into a vector of pointers:

When a vector gets destroyed or gets cleared, so does its content. However, with a vetor of pointers, the pointer gets destroyed and not the object that is pointed to:

So this will not work; it will leak memory:

   vector<C1*> v; 
   v.push_back(new C1[2]);   // C1 objects are created 
   v.clear();                // pointers are lost but C1 objects are not derleted.  

If you insert the following statement before the clear, everything is ok again: all C1 that are created are destroyed:

   for (auto &x:v)
       delete[]x; 
   v.clear(); 

Case 3: the void* alternative is doomed to fail:

If we take the working code above but use a casting to void*, the C1 objects are created, but they are not destroyed:

std::vector<void*> data;
data.push_back(new C1[2]);   // yes, creating works and ponter is in ector
for (auto &x:data) 
       delete[]x;           // ouch, deletes a void* so it doesn't know that it's a C1 object
data.clear();

How to solve it ?

Well, you need a common denominator for the objects that you store on your vector, for example:

• use inheritance and a common base type for all the elements.
• use a union with a type selector to be able to know the type of the objects that are pointed to
• use boost::variant objects

If you have a common base type you could consider the vector of vector alternative to get rid of the manual memory management.

Answer 4

Yes, for your case you'd have to explicitly free them via operator delete. However, since you seem to want to store pointers to arrays of different type, how do you plan to differentiate between them?

Ignoring that fact, your sample code screams std::unique_ptr