Prototype for function that allocates memory on the heap (C/C++)

Question

Hi all,

I'm fairly new to C++ so this is probably somewhat of a beginner question. It regards the "proper" style for doing something I suspect to be rather common.

I'm writing a function that, in performing its duties, allocates memory on the heap for use by the caller. I'm curious about what a good prototype for this function should look like. Right now I've got:

int f(char** buffer);

To use it, I would write:

char* data;
int data_length = f(&data);
// ...
delete[] data;

However, the fact that I'm passing a pointer to a pointer tips me off that I'm probably doing this the wrong way.

Anyone care to enlighten me?

Answer 1

Why not do the same way as malloc() - void* malloc( size_t numberOfBytes )? This way the number of bytes is the input parameter and the allocated block address is the return value.

UPD: In comments you say that f() basically performs some action besides allocating memory. In this case using std::vector is a much better way.

void f( std::vector<char>& buffer )
{
    buffer.clear();
    // generate data and add it to the vector
}

the caller will just pass an allocated vector:

 std::vector buffer;
 f( buffer );
 //f.size() now will return the number of elements to work with

Answer 2

Just return the pointer:

char * f() {
   return new char[100];
}

Having said that, you probably do not need to mess with explicit allocation like this - instead of arrays of char, use std::string or std::vector<char> instead.

Answer 3

Pass the pointer by reference...

int f(char* &buffer)

However you may wish to consider using reference counted pointers such as boost::shared_array to manage the memory if you are just starting this out.

e.g.

int f(boost::shared_array<char> &buffer)

Answer 4

In 'proper' C++ you would return an object that contains the memory allocation somewhere inside of it. Something like a std::vector.

Answer 5

I guess you are trying to allocate a one dimensional array. If so, you don't need to pass a pointer to pointer.

int f(char* &buffer)

should be sufficient. And the usage scenario would be:

char* data;
int data_length = f(data);
// ...
delete[] data;

Answer 6

Provided that f does a new[] to match, it will work, but it's not very idiomatic.

Assuming that f fills in the data and is not just a malloc()-alike you would be better wrapping the allocation up as a std::vector<char>

void f(std::vector<char> &buffer)
{
    // compute length
    int len = ...

    std::vector<char> data(len);

    // fill in data
    ...

    buffer.swap(data);
}

EDIT -- remove the spurious * from the signature

Answer 7

Actually, the smart thing to do would be to put that pointer in a class. That way you have better control over its destruction, and the interface is much less confusing to the user.

class Cookie {
public:
   Cookie () : pointer (new char[100]) {};
   ~Cookie () {
      delete[] pointer;
   }
private:
   char * pointer;

   // Prevent copying. Otherwise we have to make these "smart" to prevent 
   // destruction issues.
   Cookie(const Cookie&);
   Cookie& operator=(const Cookie&);
};

Answer 8

Use RAII (Resource Acquisition Is Initialization) design pattern.

http://en.wikipedia.org/wiki/RAII http://stackoverflow.com/questions/712639/please-help-us-non-c-developers-understand-what-raii-is

Answer 9

If all f() does with the buffer is to return it (and its length), let it just return the length, and have the caller new it. If f() also does something with the buffer, then do as polyglot suggeted.

Of course, there may be a better design for the problem you want to solve, but for us to suggest anything would require that you provide more context.

Answer 10

Your function should not return a naked pointer to some memory. The pointer, after all, can be copied. Then you have the ownership problem: Who actually owns the memory and should delete it? You also have the problem that a naked pointer might point to a single object on the stack, on the heap, or to a static object. It could also point to an array at these places. Given that all you return is a pointer, how are users supposed to know?

What you should do instead is to return an object that manages its resource in an appropriate way. (Look up RAII.) Give the fact that the resource in this case is an array of char, either a std::string or a std::vector seem to be best:

int f(std::vector<char>& buffer);

std::vector<char> buffer;
int result = f(buffer);

Answer 11

The proper style is probably not to use a char* but a std::vector or a std::string depending on what you are using char* for.

About the problem of passing a parameter to be modified, instead of passing a pointer, pass a reference. In your case:

int f(char*&);

and if you follow the first advice:

int f(std::string&);

or

int f(std::vector<char>&);

Answer 12

In C, that would have been more or less legal.

In C++, functions typically shouldn't do that. You should try to use RAII to guarantee memory doesn't get leaked.

And now you might say "how would it leak memory, I call delete[] just there!", but what if an exception is thrown at the // ... lines?

Depending on what exactly the functions are meant to do, you have several options to consider. One obvious one is to replace the array with a vector:

std::vector<char> f();

std::vector<char> data = f();
int data_length = data.size();
// ...
//delete[] data;

and now we no longer need to explicitly delete, because the vector is allocated on the stack, and its destructor is called when it goes out of scope.

I should mention, in response to comments, that the above implies a copy of the vector, which could potentially be expensive. Most compilers will, if the f function is not too complex, optimize that copy away so this will be fine. (and if the function isn't called too often, the overhead won't matter anyway). But if that doesn't happen, you could instead pass an empty array to the f function by reference, and have f store its data in that instead of returning a new vector.

If the performance of returning a copy is unacceptable, another alternative would be to decouple the choice of container entirely, and use iterators instead:

// definition of f
template <typename iter>
void f(iter out);

// use of f
std::vector<char> vec;
f(std::back_inserter(vec));

Now the usual iterator operations can be used (*out to reference or write to the current element, and ++out to move the iterator forward to the next element) -- and more importantly, all the standard algorithms will now work. You could use std::copy to copy the data to the iterator, for example. This is the approach usually chosen by the standard library (ie. it is a good idea;)) when a function has to return a sequence of data.

Another option would be to make your own object taking responsibility for the allocation/deallocation:

struct f { // simplified for the sake of example. In the real world, it should be given a proper copy constructor + assignment operator, or they should be made inaccessible to avoid copying the object
  f(){
    // do whatever the f function was originally meant to do here
    size = ???
    data = new char[size];
  }
  ~f() { delete[] data; }

int size;
char* data;
};

f data;
int data_length = data.size;
// ...
//delete[] data;

And again we no longer need to explicitly delete because the allocation is managed by an object on the stack. The latter is obviously more work, and there's more room for errors, so if the standard vector class (or other standard library components) do the job, prefer them. This example is only if you need something customized to your situation.

The general rule of thumb in C++ is that "if you're writing a delete or delete[] outside a RAII object, you're doing it wrong. If you're writing a new or `new[] outside a RAII object, you're doing it wrong, unless the result is immediately passed to a smart pointer"