C++ Smart Pointer Question

[gioni_go]

Hello ,
I'm a little rusty on c++ so i want to ask a questions to the gurus.
I am trying to create a smart pointer(dont wanna use boost) and i want to know if the following implementation is ok :

#ifndef SMARTPTR_H_
#define SMARTPTR_H_

#ifdef __DEBUG__SESSION
#include <stdio.h>
#endif

namespace smartptr {

inline void DEBUG(const char *msg) {
#ifdef __DEBUG__SESSION
 printf("%s\n",msg);
#endif
}

template<typename T>
class SmartPtr {
 T *m_ptr;
 bool m_isCollection;
 int *m_refCount;
 int temp;

public:

 SmartPtr() :
  m_refCount(0), m_ptr(0), m_isCollection(0) {
  DEBUG("Default constructor.");
 }

 SmartPtr(T* ptr, bool isCollection = false) :
  m_refCount(0) {
  DEBUG("T* constructor.");
  reset(ptr, isCollection);
 }

 SmartPtr(const SmartPtr &other) :
  m_refCount(0) {
  reset(other.m_ptr, other.m_isCollection);
  other.release();
  DEBUG("Copy constructor.");
 }

 T* get() const {
  DEBUG("GET.");
  return m_ptr;
 }

 bool isValid() const {
  return (m_ptr != 0);
 }

 void reset(T* ptr, bool isCollection = false) {
  destroy();
  m_refCount = &temp;
  m_ptr = ptr;
  m_isCollection = isCollection;
  *m_refCount = 1;
  DEBUG("Reset.");
 }

 T* release() const {
  T* ptr = m_ptr;
  if (m_refCount) {
   (*m_refCount)++;
  }
  DEBUG("Released pointer ownership.");

  return ptr;
 }

 void destroy() {

  if (m_ptr && m_refCount) {
   (*m_refCount)--;
   if (*m_refCount == 0) {
    if (m_isCollection) {
     delete[] m_ptr;
     m_ptr = 0;
     DEBUG("Destroyed collection.");
     return;
    }

    delete m_ptr;
    m_ptr = 0;
    DEBUG("Destroyed single pointer.");
   }
  }

  if (m_refCount != 0) {
   m_refCount = 0;
  }
 }

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

 T& operator *() const {
  return *get();
 }

 const SmartPtr& operator=(T* ptr) {
  reset(ptr);
  DEBUG("Op = T*.");
  return *this;
 }

 const SmartPtr& operator=(const SmartPtr<T> &other) {
  DEBUG("Equals");
  reset(other.m_ptr, other.m_isCollection);
  other.release();
  return *this;
 }

 ~SmartPtr() {
  destroy();
 }

};

}

#endif /* SMARTPTR_H_ */

I need to know especially the part where the reference counter is involved. I want to keep everything in the class on the stack so the m_refCount to a stack location. Is this illegal and if yes what are the dangers. Thx in advance.

[nuzzle]

I am trying to create a smart pointer(dont wanna use boost) and i want to know if the following implementation is ok :

I strongly advice against implementing your own smart pointer. They're notoriously difficult to get right.

Why don't you want to use the smart pointers of boost?

[Richard.J]

you can as well use the smart pointers in std::tr1, if your compiler is up-to-date.

[gioni_go]

These are not the answers i want to hear. Please answer to the question i asked , i already said i dont want to use boost or any other implementation.

[Richard.J]

One problem I see is that the access to the refcount does not seem to be thread-safe because it is not protected b a lock or something. I still stick to my advise not to re-invent the wheel and use something that is known to be working good.

[Lindley]

These are not the answers i want to hear. Please answer to the question i asked , i already said i dont want to use boost or any other implementation.

Yes, we just think that's a very silly attitude and would like you to explain it.

[gioni_go]

I am doing some programming for Symbian and i don't want to use the cleanup-stack stuff seems rather cumbersome and non portable. I just want a simple smart pointer , no need for thread safety , i am not using threads. I am not sure if there is an alternative, but i wanted a smart pointer that uses internally only the stack , no heap allocations. I created this simple one. Thx for your answers.

[nuzzle]

These are not the answers i want to hear. Please answer to the question i asked , i already said i dont want to use boost or any other implementation.

In that case I suggest you contact a consulting firm.

If you want any help from me I first want to know why you cannot use a smart pointer from boost or std/tr1. But you don't have to tell me because I already know from your reaction. This is homework and you're not allowed to use any of those.

[Lindley]

I'm not sure it's possible to do reference-counting without a heap allocation. At least, not without introducing considerable unnecessary complexity.

[nuzzle]

I am not sure if there is an alternative, but i wanted a smart pointer that uses internally only the stack , no heap allocations.

Your best option is to use intrusive_ptr from boost. You put the reference counter in the object. It means it will be located where the object is located, on the stack or on the heap. You don't have to make the counter threadsafe if you don't want to. It's up to you to increment/decrement the counter. Just make sure you don't delete the object if it wasn't allocated with new.

Piece of cake really and you get a robust and well tested smart pointer implementation for free.

[Zaccheus]

I don't think using the data member 'temp' as a reference counter will work.

[ninja9578]

There are lots of open source reference counting classes out there, why not lift code from one of those? wxWidgets has a nice one.

[potatoCode]

Is this illegal and if yes what are the dangers. Thx in advance.

There's nothing wrong, technically, to have m_refCount(0) point to the count variable declared on the stack. But the problem with this approach is about ten-fold.
The "danger" lies in the fact that having m_refCount on the stack instead on the heap,
defeats the very purpose of reference counting of temp variable in your SmartPtr class.
Copying m_refCount will still point to the temp, but the copying of temp will not result in correct count.
In simple words,
your SmartPtr class itself needs another smart pointer semantic inside which you did not implement.
The difficulty of implementing such semantic is the complexity noted by Lindley.
(I personally have not tried it, but you can make an educated guess how hard it would be)

T* release() const {
  T* ptr = m_ptr;
  if (m_refCount) {
   (*m_refCount)++;
  }
  DEBUG("Released pointer ownership.");

  return ptr;
 }

Why are you thinking of releasing the ownership to who knows where?
That alone doesn't make sense at all.
Assuming release() will be used by SmartPtr only is a big mistake

there are other problems.
illogical order of the all-in-one call chain to ctor, reset, destroy.
incomplete implementation of copy ctor, operator=()
(all these methods themselves have problems of their own).
bottom line is that your SmartPtr class needs a complete make over as a whole.

If your c++ is little rusty,
I'd suggest that you try much simpler smart pointer class
and plainly have reference count variable on the heap.

[nuzzle]

But the problem with this approach is about ten-fold.

Admittedly, smart pointers traditionally are used to manage heap allocated objects, but that's only by convention.

Nothing prevents you from having reference counting on stack allocated objects really. It's just that you cannot delete such objects.

So the gut reaction that reference counting for stack allocated objects is wrong, is misdirected in my view. This is why we are using C++ in the first place, to have memory management options.

And the easiest way to achieve this to use the intrusive_ptr of boost (see my previous post). It gives you great flexibility in where to place the reference counter and what actions to take when it changes.

[potatoCode]

Admittedly, smart pointers traditionally are used to manage heap allocated objects, but that's only by convention.

Nothing prevents you from having reference counting on stack allocated objects really. It's just that you cannot delete such objects.

I agree and that's very open minded perspective.

Although unrelated to this issue,
I once (when I was a really really newbie xD) tried to design factory pattern without using new.
And most of the members advised against it or thought it cann't be done.
Having had this valuable experience,
I was able keep my focus on addressing the problems the OP had,
rather than simply come out and tell him "it's a bad idea."