wondering if I can use stl smart pointers for this

Question

I currently have a c++ class as follows:

template<class T>
class MyQueue {
   T** m_pBuffer;
   unsigned int m_uSize;
   unsigned int m_uPendingCount;
   unsigned int m_uAvailableIdx;
   unsigned int m_uPendingndex;
public:
    MyQueue(): m_pBuffer(NULL), m_uSize(0), m_uPendingCount(0),   m_uAvailableIdx(0),
            m_uPendingndex(0)
    {
    }

    ~MyQueue()
    {
        delete[] m_pBuffer;
    }

    bool Initialize(T *pItems, unsigned int uSize)
    {
        m_uSize = uSize;
        m_uPendingCount = 0;
        m_uAvailableIdx = 0;
        m_uPendingndex = 0;
        m_pBuffer = new T *[m_uSize];
        for (unsigned int i = 0; i < m_uSize; i++)
        {
            m_pBuffer[i] = &pItems[i];
        }
        return true;
    }
};

So, I have this pointer to arrays m_pBuffer object and I was wondering if it is possible to replace this way of doing things with the c++ smart pointer perhaps? I know I can do things like:

std::unique_ptr<T> buffer(new T[size]);

Is using a vector of smart pointers the way to go? Is this recommended and safe?

[EDIT] Based on the answers and the comments, I have tried to make a thread-safe buffer array. Here it is. Please comment.

#ifndef __BUFFER_ARRAY_H__
#define __BUFFER_ARRAY_H__

#include <memory>
#include <vector>
#include <mutex>
#include <thread>
#include "macros.h"

template<class T>
class BufferArray
{
public:
    class BufferArray()
        :num_pending_items(0), pending_index(0), available_index(0)
    {}

    // This method is not thread-safe. 
    // Add an item to our buffer list
    void add(T * buffer)
    {
        buffer_array.push_back(std::unique_ptr<T>(buffer));
    }

    // Returns a naked pointer to an available buffer. Should not be
    // deleted by the caller. 
    T * get_available()
    {
        std::lock_guard<std::mutex> lock(buffer_array_mutex);
        if (num_pending_items == buffer_array.size()) {
            return NULL;
        }       
        T * buffer = buffer_array[available_index].get();
        // Update the indexes.
        available_index = (available_index + 1) % buffer_array.size();
        num_pending_items += 1;
        return buffer;
    }

    T * get_pending()
    {
        std::lock_guard<std::mutex> lock(buffer_array_mutex);
        if (num_pending_items == 0) {
            return NULL;
        }

        T * buffer = buffer_array[pending_index].get();
        pending_index = (pending_index + 1) % buffer_array.size();
        num_pending_items -= 1;
    }


private:
    std::vector<std::unique_ptr<T> >    buffer_array;
    std::mutex                          buffer_array_mutex;
    unsigned int                        num_pending_items;
    unsigned int                        pending_index;
    unsigned int                        available_index;

    // No copy semantics
    BufferArray(const BufferArray &) = delete;
    void operator=(const BufferArray &) = delete;
};

#endif

Answer 1

Vector of smart pointers is good idea. It is safe enough inside your class - automatic memory deallocation is provided.

It is not thread-safe though, and it's not safe in regard of handling external memory given to you by simple pointers.

Note that you current implementation does not delete pItems memory in destructor, so if after refactoring you mimic this class, you should not use vector of smart pointers as they will delete memory referenced by their pointers.

On the other side you cannot garantee that noone outside will not deallocate memory for pItems supplied to your Initialize. IF you want to use vector of smart pointers, you should formulate contract for this function that clearly states that your class claims this memory etc. - and then you should rework outside code that calls your class to fit into new contract. If you don't want to change memory handling, vector of simple pointers is the way to go. Nevertheless, this piece of code is so simple, that there is no real benefit of vector.

Note that overhead here is creation of smart pointer class for each buffer and creation of vector class. Reallocation of vector can take up more memory and happens without your direct control.

Answer 2

The code has two issues:

1) Violation of the rule of zero/three/five:

To fix that you do not need a smart pointer here. To represent a dynamic array with variable size use a std:vector<T*>. That allows you to drop m_pBuffer, m_uSize and the destructor, too.

2) Taking the addresses of elements of a possible local array

In Initialize you take the addresses of the elements of the array pItems passed as argument to the function. Hence the queue does not take ownership of the elements. It seems the queue is a utility class, which should not be copyable at all:

template<class T>
class MyQueue 
{
   std::vector<T*> m_buffer;
   unsigned int m_uPendingCount;
   unsigned int m_uAvailableIdx;
   unsigned int m_uPendingndex;

    public:
    MyQueue(T *pItems, unsigned int uSize)
    : m_buffer(uSize, nullptr), m_uPendingCount(0),  m_uAvailableIdx(0), m_uPendingndex(0)
    {
        for (unsigned int i = 0; i < uSize; i++)
        {
            m_buffer[i] = &pItems[i];
        }
    }

    private:
    MyQueue(const MyQueue&); // no copy (C++11 use: = delete)
    MyQueue& operator = (const MyQueue&); // no copy (C++11 use: = delete) 

};

Note:

The red herring is the local array. You may consider a smart pointer for that, but that is another question.