Thread: Reference Counting Template

I'm having a problem with a template class I wrote that will allow me to have reference counters for object pointers. I have objects contained in multiple lists/queues and I need to prevent the object from being deleted every time it is removed from a list.

This is what I have

Code:

template<class T>
class SmartPtr
{
    public:
        explicit SmartPtr(T* obj)
            : ptr(obj) { obj->addRef(); }
        explicit SmartPtr() {}
        SmartPtr(const SmartPtr<T>& orig)
        {
            ptr = orig.ptr;
            ptr->addRef();
        }
        ~SmartPtr()
        {
            ptr->removeRef();
            if (ptr->ref_count() == 0) delete ptr;
        }
        T& operator*()      { return *ptr;  }
        T* operator->()     { return ptr;    }
        SmartPtr<T>& operator=(SmartPtr<T> right)
        {
            if (right.ptr)
                right.ptr->addRef();

            ptr = right.ptr;

            return *this;
        }
    private:
        T* ptr;
};

Basically, I want the shared functionality of Boost's shared_ptr but I'd like to implement it.

My problem is that I am getting a segfault when I use the above code and it's not near anywhere where I use the SmartPtr so it is difficult to pinpoint what is wrong.

It looks like the assignment operator is incorrect. It shouldn't modify the right-hand value. And it should decrement the reference count on the left-hand value. (Basically, I think the copy-and-swap idiom should do the trick again.)

Next, this SmartPtr allows NULL pointers, but the destructor doesn't check for NULL. Neither does the first constructor check whether the pointer is non-NULL. And the default constructor leaves ptr uninitialized.

First, the swap method should not be implemented in terms of assignment operator.

I suppose, all it takes to swap two SmartPtr's is to swap the pointers they hold.

Code:

//member function
void swap(SmartPtr<T>& other)
{
    std::swap(ptr, other.ptr);
}

You might also implement a free swap function (in terms of member swap), which will hopefully get picked up, for example, when a standard algorithm needs to swap your SmartPtr instances.

And now assignment operator would be implemented like this:

Code:

SmartPtr<T>& operator=(const SmartPtr<T>& right)
{
    //this takes care of incrementing ref-count on right
    SmartPtr<T> temp(right);

    //swap contents of left-hand instance and the local copy
    swap(temp);

    return *this;
    //destructor of temp takes care of decrementing ref-count on the pointer previously held by *this
}

(It would also be possible to accept right by value: then temp won't be needed, since a copy will be automatically made.)

Welcome to the "copy and swap" idiom!

It's the best thing since sliced bread...