3
votes

I recently started to migrate many of my existing classes over to using smart pointers and I have a few questions about how to port some code that I think could benefit from the use of smart pointers (but I might be wrong of course). I have a UtlMemBuffer buffer manager class shown below via its header file.

Essentially, this class owns a vector of buffers made up of void*/length pairs. The buffer management is implemented via a helper method (UtlMemBuffer::append - the implementation of which is also shown below).

I would like to make this class use the new C++11 smart pointers so that ownership could be well defined and reallocation minimized. To use this class, client code typically passes in its own raw pointer buffer/length to the constructor, or it can call append. Eventually when the UtlMemBuffer's destructor is called, it frees its own copy, ensuring that there are no memory leaks while the caller is responsible for its copy. I think that instead of passing in raw pointers, using a std::shared_ptr would obviate the need for this type of double buffer ownership (i.e. the caller would be responsible for newing up a std::shared_ptr and passing it to the UtlMembuffer.

The biggest challenge as I see it is supporting the read method (the interface is similar to the way a file works) where I need to pass back flattened memory via a raw pointer. Perhaps a better design approach would be for me to pass back a std::unique_ptr which I create from flattening the internal collection of shared_ptrs. I am not quite sure what the best approach would be althought I would have to change quite a bit of current code that uses the class to take that approach.

Should the callers all pass in std::shared_ptr pointers for ex. I wonder what the best approach to making this conversion would be.

I am pretty new at this smart pointer business so any advice would be greatly appreciated. Thanks

/**
 * Smart Buffer class
 */
class UtlMemBuffer
{
public:
    // default constructor
    UtlMemBuffer(
        const UtlPath& rBufferPath = UtlPath::ssNull,
        const void* pBytes = NULL,
        const size_t& rBufLength = 0);

    // copy constructor
    UtlMemBuffer(const UtlMemBuffer& rhs);

    // move constructor
    UtlMemBuffer(UtlMemBuffer&& rhs);

    inline void swap(UtlMemBuffer& rhs) throw() {
        // enable ADL (not necessary in our case, but good practice)
        using std::swap;
        // no need to swap base members - as we are topmost class
        swap(mBufferPath, rhs.mBufferPath);
        swap(mBufferLength, rhs.mBufferLength);
        swap(mBufferBlocks, rhs.mBufferBlocks);
    }

    // unified assignment operator
    UtlMemBuffer& operator=(UtlMemBuffer rhs);

    // destructor - pure virtual
    virtual ~UtlMemBuffer();

    // add buffer to this one
    virtual OsStatus append(
        const void* pBytes,
        const size_t& rBufLength,
        size_t& rBufLengthWritten);

    // comparator
    bool operator==(const UtlMemBuffer& rhs) const;

    // comparator
    bool operator<(const UtlMemBuffer& rhs) const;

    // determine the size of the buffer
    size_t size() const;

    /**
     * comparable interface
     *
     * @returns 0 if equal, negative val if less than &
     *          negative value if greater.
     */
    virtual int compareTo(const UtlMemBuffer& rhs) const;

    /** copy the bytes into the designated buffer */
    OsStatus read (void* pReturnBuffer,
        const size_t& rMemBufferOffset,
        const size_t& rReturnBufferLength,
        size_t& rBytesRead) const;

    // free existing linked list of blocks
    void clear();

    // path property
    void setBufferPath(const UtlPath& rBufferPath);

    UtlPath getBufferPath() const;

private:
    typedef std::vector< std::pair<void*, size_t> > MemBufInfo;

    // the name of the buffer (sort of file name)
    UtlPath mBufferPath;

    // this is updated whenever we append data
    size_t mBufferLength;

    // here we have a collection of received appends (effectively blocks)
    MemBufInfo mBufferBlocks;
};

Here is the helper method which manages access to the vector of buffer blocks. As you can see, it reallocates the raw pointers and stores them in the mBufferBlocks member.

OsStatus
UtlMemBuffer::append(const void* pBytes,
    const size_t& rBufLength,
    size_t& rBytesWritten)
{
    rBytesWritten = 0;
    if (pBytes != NULL) {
        void* block = new char [rBufLength];
        memcpy(block, pBytes, rBufLength);
        mBufferBlocks.push_back(std::make_pair(block, rBufLength));
        rBytesWritten = rBufLength;
        mBufferLength += rBufLength;
    }
    return OS_SUCCESS;
}
1

1 Answers

0
votes

If the author had simply used char* to represent memory bytes, like is usual, then you could have simply stored vectors instead of pairs of void*/len.

That's what I'd probably do here. Assuming you don't have to do any actual destruction of objects in this memory space, just cast to char* and stick in a vector.