Pointer container class which can't be copied by value

Question

I need a smart pointer for my project which can be send to several methods as parameter. I have checked *auto_ptr* and *shared_ptr* from boost. But IMO, that is not suitable for my requirements. Following are my findings

auto_ptr : When passed to another method, ownership will be transferred and underlying pointer will get deleted when that method's scope ends. We can workaround this by passing auto_ptr by reference, but there is no compile time mechanism to ensure it is always passed by reference. If by mistake, user forgot to pass a reference, it will make problems.

boost::shared_ptr : This looks promising and works correctly for my need. But I feel this is overkill for my project as it is a very small one.

So I decided to write a trivial templated pointer container class which can't be copied by value and take care about deleting the underlying pointer. Here it is

template <typename T>
class simple_ptr{
public:
    simple_ptr(T* t){
       pointer = t;
    }
    ~simple_ptr(){
       delete pointer;
    }
    T* operator->(){
       return pointer;
    }
private: 
    T* pointer;
    simple_ptr(const simple_ptr<T>& t);
};

Is this implementation correct? I have made copy constructor as private, so that compiler will alert when someone tries to pass it by value.

If by chance the pointer is deleted, delete operation on the destructor will throw assertion error. How can I workaround this?

I am pretty new to C++ and your suggestion are much appreciated.

Thanks

Answer 1

To answer your first question, the best assurance you can get that this is correct is to implement a test harness around it to do some simple task, and make sure you get the behavior you expect. For me, that is far better comfort the code is right than the opinion of some random person reading it.

As for your second question, you work around delete throwing an assertion error by setting pointer to some marker value after you delete it the first time. Something like:

if (pointer) {
    delete pointer;
    pointer = NULL;
} else {
    error("Attempted to free already freed pointer.");
}

The problem you're going to run into here is that your overloaded -> operator returns the value of the pointer, which means whoever you return this to can also call delete, causing the check I propose above not to work. For example:

simple_ptr<int> myPtr = someIntPointer;
...
delete myPtr.operator->(); /* poof goes your pointered memory region */

I might recommend that you not rely on the operator-> overloading, and just require that those using this class call a method that dereferences the pointer internally before passing that value back to the user.

Hope this helps.

Answer 2

Is this implementation correct? I have made copy constructor as private ...

You could do the same for the assignment operator:

simple_ptr& operator=(const simple_ptr<T>& t);

A const version of the dereference operator might be useful too, and, smart pointers usually define the other kind of dereference operator as well:

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

If by chance the pointer is deleted, delete operation on the destructor will throw assertion error. How can I workaround this?

Do you mean, if I do this:

//create instance
Car* car = new Car;
//assign to smart ptr
simple_ptr<Car> ptr(car);
//explicit delete
delete car;
//... assertion when ptr is destroyed ...

A way (I don't know if it's a good way) to prevent that is to declare the constructor, and/or the destructor, and/or the delete operator of the T class as private, and say that simple_ptr is a friend of the T class (so that only the simple_ptr class can create and/or destroy and/or delete instances of T).

Marking the new operator as private in T class seems to be impossible as I have to modify all the classes which will be used with simple_ptr

Yes that's true: to do my suggestion immediately above, you would need to modify the class definitions.

If your question is "how can I make double deletes impossible, without modifying class definitions?" then I think the answers are:

• You can't: it's up the application code (which uses these classes) to be careful
• You can: by providing your own heap manager i.e. your own implementation of global operator new and global operator delete and, in your smart_ptr code, interrogate your heap manager to see whether this incarnation of this pointer is still allocated, before you delete it

Answer 3

I've seen sometimes usage of simple DISABLE_COPY macros:

#define DISABLE_COPY(Class) \
        Class(const Class &); \
        Class &operator=(const Class &);

So it's a common practice to define copy constructor and assignment operator as private for your task.

Answer 4

What you have done is boost::scoped_ptr

Please also read comment by j_random_hacker.

Answer 5

Please use boost::scoped_ptr<> as suggested by Martin York, because it:

• Does exactly what you want (it's a noncopyable pointer)
• Has no overhead above that of a standard C pointer
• Has been carefully crafted by super-intelligent C++ wizards to make sure it behaves as expected.

While I can't see any problems with your implementation (after applying the changes suggested by ChrisW), C++ has many dark corners and I would not be surprised if there is some obscure corner case which you, I and the others here have failed to spot.

Answer 6

You've got two choices:

1. boost::scoped_ptr already detailed by j_random_hacker, because it's non-copyable (doesn't share ownership like shared_ptr) and non-movable (doesn't transfer ownership like auto_ptr. auto_ptr has a copy constructor, but that one does not copy. It moves the original pointer to *this). boost::scoped_ptr is exactly what you need.
2. const auto_ptr doesn't allow transfer of ownership. And take your parameter by reference to const (auto_ptr<T> const&). If the writer of a function accepts by value instead, it still won't work if you try passing a const auto_ptr, because its copy constructor needs a non-const auto_ptr.

Until C++1x, boost::scoped_ptr is the best choice for your needs, or a const auto_ptr if you have to use official standard stuff (read this). In C++1x, you can use std::unique_ptr as a better alternative to auto_ptr, because you have to explicitly state when you want to transfer ownership. Trying to copy it will result in a compile time error. unique_ptr is detailed a little in this answer.

Answer 7

You should user a boost::scoped_ptr<> as has been mentioned already.

In general though, if you need to make a class non-copyable, you should inherit from boost::noncopyable, i.e.

#include <boost/utility.hpp>

class myclass : boost::noncopyable
{
    ...
};

This does all the work of making it non-copyable and is nicely self-documenting.