CODEWITHSUNDEEP
c++constructorheap-corruption
Snowman
Snowman

Reputation: 1543

Heap Corruption Detected - class with pointers

The following code, results in an Assertation error. In addition, a warning error message indicates that heap corruption is detected.

class A {
    int* a; // dynamic array of ints
    A() {};
    A(int size) {
        a = new int[size]; 
    }
    ~A() {
        delete [] a;
        a = nullptr;
    }
}

*** in code somewhere ***
int size = 5;
A temp = A(size);

Upvotes: 1

Views: 1730

Answers (2)

Snowman
Snowman

Reputation: 1543

The cause of the error is:
A temp = A(size);
 This line calls:

  1. A's copy constructor, here: A temp = A(size); The problem is, that this creates a shallow copy, since it uses the default copy constructor, and we have a pointer in the class, which needs a deep copy!
  2. A's parametrized constructor, here: A(size);
  3. A's destructor, which will delete our pointer created by A temp and null it.

Then when temp variable goes out of scope, its Destructor will be called again, and thus result in a Assertation fail.

Solutions:

 1. A temp(size);
instead of A temp = A(size);
this will only call the parametrized constructor.
or
2. Overwrite the default copy constructor to create a deep copy!

Another correction by Marco Costa
It is better to initialize a to nullptr in the default constructor.

Another correction by user4581301
Destructor should check, if a is a nullptr, before deleting.

Additional readings:
1. Why aren't pointers initialized with NULL by default?
2. Rule-of-Three becomes Rule-of-Five with C++11? suggested by Chad
3. scalar deleting destructor issue suggested by Chad

Upvotes: 5

PaulMcKenzie
PaulMcKenzie

Reputation: 35440

As others have stated, you need to follow the rule of 3 (or 5).

However, there is another issue with your class, and that is you did not record the number of elements. This is essential if you want to implement these functions correctly. There is no standard way to retrieve the number of items you used to call new[] with, thus you need to record this size value as a member.

class A {
    int* a; 
    int aSize;  // records the number of elements.
    A() : aSize(0), a(nullptr) {}
    A(int size) : aSize(size) 
    { a = new int[size]; }

//...
};

You need to add the size member to your class, otherwise you can't implement the copying functions that are required.

Upvotes: 1

Related Questions