Я написал реализацию общего указателя. Я бы хотел его обзор.

Вроде работает, но прогон через Valgrind показывает, что где-то в моих тестах происходит утечка памяти, но я не знаю где.

Вот мой .h файл:

#include <stdexcept>

//Try to create a smart pointer? Should be fun.

template <class T>
class rCPtr {

    T* ptr;
    T** ptrToPtr;
    long* refCount;

public:

    rCPtr()
    {
        try
        {
            ptr = new T;
            ptrToPtr = &ptr;
            refCount = new long;
            *refCount = 1;
        }
        catch (...)
        {
            if (ptr)
                delete ptr;
            if (refCount)
                delete refCount;
        }
    }



    explicit rCPtr(T* pT)
    {
        try
        {
            ptr = pT;
            ptrToPtr = &ptr;
            refCount = new long;
            *refCount = 1;
        }
        catch(...)
        {
            if (ptr)
                delete ptr;
            if (refCount)
                delete refCount;
        }
    }

    ~rCPtr()
    {
        (*refCount)--;
        if (*refCount <= 0)
        {
            delete ptr;
            ptr = nullptr;
            delete refCount;
            refCount = nullptr;
        }
    }

    // I wonder if this is even necessary?
    bool exists() const {
        return *ptrToPtr; // Will be null if already freed (SCRATCH THAT IT CRASHES)
    }

    //Copy
    rCPtr(const rCPtr& other) : ptr(other.ptr), refCount(other.refCount)
    {
        (*refCount)++;
    }


    // If you pass another of same object.
    rCPtr& operator=(const rCPtr &right)
    {

        if (this == &right) {return *this;}

       T* leftPtr = ptr;
       long* leftCount = refCount;

       ptr = right.ptr;
       refCount = right.refCount;


        (*refCount)++;
        (*leftCount)--;

        // delete if no more references to the left pointer.
        if (*leftCount <= 0)
        {
            delete leftPtr;
            leftPtr = nullptr;
            delete leftCount;
            leftCount = nullptr;
        }
        return *this;
    }

    // This will create a 'new' object (call as name = new var) Make sure the type matches the one used for this object
    rCPtr& operator=(const T* right)
    {
        if (right == ptr) {return *this;}

        T* leftPtr = ptr;
        long* leftCount = refCount;

        ptr = right;
        *refCount = 1; // New refCount will always be 1

        (*leftCount)--;

        if (*leftCount <= 0)
        {
            delete leftPtr;
            leftPtr = nullptr;
            delete leftCount;
            leftCount = nullptr;
        }

    }


    T* operator->() const
    {
        if (exists())
        return *ptrToPtr;
        else return nullptr;

    }


    T& operator*() const
    {
        if (exists())
        return **ptrToPtr;

        // I dont know what else to do here
            throw std::out_of_range("Pointer is already deleted");
    }
    // Gives ref to ptr
    // Pls don't try to delete this reference, the class should take care of that
    const T& get() const
    {
        return *ptr; // This reference does not count to refCount
    }

    // if used in bool expressions if(rCPtr) {I think}
    explicit operator bool() const
    {
        return *ptrToPtr;
    }

    // returns the number of references
    long getRefCount() const
    {
        return *refCount;
    }

    // Will attempt to cast the stored pointer into a new type & return it
    // You probably should not delete this one either
    template <class X>
    X* dCast()
    {
        try
        {
            // X must be poly morphic or else, the cast will fail and cause an compiler error.
            auto casted = dynamic_cast<X*>(*ptrToPtr);
            return casted;
        }catch(const std::bad_cast& me)
        {
            return nullptr;
        }


    }

    // Resets current instance, if other instances exist will not delete object
    void reset()
    {
        (*refCount)--;
        ptrToPtr = nullptr;
        if (*refCount <= 0) // If the amount of references are 0 (negative count may explode ??)
        {
            delete ptr;
            ptr = nullptr;
            delete refCount;
            refCount = nullptr;
        }
    }


};

//Notes just finished typing it up. It compiled so that is good
//Valgrind DOES NOT like it though

Я воспользуюсь возможностью поблагодарить всех, кто ответит на это

Члены данных

Я не вижу смысла T** ptrToPtr переменная-член. Каждый экземпляр *ptrToPtr можно заменить на ptr. Удалите этого участника, и у вас будет на одну вещь меньше delete.

Почему refCount указатель на long? Если вы сделаете это просто long тогда тебе не нужно delete Это. Я вижу, что происходит. Каждый экземпляр с одним и тем же указателем должен иметь доступ к одному и тому же refCount. Понятно.

Теперь я вижу твое мышление ptrToPtr. Помните, что указатели содержат только адреса. Если вы копируете указатель, оба указателя относятся к одним и тем же данным. Вам не нужно делиться ссылками на указатели. Вот почему член ptrToPtr не нужен. Указатель и копия указателя точно так же указывают на одни и те же данные. Отныне считайте его удаленным из класса.

Методы

Конструктор по умолчанию: rCPtr()

Конструктор по умолчанию выделяет для нового построенного по умолчанию T и увеличивает refCount к одному. Почему? Нет данных для хранения. Нет причин звонить new когда пользователь ничего не запрашивал для сохранения. Лучшая версия была бы

rCPtr() : ptr(nullptr), refCount(nullptr)
{
}

Это также сделает ваш exists() метод работает правильно.

Конструктор с аргументом указателя: explicit rCPtr(T* pT)

Во-первых, хорошая работа с использованием explicit. Это хорошее значение по умолчанию для конструкторов с одним аргументом.

Здесь единственная строка, которую можно бросить, — это refCount = new long;. Итак, как и в конструкторе по умолчанию, нет необходимости в try блокировать.

rCPtr(T* pT) : ptr(pT), refCount(pT ? new long(1) : nullptr)
{
}

Во-первых, обратите внимание, что вы можете комбинировать new оператор с инициализацией (new long(1)). По возможности создавайте переменные с конечными значениями в том же операторе, который их создает.

Нет необходимости в try блок здесь. Если new long генерирует исключение (то есть ваша программа пытается выделить место для одного long), то ваша программа (или весь ваш компьютер) вот-вот выйдет из строя, поэтому нет причин пытаться исправить это.

Обратите внимание на различное значение инициализации refCount Переменная. Если этот экземпляр содержит nullptr, тогда refCount должен быть nullptr как в конструкторе по умолчанию.

Деструктор: ~rCPtr()

Этот метод точно такой же, как и reset(). Итак, давайте повторно воспользуемся методом.

~rCPtr()
{
    reset();
}

Вы захотите повторно использовать reset() насколько это возможно, потому что он содержит самый важный код: тот, который освобождает ресурсы, когда последний rCPtr разрушен. Это так важно, что вы должны написать это только один раз и сделать это правильно.

Проверить ненулевой указатель: bool exists() const

Эта функция может просто вернуть ptr. А nullptr конвертируется в false, что-нибудь еще true. Это правильно работает с фиксированными конструкторами.

Конструктор копирования: rCPtr(const rCPtr& other)

Ваш конструктор копирования в порядке. Хотя член ptrToPtr не был скопирован или восстановлен с помощью ptrToPtr = &ptr. Итак, скопировал rCPtrs имеют неинициализированные указатели, которые вызывают сбой программы (или, что еще хуже, возвращают ненужные данные) при разыменовании. Но ptrToPtr больше нет, так что это больше не проблема.

Оператор присваивания с rCPtr аргумент: rCPtr& operator=(const rCPtr &right)

Рассмотрим эту альтернативу оператору присваивания:

rCPtr& operator=(const rCPtr &right)
{
    if(ptr == right.ptr) { return *this; }
    reset();
    ptr = right.ptr;
    refCount = right.refCount;
    (*refCount)++;
    return *this;
}

Тебе плевать на предыдущее состояние rCPtr экземпляр перед назначением, поэтому просто позвоните reset() чтобы освободить предыдущий указатель. Если указатель совпадает с указателем в аргументе, указатель не будет deleted, поскольку аргумент также содержит ссылку на него, поэтому счетчик ссылок не может достигать нуля.

Оператор присваивания с указателем: rCPtr& operator=(const T* right)

Убедите себя, что эта альтернатива оператору присваивания ведет себя так же, как ваш код:

rCPtr& operator=(const T* right)
{
    if(right == ptr) { return *this; }
    reset();
    ptr = right;
    refCount = new long(1);
    return *this;
}

Как и в предыдущем операторе присваивания, вам все равно предыдущее состояние rCPtr экземпляр перед назначением, поэтому просто позвоните reset() чтобы освободить предыдущий указатель.

Еще одна проблема: я не думаю, что это нужно компилировать. Метод присваивает const T* к T*. Этого нельзя допускать, потому что данные, на которые указывает аргумент, являются const. Вы компилируете с -permissive?

Также я добавил недостающие return *this;.

Получение базовых данных: const T& get() const

Подпись этого метода не соответствует get() в стандартных интеллектуальных указателях C ++. В get() метод в std::shared_ptr а также std::unique_ptr вернуть необработанный указатель: T*. Итак, ваш метод должен быть

const T* get() const
{
    return ptr;
}

Разыменование: T* operator->() const

В вашем коде сейчас, если указатель не nullptr вернуть указатель, иначе вернуть nullptr. Это эквивалентно простому возврату указателя return ptr;. Или же, return get();. Хорошо использовать методы повторно, чтобы работа выполнялась в очень немногих местах, за которыми легче следить.

Разыменование: T& operator*() const

Во-первых, это не должно компилироваться. Метод возвращает не-const ссылка из const метод. Это позволит изменить данные, на которые указывает указатель, из const rCPtr. Это не кажется правильным. Должно быть два метода разыменования указателя: const T& operator*() const а также T& operator*().

Итак, после замены **ptrToPtr с участием *ptr, нам нужно подумать, что происходит, когда exists() возвращает false. Прямо сейчас ваш код выдает исключение. Это правильный выбор. Другой выбор, сделанный std::unique_ptr а также std::shared_ptr просто позвонить *ptr независимо, и выйдет из строя программа, если это nullptr. Это также действительно и предупреждает пользователя о том, что он несет ответственность за то, что этого не делает. Ваш путь безопаснее, другой путь может быть быстрее. Любой выбор хорош.

Проверка ненулевого указателя: explicit operator bool() const

Две вещи. Первый, explicit здесь не используется, поскольку он используется только для конструкторов с одним аргументом. Во-вторых, вы можете позвонить return exists(); здесь, чтобы повторно использовать этот метод.

Получите количество ссылок: long getRefCount() const

Этот метод не будет работать после вызова reset() поскольку refCount является deleted. Нам нужно проверить exists() перед разыменованием nullptr.

long getRefCount () const {return refCount? * refCount: 0; }

Кастинг: template <class X> X* dCast()

Интересный метод. Но приведение к указателю возвращает nullptr если заброс неудачен. std::bad_cast выбрасывается только в том случае, если приведение относится к ссылочному типу. Таким образом, этот метод можно свести к

template <class X>
X* dCast()
{
    return dynamic_cast<X*>(ptr);
}

Удаление данных: void reset()

Самый важный метод. После удаления **ptrToPtr оператор, метод должен избавиться от любых ссылок на указанные данные. Даже если есть другие ссылки, этот экземпляр должен перестать ссылаться на них. Итак, указатели теперь установлены на nullptr безусловно.

void reset()
{
    if( ! exists()) { return; }

    (*refCount)--;
    if (*refCount <= 0) // If the amount of references are 0 (negative count may explode ??)
    {
        delete ptr;
        delete refCount;
    }
    ptr = nullptr;
    refCount = nullptr;
}

С некоторыми из приведенных выше изменений мы должны убедиться, что refCount не является nullptr прежде чем проверить его разыменованное значение. Одна из проверок согласованности для этого класса — убедиться, что оба ptr а также refCount находятся nullptr или ни то, ни другое.

Ваш комментарий говорит мне, что вы боитесь, что негативный refCount означает, что что-то пошло не так. Вы правы в этом. Чтобы избежать этой ситуации, потребуется изучение и тестирование вашего кода, поскольку это, вероятно, означает, что delete скоро будут вызваны уже deleted данные.

Следующий черновик кода

#include <stdexcept>

template <class T>
class rCPtr {

    T* ptr;
    long* refCount;

public:
    rCPtr() : ptr(nullptr), refCount(nullptr)
    {
    }

    rCPtr(T* pT) : ptr(pT), refCount(pT ? new long(1) : nullptr)
    {
    }

    ~rCPtr()
    {
        reset();
    }

    bool exists() const {
        return get();
    }

    rCPtr(const rCPtr& other) : ptr(other.ptr), refCount(other.refCount)
    {
        (*refCount)++;
    }


    // If you pass another of same object.
    rCPtr& operator=(const rCPtr &right)
    {
        if(ptr == right.ptr) { return *this; }
        reset();
        ptr = right.ptr;
        refCount = right.refCount;
        (*refCount)++;
        return *this;
    }

    rCPtr& operator=(T* right)
    {
        if(right == ptr) { return *this; }
        reset();
        ptr = right;
        refCount = new long(1);
        return *this;
    }

    T* operator->() const
    {
        return get();
    }

    T& operator*() const
    {
        if (exists())
            return *get();
        else
            throw std::out_of_range("Pointer is already deleted");
    }

    // Gives ref to ptr
    // Pls don't try to delete this reference, the class should take care of that
    T* get() const
    {
        return ptr;
    }

    // if used in bool expressions if(rCPtr) {I think}
    operator bool() const
    {
        return exists();
    }

    // returns the number of references
    long getRefCount() const
    {
        return refCount ? *refCount : 0;
    }

    // Will attempt to cast the stored pointer into a new type & return it
    // You probably should not delete this one either
    template <class X>
    X* dCast()
    {
        return dynamic_cast<X*>(*ptrToPtr);
    }

    // Resets current instance, if other instances exist will not delete object
    void reset()
    {
        if( ! exists()) { return; }

        (*refCount)--;
        if (*refCount <= 0) // If the amount of references are 0 (negative count may explode ??)
        {
            delete ptr;
            delete refCount;
        }
        ptr = nullptr;
        refCount = nullptr;
    }
};

Следующие вещи, о которых стоит подумать

Этот код работает для однопоточных программ, но что произойдет, если два экземпляра rCPtr указывая на одни и те же данные (*refCount == 2) находятся в разных потоках программы и reset() вызывается на обоих одновременно? То есть, что происходит, когда каждая строка reset() вызывается одновременно в обоих экземплярах параллельно? Для общих указателей требуется std::mutex в начале reset() функция, чтобы убедиться, что многопоточная работа не вызывает проблем (есть и другие способы).

Проверка кода

Не вижу смысла в этом члене:

    T** ptrToPtr;

    rCPtr()
    {
        try
        {
            ptr = new T;

Вызов new для создания нового T здесь неуместен (вполне нормально иметь пустой объект). Поскольку пользователь может развернуться и установить новый указатель на свой. В результате вам не нужен блок try catch на этом.

не обращайте внимания на то, как вы используете здесь try / catch, но стоит отметить, что try / catch для конструкторов можно написать, чтобы включить список инициализаторов.

   rCPtr()
      : ptr(nullptr)
      , refCount(nullptr)
   {}
    

Да, вам нужен блок try в этом конструкторе (потому что вы гарантировали, что объект будет управляться и, таким образом, уничтожен). Обратите внимание, что это уничтожение связано не только с вызовом удаления и управления памятью, но и с правильным управлением ресурсами.

    explicit rCPtr(T* pT)
    {
        try
        {
            ptr = pT;
            ptrToPtr = &ptr;
            refCount = new long;  // Why not allocate and initialize
            *refCount = 1;        // in one statement?
        }
        catch(...)
        {
            if (ptr)          // Don't need to check for null before
                delete ptr;   // calling delete.

            if (refCount)        // If the new long throws this is in
                delete refCount; // an indeterminate state. So calling
                                 // delete here is actually UB.

            // If there is an exception you need to make sure that
            // it continues.
            // otherwise your object contains invalid data by
            // makeing the exception continue you are syaing that the
            // construction failed and the object is not valid.

            throw; // re-throws the current exception.
        }
    }

Хотя эта попытка / уловка прекрасна. Мне нравится специальная версия try / catch для конструкторов, так как она позволяет вам помещать try-catch вокруг списка инициализаторов.

    explicit rCPtr(T* pT)
    try
        : ptr(pT)
        , ptrToPtr(&ptr)
        , refCount(new long(1))
    {}
    catch(...)
    {
        delete ptr;
        // don't need to call delete on refCount.
        // this is the only thing that could have failed and thrown
        // so the value is in an indeterminate state anyway,

        // Note: rethrow is not needed here.
    }

У меня проблема с проверкой, является ли refCount «меньше» или равным нулю. Если ваш refCount опустился ниже нуля, когда вы этого не ожидаете, то у вас есть серьезная ошибка в вашем приложении (вы, вероятно, уже вызвали delete для любого ненулевого ptr, и, следовательно, это, вероятно, будет второй вызов для удаления в указатель, и вы только что зашли на территорию UB).

Если ваш счетчик ссылок делает что-то неожиданное, единственное, что вы можете сделать, это выйти из приложения и надеяться, что данные не испортили ничего серьезного.

    ~rCPtr()
    {
        (*refCount)--;


        // If refCount is smaller than z
        if (*refCount <= 0)
        {
            delete ptr;
            ptr = nullptr;         // nulling out ptr is a waste of time.
                                   // It also hides errors. The debug
                                   // standatd libraries have extra code for
                                   // doing memory validation.
                                   //
                                   // If you have a bug and don't reset the
                                   // ptr to null then the debug libraries
                                   // may be able to help you detect it.
                                   //
                                   //
            // There is a small posabilty that the previous delete throws.
            // you still want to make sure that the next delete works
            // even if the previous one throws.
            delete refCount;
            refCount = nullptr;
        }
    }

Вы устанавливаете только две из трех переменных-членов. Убедитесь, что конструкторы всегда инициализируют все члены.

    rCPtr(const rCPtr& other) : ptr(other.ptr), refCount(other.refCount)
    {
        (*refCount)++;
    }

Примечание: они одинаковы, если оба используют один и тот же refCount объект.

    // If you pass another of same object.
    rCPtr& operator=(const rCPtr &right)
    {

       // I would check if refCount == right.refCount
       // If they are the same then you are not going to change.
       if (this == &right) {return *this;}


       T* leftPtr = ptr;
       long* leftCount = refCount;

       ptr = right.ptr;
       refCount = right.refCount;


        (*refCount)++;
        (*leftCount)--;

        // delete if no more references to the left pointer.
        if (*leftCount <= 0)
        {
            delete leftPtr;
            leftPtr = nullptr;

            // Again there is a small chance that the above delete will throw.
            // You need to make sure that this second delete is called even
            // if that happens.
            delete leftCount;
            leftCount = nullptr;
        }
        return *this;
    }

Лично я бы по-прежнему использовал идиому копирования и обмена для реализации этой функции. Это проверенный и проверенный метод, который дает вам надежную гарантию исключения.

    rCPtr& operator=(const rCPtr &right)
    {
        rCPtr  copy(right);
        swap(copy);
        return *this;

        // This way the destructor handles all the deallocation and ref
        // counting, and you keep that all in the same space.
        //
        // Note: Here it is the destructor of `copy` that will make sure
        //       the object ref conts are all kept correctly aligned.
    }

    rCPtr& operator=(const T* right)
    {
        // If you assign a pointer that is already being managed.
        // That is a major bug in your users code.
        // Not sure ignoring it is the correct solution.
        //
        // Not sure what the best answer is either.
        if (right == ptr) {return *this;}

        T* leftPtr = ptr;
        long* leftCount = refCount;

        ptr = right;
        *refCount = 1; // New refCount will always be 1

        (*leftCount)--;

        if (*leftCount <= 0)
        {
            delete leftPtr;
            leftPtr = nullptr;

            // Same issue with the delete as before.
            // This sort of emphasizes that common code should be
            // written once and in its own function. That way if you
            // find a bug you only have to fix it in one place and
            // that remoes the human errror of fixes that same bug
            // in multiple pleaces.
            delete leftCount;
            leftCount = nullptr;
        }

    }

В стандартной версии вызова умных пойтнеров * или же -> определяется как UB, если содержащийся указатель равен нулю. Вызывающий объект несет ответственность за то, чтобы интеллектуальный указатель содержал соответствующее значение.

    T* operator->() const
    {
        if (exists())
        return *ptrToPtr;       // Indentation issue.
        else return nullptr;

    }    

    T& operator*() const
    {

        if (exists())
        return **ptrToPtr;       // Indentation issue.

            throw std::out_of_range("Pointer is already deleted");
    }

Примечание dynamic_cast<> при применении к указателям не выбрасывает (в случае ошибки возвращает nullptr).

    // Will attempt to cast the stored pointer into a new type & return it
    // You probably should not delete this one either
    template <class X>
    X* dCast()
    {
        try
        {
            // X must be poly morphic or else, the cast will fail and cause an compiler error.
            auto casted = dynamic_cast<X*>(*ptrToPtr);
            return casted;
        }catch(const std::bad_cast& me)
        {
            return nullptr;
        }


    }

Я написал пару статей по этому поводу:

Уникальный Ptr
Общий Ptr
Специализированные конструкторы

Как бы я это написал:

#включать

// Пытаться создать умный указатель? Должно быть весело.

template <class T>
class rCPtr
{
    template<typename U>
    class DeletePtrUtility
    {
        U* ptr;
        public:
            DeletePtrUtility(U* p) : ptr(p){}
            ~DeletePtrUtility()            {delete ptr;}
    };

    T*    ptr;
    long* refCount;

    public:

    rCPtr()
        : ptr(nullptr)
        , refCount(nullptr)
    {}

    explicit rCPtr(T* pT)
    try
        : ptr(pT)
        // Only allocate a refCount if the pointer is not null
        // this matches the default constructor action.
        , refCount(pt ? new long(1) ? nullptr)
    {}
    catch(...)
    {
        delete ptr;
    }

    ~rCPtr()
    {
        // This is the only place where delete happens.
        // So make sure it is correct. All other operations
        // that can result in a delete will do this be invoking
        // the destructor (using copy and swap).

        if (refCount)
        {
            // If the pointer stored is null then the refCount
            // is also null so we can't de-reference it.
            --(*refCount);
            if (*refCount == 0)
            {
                // Make sure delete is always called on both
                // pointers (even if there is an exception).
                DeletePtrUtility<T>     deleteDataPtr(ptr);
                DeletePtrUtility<long>  deleteRefCPtr(refCount);
            }
        }
    }

    void swap(rCPtr& other)
    {
        std::swap(ptr,      other.ptr);
        std::swap(refCount, other.refCount);
    }
    friend void swap(rCPtr& lhs, rCPtr& rhs)
    {
        lhs.swap(rhs);
    }

    rCPtr(const rCPtr& other)
        : ptr(other.ptr)
        , refCount(other.refCount)
    {
        if (refCount) {
            // If there is an object, increment refcount.
            ++(*refCount);
        }
    }

    rCPtr& operator=(const rCPtr &right)
    {
        // Use copy and swap idiom
        rCPtr   copy(right);   // the minimum required to create object.
        swap(copy);            // Swap current and new state.
        return *this;
        // The destructor on copy will do the correct thing in
        // decrementing the reference count and if retuired
        // deleting the object.   
    }

    rCPtr& operator=(const T* right)
    {
        // Use copy and swap idiom.
        rCPtr   newRef(right);
        swap(newRef);
        return *this;
    }

    // Trivial operators.
    // keey these as one liners.
    T* operator->() const          {return ptr; }
    T& operator*() const           {return *ptr;}
    const T& get() const           {return *ptr;}
    long getRefCount() const       {return refCount ? *refCount : 0;}

    bool exists() const            {return ptr != nullptr;}    
    explicit operator bool() const {return exists();}

    template <class X>
    X* dCast()
    {
        // X must be poly morphic or else, the cast will fail and cause an compiler error.
        return dynamic_cast<X*>(ptr);
    }

};