Making a safe buffer holder in C++

Question

There are situations in which I need to pass a char* buffer back and forth. My idea is to create an object which can hold the object that owns the data, but also expose the data as char* for someone to read. Since this object holds the owner, there are no memory leaks because the owner is destructed with the object when it's no longer necessary.

I came with the implementation below, in which we have a segfault that I explain why it happens. In fact it's something that I know how to fix but it's something that my class kinda lured me into doing. So I consider what I've done to be not good and maybe there's a better way of doing this in C++ that is safer.

Please take a look at my class that holds the buffer owner and also holds the raw pointer to that buffer. I used GenericObjectHolder to be something that holds the owner for me, without my Buffer class being parametrized by this owner.

#include <iostream>
#include <string>
#include <memory>
#include <queue>

//The library:
class GenericObjectHolder
{
public:
    GenericObjectHolder()
    {
    }

    virtual ~GenericObjectHolder() {
        
    };
};

template <class T, class Holder = GenericObjectHolder>
class Buffer final
{
public:
    //Ownership WILL be passed to this object
    static Buffer fromOwned(T rawBuffer, size_t size)
    {
        return Buffer(std::make_unique<T>(rawBuffer), size);
    }

    //Creates a buffer from an object that holds the buffer
    //ownership and saves the object too so it's only destructed
    //when this buffer itself is destructed
    static Buffer fromObject(T rawBuffer, size_t size, Holder *holder)
    {
        return Buffer(rawBuffer, std::make_unique<T>(rawBuffer), size, holder);
    }

    //Allocates a new buffer with a size
    static Buffer allocate(size_t size)
    {
        return Buffer(std::make_unique<T>(new T[size]), size);
    }

    ~Buffer()
    {
        if (_holder)
            delete _holder;
    }

    virtual T data()
    {
        return _rawBuffer;
    }

    virtual size_t size() const
    {
        return _size;
    }

    Buffer(T rawBuffer, std::unique_ptr<T> buffer, size_t size)
    {
        _rawBuffer = rawBuffer;
        _buffer = std::move(buffer);
        _size = size;
    }

    Buffer(T rawBuffer, std::unique_ptr<T> buffer, size_t size, Holder *holder)
    {
        _rawBuffer = rawBuffer;
        _buffer = std::move(buffer);
        _size = size;
        _holder = holder;
    }


    Buffer(const Buffer &other)
        : _size(other._size),
          _holder(other._holder),
          _buffer(std::make_unique<T>(*other._buffer)) 
    {
    }


private:
    Holder *_holder;
    T _rawBuffer;
    std::unique_ptr<T> _buffer;
    size_t _size = 0;
};

//Usage:
template <class T>
class MyHolder : public GenericObjectHolder
{
public:
    MyHolder(T t) : t(t)
    {
    }

    ~MyHolder()
    {
    }

private:
    T t;
};

int main()
{
    std::queue<Buffer<const char*, MyHolder<std::string>>> queue;
    std::cout << "begin" << std::endl;
    {
        //This string is going to be deleted, but `MyHolder` will still hold
        //its buffer
        std::string s("hello");
        auto h = new MyHolder<std::string>(s);
    
        auto b = Buffer<const char*, MyHolder<std::string>>::fromObject(s.c_str(),s.size(), h);
        queue.emplace(b);
    }
    {
        auto b = queue.front();
        //We try to print the buffer from a deleted string, segfault
        printf(b.data());
        printf("\n");
    }
    std::cout << "end" << std::endl;
}

As you see, the s string is copied inside the object holder but gets destructed right after it. So when I try to access the raw buffer that buffer owns I get a segfault.

Of course I could simply copy the buffer from the s string into a new buffer inside my object, but It'd be inefficient.

Maybe there's a better way of doing such thing or maybe there's even something ready in C++ that does what I need.

PS: string is just an example. In pratice I could be dealing with any type of object that owns a char* buffer.

Live example: https://repl.it/repls/IncredibleHomelySdk

Answer 1

Your core problem is that you want your Holder to be moveable. But when the Owner object moves, the buffer object might also move. That will invalidate your pointer. You can avoid that by putting the owner in a fixed heap location via unique_ptr:

#include <string>
#include <memory>
#include <queue>
#include <functional>

template <class B, class Owner>
class Buffer
{
public:
    Buffer(std::unique_ptr<Owner>&& owner, B buf, size_t size) :
      _owner(std::move(owner)), _buf(std::move(buf)), _size(size)
    {}

    B data() { return _buf; }
    size_t size() { return _size; }

private:
    std::unique_ptr<Owner> _owner;
    B _buf;
    size_t _size;
};

//Allocates a new buffer with a size
template<typename T>
Buffer<T*, T[]> alloc_buffer(size_t size) {
    auto buf = std::make_unique<T[]>(size);
    return {std::move(buf), buf.get(), size};
}

Here's a repl link: https://repl.it/repls/TemporalFreshApi

---

If you want to have a type-erased Buffer, you can do that like this:

template <class B>
class Buffer
{
public:
    virtual ~Buffer() = default;
    B* data() { return _buf; }
    size_t size() { return _size; }

protected:
    Buffer(B* buf, size_t size) : 
      _buf(buf), _size(size) {};
    B* _buf;
    size_t _size;
};


template <class B, class Owner>
class BufferImpl : public Buffer<B>
{
public:
    BufferImpl(std::unique_ptr<Owner>&& owner, B* buf, size_t size) :
      Buffer<B>(buf, size), _owner(std::move(owner))
    {}
    
private:
    std::unique_ptr<Owner> _owner;
};

//Allocates a new buffer with a size
template<typename T>
std::unique_ptr<Buffer<T>> alloc_buffer(size_t size) {
    auto buf = std::make_unique<T[]>(size);
    return std::make_unique<BufferImpl<T, T[]>>(std::move(buf), buf.get(), size);
}

Again, repl link: https://repl.it/repls/YouthfulBoringSoftware#main.cpp