C++ pointers again, what happens, how safe?

Started by
13 comments, last by Zahlman 16 years, 6 months ago
Quick questions about raw pointers and safety...what happens when:

// Example 1.  What happens to the pointer and allocated memory here?
template<typename T>
void function_that_takes_ptr(T *ptr)
{
    ptr->doSomething();
}

function_that_takes_ptr(new thing);

// Example 2.  Is this pointer behaviour 'perfectly' safe?
template <typename T>
class raw_ptr_class
{
    public:
       ~raw_ptr_class()
       {
           delete ptr;
       }

    private:
       T *ptr;
};

void function_uses_vector_of_ptr_classes(const T &arg)
{
    std::vector<raw_ptr_class<T> >vec;
    // Do something with vector and arg.
}

std::string strg;
function_uses_vector_of_ptr_classes(strg);
I know that auto_ptr<>(), boost::<various pointers> are safer, but I find that passing around auto_ptrs etc can get quite troublesome, and raw pointers are sometimes the only choice, so I'm trying to better understand what's going on 'under the hood'. --random
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.
Advertisement
When dealing with pointers, it's essential to determine ownership. Who has the responsibility to delete it when it's no longer needed?
In your code, ownership starts out at the call site, but is transferred to the function you call since there's no way to hold on to the pointer at the call site.
Thus, it's now function_that_takes_ptr that owns the pointed to memory, and it has to either give some other entity ownership of it, or clean it up.
Unless doSomething() assumes ownership of the pointer it's invoked with (unlikely), you will have a leak.

The class in your second example has several flaws. It doesn't have a copy constructor, nor assignment operator. It also doesn't initialize the pointer, so upon destruction it will attempt delete an indeterminate memory position.
Note that deleting a null pointer is valid and a no-op, but deleting an uninitialized pointer is UB.

To make it is hell. To fail is divine.

Thanks for the info...OK, in the example 2 I assumed that the compiler would automatically create all the copy ctors and assignments, but I get your point about it not being intitialized. How about this modification...

// Example 2.  Is this pointer behaviour 'perfectly' safe?template <typename T>class raw_ptr_class{    public:       raw_ptr_class():ptr(new T){ }       ~raw_ptr_class()       {           delete ptr;       }    private:       T *ptr;};void function_uses_vector_of_ptr_classes(const T &arg){    std::vector<raw_ptr_class<T> >vec;    // Do something with vector and arg.}std::string strg;function_uses_vector_of_ptr_classes(strg);


Also, are you saying in Example 1 that when the pointer goes out of scope there will be a potential memory leak, or will the pointer be deleted, as any other variable would be removed from memory when the function goes out of scope? In other words, is it necessary to explicitly delete (and zero) pointers that are passed to functions in this way?

Edit: This is another question -- 'good practice' with pointers advises that the pointer be deleted and set to zero, no? So:

T *ptr;ptr = new T;// ...do something...delete ptr;ptr = 0;


But in the case of the destructor of a class, I've been taught that only delete is required, and resetting pointers is not required. Why?

--random


[Edited by - random_thinker on October 3, 2007 6:40:13 AM]
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.
there is no reason to set a deleted pointer to 0. It will only give you a false sense of security. Your new code is much better but still has errors. Your raw pointer class needs copy/assignment operators or it is not safe. You see look at this example:

raw_ptr_class<int> a;
raw_ptr_class<int> b;
a=b;

You have a memory leak AND a double deletion. You see "a" creates and object and assigns it to a.ptr, b does the same for b.ptr. Then when you assign b to a then both ptr's point to the object created by b. Who is pointing to the object created by a? No one. That is a memory leak. Then it comes time for the destructors. b's destructor gets called and everything is fine. However when a tries to delete the object that it shared with b, well things are not fine and your program crashes
When all pointers to an object with dynamic storage go out of scope, the object is lost forever, thus creating a memory leak.
Local variables have automatic storage, and are cleaned up when their scope is exited.
Objects with dynamic storage should be deleted once. That is why determining ownership is important, so that the object is not deleted zero, two or more times, which is an error.

As for assigning null to deleted pointers, it's mostly a matter of preference.
Some argue that it helps avoid using invalid pointers, but one shouldn't use pointers not pointing at anything at all.
There may also be more than one pointer to the object, and you would then have to null all out to be consistent.

To make it is hell. To fail is divine.

Rule of three, if you need any of destructor, overloaded assignment operator or copy constructor, then you probably need all of them. If you wish, you can make operator=() and the copy constructor private (and unimplemented).

Your raw_ptr_class is not safe for use in a std::vector because it doesn't obey proper copy construction.
Thanks for the information, it's helping greatly. But coming back to the issue of class member pointers, copy ctors and assignment operators, I was not aware of such a problem, thank you for educating me on this.

I am surprised that the pointer does not need to be reset to zero (or NULL). So many websites/books recommend this approach.

Also, I use this technique with composition and pimpl type classes. So what you are telling me now, is that there is likely a gaping hole in these classes, and that I either need to declare copy ctors and assignment operators private (use boost::noncopyable) or go with an auto_ptr type technique.

--random
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.
Just to summarize, to make sure that I understand:

1. Whoever owns the pointer, is responsible for deleting it when it is no longer needed.
2. Classes with raw pointers must carefully control creation, copying, assignment and deletion.
3. When deleted, there is no need to reset to zero or NULL.
4. Use std::auto_ptr<> or boost::<various pointers> whenever possible.

Do you think that would solve the majority of pointer-related memory leaks?

--random
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.
Quote:Original post by random_thinker
Just to summarize, to make sure that I understand:

1. Whoever owns the pointer, is responsible for deleting it when it is no longer needed.
2. Classes with raw pointers must carefully control creation, copying, assignment and deletion.
3. When deleted, there is no need to reset to zero or NULL.
4. Use std::auto_ptr<> or boost::<various pointers> whenever possible.

Do you think that would solve the majority of pointer-related memory leaks?

--random


It isn't perfect. A problem with boost::shared_ptr this is circular references. One must be extremely careful with std::auto_ptr as it has unusual (and arguably unintuitive) copy semantics.

When you reset a pointer, only set the pointer to NULL if it is to be used again. A simple example might be a lemmings game, you could keep a pointer (or pointer object like boost::shared_ptr/weak_ptr) to the currently selected lemming somewhere, or a NULL pointer if no lemming is currently active.

If the object isn't intended to be used again there is a possibility, as mentioned, that logically incorrect double deletes will go undetected.
Thx RipOff--

I suppose it is also a good idea to always ensure that pointers are initialized when they are declared as well, to add to the above list.

--random
--random_thinkerAs Albert Einstein said: 'Imagination is more important than knowledge'. Of course, he also said: 'If I had only known, I would have been a locksmith'.

This topic is closed to new replies.

Advertisement