C++ smart pointer class

Question

#include <iostream>
template <typename T>
class my_smart_ptr {
 T* raw;
 unsigned * p_count;
public:
 my_smart_ptr(T* t):
 raw(t),
 p_count (new unsigned(1))
 {
 }
 my_smart_ptr(const my_smart_ptr<T> & p):
 raw(p.raw),
 p_count(p.p_count)
 {
 ++(*p_count);
 }
 const my_smart_ptr<T> & operator=(const my_smart_ptr<T>& p)
 {
 if (p == *this)
 {
 return *this;
 }
 raw = p.raw;
 --(*p_count);
 ++(*(p.p_count));
 return *this;
 }
 ~my_smart_ptr()
 {
 --(*p_count);
 if (*(p_count) == 0)
 {
 delete raw;
 }
 }
};
class A{
public:
 A()
 {
 std::cout<< "A()" << std::endl;
 }
 ~A()
 {
 std::cout<< "~A()" << std::endl;
 }
};
int main()
{
 my_smart_ptr<A> a (new A);
 return 0;
}

• Edward: Welcome to Code Review! It's usually helpful to provide a text introduction to your code in addition to just the title. What is the code supposed to do? What were the design goals? Does it work as you expect? See the Help Center for more details on how to ask good questions here.
• Loki Astari: Worth reading: lokiastari.com/blog/2014/12/30/… :-)

Answer 1

I suppose that your class is trying to simulate a shared_ptr, however, I'm missing the part of overloading the dereference methods * and ->, because without them, your class seems useless (I can't do much with the pointed object).

Answer 2

You have an error in operator=. When the function returns, the p_count member of the LHS still points to the old p_count. It should be equal to the p_count of the RHS. Also you forget to release the old data when the count reaches zero.

const my_smart_ptr<T> & operator=(const my_smart_ptr<T>& p)
{
 if (p == *this)
 {
 return *this;
 }
 // Keep a copy of the old values.
 // Don't change these until the state of the
 // object has been made consistent.
 unsigned* oldCount = p_count;
 T* oldRaw = raw;
 // Update the state of the object.
 // Making sure to get the local count correct.
 raw = p.raw;
 p_count = p.p_count; // Without this the use count is incorrect.
 ++(*(p.p_count));
 // Now clean up the old state.
 --*oldCount;
 if (*oldCount == 0) {
 delete oldCount;
 delete oldRaw;
 }
 return *this;
}

It is customary for the operator= function to return a non-const reference. After all, you just modified the object.

my_smart_ptr<T> & operator=(const my_smart_ptr<T>& p) { ... }