From heap to std::string

I'm writing a slightly more complex example on ASIO TCP than the first one, where the server sends a message through the socket to the client that reads it, and then both terminate.

The problem that we face in this post has not much to do with ASIO but on how we could smartly create a std::string from a c-string on the heap without risking memory leaks.

This issue arises from the fact that I want to send/receive through a socket an integer representing the c-string size, and then the string itself, as a stream of characters. The sending part goes smoothly, but receiving it is a bit bumpy. The message has a variable size, so we are forced to store it on the heap, and only after we know its size. But this forces us to writing code that it is clumsy, given that we should remember to delete it after the std::string has been generated from it, and should carefully pay attention to the risk of introducing memory leaks.

This was the first clumsy version:
std::string f(/* ... */)
{
    // ...
    int len;
    boost::asio::read(sock_, boost::asio::buffer(&len, sizeof(int)));

    char* local = new char[len+1];
    boost::asio::read(sock_, boost::asio::buffer(local, len)); // 1
    local[len] = '\0';

    std::string message(local);
    delete local; // 2
    return message; // 3
}
1. First issue: if we have an exception here, the memory associated to "local" is lost in space (or better, in heap).
2. Second issue: we must delete what we have newed, and we must do it before exiting the scope, so this is the exact place where to put this line. It is correct. But does it look elegant to you?
3. Third issue: if we had returned the string ctor, any compiler was good to optimize an unnecessary copy out. But not this case. Sure, if your compiler already support C++0x capabilities and the std::string implementation you use has a move constructor you can take advantage of it here.

So, you could get rid of issue 3 using a std::string move constructor, if available, issue 2 is a minor one, but still issue 1 should not be taken too lightly.

A solution is encapsulating that code in a class, letting the C++ stack unwinding capabilities to take care of details:
class Reader
{
private:
    char* local_;
public:
    Reader() : local_(nullptr) {}
    ~Reader() { delete local_; } // 1

    std::string read(boost::asio::ip::tcp::socket& sock_)
    {
        int len;
        boost::asio::read(sock_, boost::asio::buffer(&len, sizeof(int)));

        local_ = new char[len+1];
        boost::asio::read(sock_, boost::asio::buffer(local_, len)); // 2
        local_[len] = '\0';

        return std::string(local_); // 3
    }
};

// ...

std::string f()
{
    Reader r; // 4
    return r.read(sock_);
}
1. The call to delete is nicely put in the class destructor.
2. Even in case of exception, the dtor is called, so we can ensure delete is called on local_.
3. No extra copy object is generated by your compiler for this code.
4. The reader object is on the stack, so we can rely on the compiler for its correct memory management

1 comment: