C++ free memory passing to argument in function or leave it to the caller

Question

I hope this question is not too subjective.. all I want is good design that would prevent memory leaks happening later in my code. I coulnd't find any question on SO. I always found questions about what to do after allocating data in function which is not my case.

Also for some reason I am not using newer C++ standards like C++11 (shared pointers).

Let me demonstrate by example:

I have logic of data buffering which are later being sent. The buffering is done in one class and sending in another class.

In one point of code I am taking some data from buffer, process it (check the type of data etc) and then send it with function send:

bool send_data(char *data, size_t data_length) {

The data are consumed and are no longer needed. Shall I free them in the send_data or shell I leave that to the caller?

1. Free it inside:

   bool send_data(char *data, size_t data_length) {
     //... process data ...
     send(data, data_length, ...);
     delete[] data;
   }
   

2. Leave it and let the caller free it:

   send_data(data,data_length);
   delete[] data;
   

Or is there a design flaw and I should do something totally different?

---

The reason for not using C++11 is that the code is big & old - should I rewrite it completely? But I am considering to rewrite just some parts of the code because something is better then nothing.

And also the usage of some pointers spans lots of places of code I would have to change them all. Sometimes its not so easy to find them all because the usage may be hidden by casting and the buffering etc.

The data buffering is important. I have lot of data in buffers not just one char array. I am not sure if the data can be made static as some of you have in answers.. I will think about it.

Answer 1

If you don't want to use c++11, you can use std::auto_ptr, or if you can actually, use std::unique_ptr. But as I see, it seems like you are using char * for, may be, an array. If so, don't use smart pointers (at least without custom deallocators) and call delete[] instead of delete. Also, if you need to pass a char *, you can use

std::vector<char> vec;
vec.push_back(...);
//...
send_data(&vec[0], vec.size());

If you are sure you strongly want to use char *, delete it in the caller, it is far better practice. Because who allocates memory is it's owner, so owner deletes it. Also it removes a side effect from callee, and makes it callee easier to use for other developers, who won't expect i.e that send_data will also delete their data.

Answer 2

If you want good design to prevent memory leaks, the answer is to use a container, and there are plenty of containers to chose from that don't require C++11. If you want it to be as "freeform raw data" as possible, then yes, you should use a newer standard and use unique or shared pointers - is there any particular reason you're still stuck in the last decade compiler-wise?

If you want to handle it the C way (which is what you're insisting on doing above), then it's really application dependent. If you meet the following constraints:

1) Only one thread will use the data at a time 2) The data size is never prohibitive 3) There's nothing else that would make it unreasonable to leave it sticking around

... then I recommend storing it in a static pointer, wherein nobody ever needs to free it. That's what a lot of the stdlib library functions do when they deal with strings.

Answer 3

C++ style would be to use the safe ptr wrappers.

C style, as here, means definitely leave it to the caller.

The call could be:

char data[256];
...
send(data, sizeof(data);

So no delete[] data

To be a bit more safe, you could hide the original send and manage data and its deletion separately. C++ as class, C style as a couple of functions.

struct Data {
    char* data;
    size_t size;
};

void send(struct Data* data) {
    if (!data->data) {
        throw new IllegalStateException("...");
    }
    _send(data->data, data->size);
    delete[] data->data;
    data->data = NULL;
}