CODEWITHSUNDEEP
c++memory-managementvectordynamic-arrays
Jose
Jose

Reputation: 631

C++ memory management: creating a new array from vectors in a function

Suppose I have a non-allocated array of floats say 

float *a;

And then I use a function g where I expect to do the deallocation of the array of a outside the scope of the function (so I am calling g(&a)).

void g(float** f)
{
  std::vector<float> *v = new std::vector<float>(10);
  *f = &v[0]
}

Will this be a problem? I do feel that the allocation for v takes more space than that of a simple array of floats. So if I do delete *f outside the scope of this function, there might still be something left in the memory from the vector v. If so, how can one do this safely? The important thing (in my real function) is that I really need to use a vector inside g and that I really need to allocate *f inside g such that *f has the contents of v and does not disappear outside the scope of g.

Upvotes: 1

Views: 215

Answers (3)

Tony Delroy
Tony Delroy

Reputation: 106116

We can envisage what this...

std::vector<float> *v = new std::vector<float>(10);

...does something like this:

1) At memory address &v std::vector<float>* v
                      |
                      v
2) At memory address v  std::vector<float>
                      |
                      v
3) At memory address v.data() aka &v[0]  floats[capacity]

Of these three memory locations:

1) goes is on the stack and goes out of scope when g() exits

2) sits there until delete v is done, which would never happen in your proposed code

3) may have been allocated by the vector, but the vector's also free to have allocated memory then started putting the float data at some offset into it, so any deallocation attempt using the address of data could result in deletion using an address that wasn't returned by new - that's undefined behaviour. This is more likely than it sounds, as some memory allocated by new[] has to be used to store the number of elements so that delete[] can loop the right number of times calling their destructors - that might be before or after the data. Of course, implementations may also put other things into the dynamically allocated memory: for example, values related to detecting accidental memory access immediately before or after the valid indices.

I wouldn't recommend it, but acknowledge that you could probably look at your vector implementation and work out to delete 3), but it would be non-portable and could break with optimisation levels and other compiler options.

You've no chance of deleting 2) with the code you propose - that might be acceptable leakage depending on your application.

Overall, my advice from best to worst:

1) I know you said you can't but I have to say it: rewrite the code to use/pass std::vector<float> by reference or value if at all possible, completely avoiding any explicit new and delete

2) copy the data from the vector to a plain float array if that's not way too slow for your needs

3) you say you need a std::vector inside g(), perhaps you could explore whether instantiation with an alternative allocator could give you well defined behaviour in terms of "unlinking" the allocated memory or (worse as it implies breaking dynamic resizing for push_back etc) specifying to the vector a new[]-ed buffer to use (but not delete[] on destruction)... I haven't looked into allocators enough to know how feasible this is.

4) you might be able to write a vector-like class with the behaviours you need, preferably a minimal interface just supporting what's absolutely necessary, but if you need extensive vector behaviour it may be best to bite the bullet and copy/modify a Standard vector implementation

Upvotes: 1

Dietmar K&#252;hl
Dietmar K&#252;hl

Reputation: 153840

Yes, it will be a problem: if you allocate a std::vector<float> with new you need to delete a std::vector<float> and there is no way to obtain the container from the address of an element. If I really had to allocate an array of something I'd probably do it like so

template <typename Range>
std::unique_ptr<typename Range::value_type[]> copy(Range const& r) {
    using value_type = typename Range::value_type;
    std::unique_ptr<value_type[]> rc(new value_type[r.size()]);
    std::copy(r.begin(), r.end(), rc.get());
    return rc;
}

Before doing something like that I would try hard to redesign what is being done, though.

Upvotes: 0

Tim
Tim

Reputation: 9172

There's a couple of issues here.

void g(float** f);  // will allocate a new array of floats.

// usage:
float* foo;
g(&foo);  // now foo is allocated.

// reclaim the memory:
delete [] foo;   // is this right?
free foo;        // is this right?

Any time you have a function that returns dynamically allocated memory, you need to know how it was allocated (new, new[], malloc), so that you can release it properly (delete, delete[], free). If you cross the streams, bad things happen.

Now, for you particular section of code:

void g(float** f)
{
  //  This allocates a new vector.  Key words:  'new' 'vector'
  std::vector<float> *v = new std::vector<float>(10);
  *f = &v[0];  // Now you're returning a pointer to the internals of this new vector.
  // but then the vector [v] is forgotten, so how can you ever clean it up properly?
}

The only correct way to clean up v is to call delete v; (delete because it was allocated with new). But, outside of this function, there's no knowledge of the vector (anyone using this is going to think it just new[]ed an array). So this won't work.

You could instead just use a vector for real:

void g(vector<float>* f);
// Usage:
//   vector<float> foo;
//   g(&foo);

No dynamic allocation, no new, no raw pointers, much cleaner.

Upvotes: 0

Related Questions