4

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?

Christophe
  • 68,716
  • 7
  • 72
  • 138
codebender
  • 469
  • 1
  • 6
  • 23
  • 5
    The real question is how are you going to use this code? How will you know what type is stored in the vector if all you have is a `void *`? – PaulMcKenzie Jan 14 '18 at 20:58
  • The function that requires the void** takes another argument with the array of types. – codebender Jan 14 '18 at 21:01
  • Are you trying to interface to a `C` function or `C`-based API? I see no reason for a design like that for a C++ program. – PaulMcKenzie Jan 14 '18 at 21:04
  • Yes, it's interfacing to a C API and the code is actually using c++/cli, but vector and c++/cli ref classes don't seem mix... – codebender Jan 14 '18 at 21:06
  • 1
    The API may know what types, but your code loses all type information, thus issuing the call to `delete []` will be between awkward coding to impossible. – PaulMcKenzie Jan 14 '18 at 21:09

4 Answers4

4

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.

Community
  • 1
  • 1
eerorika
  • 232,697
  • 12
  • 197
  • 326
  • @codebender the final suggestion that I added in an edit does address that concern. – eerorika Jan 14 '18 at 21:23
  • Can the `delete[] static_cast(data[i]);` be used for custom struct, typedef or even a char** (array of char arrays)? – codebender Jan 15 '18 at 00:08
  • 1
    @codebender sure. If you convert a `custom_struct*` to `void*`, you can convert it back using `static_cast`. Same applies to all data pointers (but not member or function pointers). – eerorika Jan 15 '18 at 00:11
  • Should the internal array elements be addressable like `(static_cast(data[i]))[j];` where j is the internal array index? – codebender Jan 16 '18 at 01:05
  • I would move the last code fragment to the top, it is the real answer to this question – Caleth Jan 16 '18 at 09:16
2

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.

Christophe
  • 68,716
  • 7
  • 72
  • 138
  • So `delete[] static_cast(data[0]);` as suggested in another answer will not work? – codebender Jan 14 '18 at 22:16
  • Yes it will. But The problem is: how do you know which of the vector element is double*, which is int* and which is C1* ? – Christophe Jan 15 '18 at 00:02
  • There is another array of the types that the c API function requires, so they are known and a switch can be used to go through each. – codebender Jan 15 '18 at 00:07
1

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

CookiePLMonster
  • 195
  • 3
  • 11
  • Could you give a little more details on why std::unique_ptr might be better? – codebender Jan 14 '18 at 20:58
  • 3
    Automatic lifetime management, so any time that pointer gets removed from your vector it'll be deleted implicitly. – CookiePLMonster Jan 14 '18 at 20:59
  • 2
    It's certainly a good idea to use unique_ptr to specify the deleter to use. However, could you show an example that allows to combine unique_ptr to different types in the same vector ? – Christophe Jan 14 '18 at 21:49
  • Right, it hasn't occured to me that one can't assume deleting different types via the same deleter isn't valid (would probably work in the case of fundemantal types, but I wouldn't recommend making an assumption like this). I guess in this case `std::variant` works best, then. – CookiePLMonster Jan 14 '18 at 21:52
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)

Luis Colorado
  • 10,974
  • 1
  • 16
  • 31
  • The C API being used has a void** array that needs the different type arrays and there is no way to know at compile time what the array types need to be. What would you suggest instead? – codebender Jan 16 '18 at 13:31
  • Why are you using the C api and tag your question (and use) C++ instead? Did you hear about C++ strong typing and how templates come on to help on this, and the poor typing of plain C? Is this a C question or a C++? Cannot you wrap both types into some `union` or `class` and make the `vector` parameter stronger? Do you insist in having trouble with your code? I have to admit that by now I have no idea of what kind of data you want to store in the `vector`. I thought it was a bunch of floating point values, but now it seems you want to store arrays. – Luis Colorado Jan 17 '18 at 12:41
  • The API is from a vendor and I have no option but to use it. The void** is required by the API to return arrays of different types. I'm using VS2017 and c++/cli to wrap as many parts in strongly typed as I can. The ref class I have most of the variables wrapped in, will not allow the use of a std::vector variable inside, so its construction/destruction is having to be coded within the member function making the call. – codebender Jan 17 '18 at 14:13
  • @codebender, anyway, you had better to use a wrapper class, use a `vector` and, inside, do the proper castings to the right type, so you have only one point of failure, instead of many, spread on all the code. – Luis Colorado Jan 18 '18 at 06:35