Smart Ptr
It’s a really small smart pointer. 9 lines of code. Of course, I had to cut a few corners (newlines;) to make this happen. But, the readability is maintained.
It doesn’t do much other than being smart. Fit for trivial tasks, such as tracking and deleting local memory allocations, avoiding uninitialized references, etc. It’s so small, it cannot count references. But, it does deallocate when it goes out of scope.
Features:
1. Deallocation when the smart pointer goes out of scope.
2. Invalid reference check.
3. Not dependent on any library (including STL). Useful when you need a small binary footprint.
Restrictions:
1. No reference counting. So, avoid assigning one smart_ptr to another. (Enable test2 in the attached program to see the problem.)
2. Works only with “new” and not with “new []” or “malloc”. Needs further templatization for that to be handled. But, kills the simplicity of the current class.
Code follows:
template<typename T>
class smart_ptr
{
public:
T* operator->() {if(ptr) return ptr; throw -1;}
T& operator*() {if(ptr) return *ptr; return NULL;}
smart_ptr() {ptr = NULL;}
smart_ptr(T* p):ptr(p) {}
~smart_ptr() {if (ptr) delete(ptr); ptr = NULL;}
smart_ptr& operator=(T* p) {if(ptr) delete(ptr); ptr = p; return *this;}
bool operator==(const T& p1) {return (p1.ptr == ptr);}
bool operator==(const int i) {return (i == (int)ptr);}
protected:
T* ptr;
};
Command line for VC++ compiler to build this program:
1. Open Visual Studio command prompt
2. Compile and Link: >cl smart.cpp
3. Run: >smart.exe
Command line for g++ compiler to build this program:
1. Compile and Link: >g++ smart.cpp -o smart
2. Run: >./smart
For usage, see the complete program: smart.cpp
9 lines of code, so many flaws:
1. You haven’t declared a copy constructor, your compiler will provide a default cc automatically and this will allow your pointer to propagate out of scope, leading to double free’ing and segfaults.
2. No assignment operator taking another smart_ptr reference, the compiler will provide one automatically. Same problem as above.
3. Your assignment operator doesn’t check if “ptr == p”.
4. Casting a pointer to “int” in operator== is not portable.
5. You don’t need to check for a null pointer with “if (ptr){” before calling delete.
6. NULL is a C’ism. You should use “0″ in C++ code.
7. operator* can’t return NULL since NULL is a pointer, not a reference. This shouldn’t even compile! Did you even test it? If anything you would return *(NULL) but returning a reference to null is undefined behaviour and badness. Throw an exception instead.
8. Many of your operators should be declared as “const” functions.
…and that is just what I can see right now
Andrew
19 Oct 08 at 7:41 pm
There is no need to conditionally invoke delete operator test if pointer is not equal to zero, delete does that already therefore deleting null pointer is a no-op.
valdemar
19 Oct 08 at 8:12 pm
A few implementation concerns, and a huge design one.
First, the design problem: you should not be writing this code. Writing a simple smart pointer is something which has been done a thousand times by people much smarter than you or I. Go start at the standard library (auto_ptr), or look at the many C++ utility libraries out there (e.g. boost’s complementary scoped_ptr). If you really thing there is some fatal flaw to them, you should clearly explain and prove that the problem exists (referring to binary footprint in a list doesn’t count).
To restate: If you’re really doing this to minimize binary footprint at the cost of introducing memory management bugs, you’re almost certainly making a design mistake. The standard library’s implementations of auto_ptr have a lot of thought and optimization put into it, at the same time following a well-understood semantics which are carefully designed to minimize mistakes, Furthermore, the design of the standard library smart pointers continue to evolve with the cutting edge as compiler advances enable improvement. See C++0x’s unique_ptr (a replacement for auto_ptr) for an example of this.
Now, the implementation problems, in order of significance:
1) Returning a NULL reference in operator* is a big no-no. References are guaranteed to be non-null, which affects the way a caller deals with your returned value in important ways, but you violate this important guarantee. Instead you should throw if the pointer is null.
2) You should be able to delete a null pointer without any ill effects, no need to test there.
3) Also, you may as well use null rather than NULL, as the former is a C++ language keyword, with a standardized meaning.
Finally, I could be wrong on any of the points above, so don’t take it as gospel. And if you accept that I’m right, don’t be discouraged, be glad someone is out there to set you right.
Ben W.
20 Oct 08 at 12:59 am
Please have a look at std::auto_ptr, it’s nearly the same, but with additional features ala support for assignment.
schula
20 Oct 08 at 2:57 am
Your “if (ptr) delete(ptr);” code can be replaced with “delete(ptr);”.
delete already checks for NULL pointers, see:
http://en.wikipedia.org/wiki/Operator_delete
Andrew
20 Oct 08 at 8:19 am
Plus your implementation is very nasty if copied.
You should declare your copy constructor and assignment operator to be private to prevent problems.
Andrew
20 Oct 08 at 8:21 am
1: You should have const overloads for the dereference and arrow operators. Your operator-> should not throw -1; but should throw something derived from std::exception.
2: The dereferencing operator should always return a valid reference, which NULL is not. This function should also throw something derived from std::exception if it encounters an invalid pointer. I’m actually surprised that compiles in the first place.
3: The overload of operator==(int) should not even exist, comparison with other pointer types should be handled by a template function. In fact, this is pretty much UB, as pointers are not integers, and should not be treated as such.
4: In addition, your operator==s should be const functions, as they should not modify the underlying object.
5: You should provide assignment operators for other shared pointers as well.
Desu
20 Oct 08 at 11:35 am
I’m overwhelmed with the feedback. I had put together this class in 15 min. That was quick dirty. I knew the quick part and with the feedback I’ll clean up the dirty part. Repost pending.
Thanks.
admin
21 Oct 08 at 1:19 pm