CODEWITHSUNDEEP
c++pointers
philm
philm

Reputation: 885

Error when deleting these pointers

I know that when I initialize a pointer with new, that I need to delete the pointer or a memory leak will occur. I have a class that is setup as such:

class foobar
{
private:

    //! Pointer to the global nodal list
    int *p_test1;

    //! Pointer to the global block label list
    int *p_test2;

public:
    foobar(int *test1, int *test2)
    {
        p_test1 = test1;
        p_test2 = test2;
    }

    ~foobar()
    {
        delete p_test1;
        delete p_test2;
    }
};

Now, when the destructor is called, the program crashes and the console says Program exited with return code -6.

The debug window states that:

Program Received signal SIGABRT

When I got through the call stack, one of the last items is the destructor. The stack appears to attempt to free the memory but fails.

I ma wondering why this is the case? And what is the preferred method on freeing the memory with the class setup as such.

As a side note, if I comment out the code in the destructor, the pointers will of course not be freed when the instance of the foobar class is finished. However, when I call another instance of the class (say that the creation of the instance is dictated by the user pressing a button on a GUI), then the program crashes. Again, why is this? I have a feeling that this is related to not properly destroying the pointers.

Upvotes: 1

Views: 348

Answers (1)

mksteve
mksteve

Reputation: 13073

As has been mentioned in the comments, there is no new in this example, so it makes it difficult to explain the balance between new and delete.

The rule is if you new a pointer you should delete it.

int * p = new int( 5 );

std::cout << "The value of p is " << *p << std::endl;

delete p;

However, if you use an object, as you didn't new it, you shouldn't (normally) delete it.

void somefunction( int * p )
{
    std::cout << "The value of p is " << *p << std::endl;

    delete p; // this looks smelly - deleting a pointer which was allocated.
}

To try and simplify this problem (which pointers should I delete), modern C++ has some good guidelines.

Resource Acquisition Is Initialization

RAII puts resources into a class. A full class (as opposed to a pointer), has a very clear lifecycle. When it is destroyed because it goes out of scope, then the destructor is called.

 class PointerHolder {
      int * mp;
   public:
      PointerHolder( int value ) : mp( NULL) {
          mp = new int( value );
      }
      ~PointerHolder() {
          delete mp;
      }
 }

Although the above example breaks the rule of 5.

Rule of 5 (or zero)

The problem with PointerHolder above, is it doesn't implement all of the 5 special operators.

PointerHolder a( 12 );
PointerHolder b( 10 );

a = b;  // What is going to happen?????

a gets the value of b's pointer, and when the second is destroyed, it is the second delete of the same piece of memory, and the program crashes.

The rule of 5 states, if you implement either the destructor, the move operator, the copy constructor, or operator = you should implement (or remove all of them).

In this case, removing the move and copy operators, would produce a simple unique_ptr.

The rule of zero, is that a class is more re-usable, if it implements all of the operators, or none of them.

If you need to implement only some of the operators, it is probable that you have not identified the "resource" - which should be a single class with all 5 overloaded operators.

Modern C++

Since C++11, the use of new and delete, is considered confined to library writers. As the use of std::shared_ptr and std::unique_ptr manage the new delete lifecycle for you, so you can focus on the problem at hand.

Upvotes: 2

Related Questions