how do I delete allocated memory and still return its value from method

143 views Asked by At

I have this function as part of a class I am writing:

const char* sockets::TCPSocket::doRecv(int flags)
{
    char* incomingDataBuffer = new char [this->bufferSize];
    ssize_t bytesReceived    = recv(this->filedes, incomingDataBuffer, this->bufferSize, flags);

    // TODO set timeout - If no data arrives, the program will just wait here until some data arrives.
    if (bytesReceived == 0 || bytesReceived == -1)
    {
        // TODO error handling
    }

    // TODO avoid memory leak
    // delete[] incomingDataBuffer;
    // incomingDataBuffer = 0;


    return incomingDataBuffer;
}

As you can see my issue here is that I need to dynamically allocate the buffer size for my incoming string and I would like to return that value to the user. I do not want to leave it to the user to have to then free that memory as that seems like poor encapsulation.

My instinct is to create a static copy of the incomingDataBuffer c string and return that to my user. However despite some heavy searching I have been unable to find a common method of doing this which leads me to think I may simply be taking a wrong approach.

Clearly I have other options.

  • I could make incomingDataBuffer a class member and then deal with its deletion in the destructor, but this somehow doesn't feel right as it has no other reason to be a class member.

  • I figure I could iterate through the array and convert it into a vector which can be returned and converted to a string. But again this doesn't feel quite right as the incomingDataBuffer could in some cases be pretty large and this kind of action could be quite expensive.

Anyway, I guess this must be a common problem with a standard approach, so what is the correct c++ way?

3

There are 3 answers

11
Barry On BEST ANSWER

The standard C++ way would be to use a std::vector:

std::vector<char> sockets::TCPSocket::doRecv(int flags)
{
    std::vector<char> incomingDataBuffer(this->bufferSize);
    ssize_t bytesReceived = recv(this->filedes, 
        &incomingDataBuffer[0], this->bufferSize, flags);

    // TODO set timeout - If no data arrives, 
    // the program will just wait here until some data arrives.
    if (bytesReceived == 0 || bytesReceived == -1)
    {
        // TODO error handling
    }

    // on success, do this so that call site knows how much data
    // there actually is
    incomingDataBuffer.resize(bytesReceived);
    return incomingDataBuffer;
}

Since vector manages its memory, there is no issue of memory leak here. By returning it, you're just transferring responsibility of memory management to the caller - but the caller doesn't have to do anything special. When the vector goes out of scope, the memory is deleted automatically.

0
lisyarus On

The C++ way would be to use std::unique_ptr[].

std::unique_ptr<const char[]> sockets::TCPSocket::doRecv(int flags)
{
    std::uniqure_ptr<char[]> incomingDataBuffer(new char [this->bufferSize]);
    ssize_t bytesReceived    = recv(this->filedes, incomingDataBuffer.get(), this->bufferSize, flags);


    return incomingDataBuffer;
}

std::unique_ptr<char[]> does delete [] in it's destructor, and returning it from function does no copying of the data (since it is simply moved).

0
Mortano On

Just use std::vector<char> instead of the dynamically allocated buffer:

std::vector<char> incomingBuffer(this->bufferSize);
ssize_t bytesReceived    = recv(this->filedes, incomingDataBuffer.data(), this->bufferSize, flags);
// Other stuff
return incomingBuffer;

This way memory is dynamically free'd once the vector leaves scope on the client side. With C++11 and move semantics, there also will be no expensive copy of the vector. In general, try to avoid explicit new/delete in modern C++, that's what the STL containers are for.

Just for completeness: The other alternative would be to use std::unique_ptr<char[]>, but for arrays its syntax is inferior to std::vectors in my opinion.