CODEWITHSUNDEEP
c++visual-studio-2010
user3314899
user3314899

Reputation: 53

Improper de-allocation of memory?

When testing my code, I consistently get errors regarding the use of delete as my class's testbed states that delete is being called on an array allocated with new[]. I have delete in my ~IntVector and two expand functions with the expand functions expanding the capacity while reallocating memory for a dynamically allocated array.

How do I use delete correctly to prevent memory leaks and resolve this error?

main file

#include "IntVector.h"
#include <iostream>
#include <vector>
using namespace std;


IntVector::~IntVector(){
    delete[] data;
}

void IntVector::expand(){
    cap = cap * 2;
    int *data2 = data;
    data = new int[cap];
    data = data2;
    delete data2;
    delete[] data2;
}

void IntVector::expand(unsigned amount){
    cap = amount;
    int *data2 = data;
    data = new int[cap];
    data = data2;
    delete data2;
    delete[] data2;
}

header

#ifndef INTVECTOR_H
#define INTVECTOR_H

using namespace std;
class IntVector{
private:
    unsigned sz;
    unsigned cap;
    int *data;
private:
    void expand();
    void expand(unsigned amount);
};

#endif

Upvotes: 0

Views: 152

Answers (3)

Zac Howland
Zac Howland

Reputation: 15870

In addition to the rule of there violation, you are also attempting to delete variables twice in your expand functions:

void IntVector::expand()
{
    cap = cap * 2;
    int *data2 = data;
    data = new int[cap];
    data = data2;
    delete data2; // this should not be here!
    delete[] data2; // this will be a problem now!
}

You can only delete data once, and if you created something with new[], it needs to be deleted with delete[].

Both of your expand functions should look more like:

// copy-swap
void IntVector::expand()
{
    IntVector tmp;
    tmp.reserve(cap * 2);
    tmp.resize(sz);
    std::copy(data, data + sz, tmp.data);
    std::swap(*this, tmp);
}

or 

// raw implementation
void IntVector::expand()
{
    unsigned int newCap = cap * 2;
    int* newData = new int[newCap];
    std::copy(data, data + sz, newData);
    delete [] data;
    data = newData;
    cap = newCap;
}

The copy-swap version would let you reuse the other functions (destructor, copy-assignment operator) and will be more exception safe. The raw implementation should not modify your internal data elements until the new ones are already properly created. This prevents an exception that may be thrown in the new from leaving your vector in a bad state (e.g. when the cap is not actually your capacity).

Upvotes: 0

Some programmer dude
Some programmer dude

Reputation: 409482

When allocating with new[] you have to use delete[]. Your expand function uses plain delete. It also contains some other errors (reassigning pointers, double deletion, etc.).

And where's your copy-constructor? Copy-assignment operator? You might want to read about the rule of three.

Upvotes: 3

Luchian Grigore
Luchian Grigore

Reputation: 258698

You're probably running into this issue because you're not obeying the rule of three - you need a copy constructor and an assignment operator in your class that does a deep copy.

If you do something like

IntVector x(IntVector(10));

you'll be left with a dangling pointer in x, because the original is de-allocated when the temporary IntVector(10) goes out of scope.

Upvotes: 2

Related Questions