开发者

Where are the memory leaks?

开发者 https://www.devze.com 2023-03-05 17:59 出处:网络
I have this piece 开发者_如何学Goof code and for some reason it creates 6 memory leaks and I can\'t figure out where.

I have this piece 开发者_如何学Goof code and for some reason it creates 6 memory leaks and I can't figure out where.

    Lemon::Lemon()
{
    this->nrOf=0;
    int x=10;
    this->matrix=new int*[x];
    this->lines=new string[x];
    for (int i = 0; i < x; ++i) 
        this->matrix[i] = new int[x];
    for(int i=0;i<x;++i)
        for(int j=0;j<x;++j)
            this->matrix[i][j]=-1;

}
Lemon::Lemon(int n)
{
    this->x=this->nrOf;
    this->matrix=new int*[x];
    this->lines=new string[x];
    for (int i = 0; i < x; ++i) 
        this->matrix[i] = new int[x];
    for(int i=0;i<x;++i)
        for(int j=0;j<x;++j)
            this->matrix[i][j]=-1;
}

Lemon::~Lemon()
{
    for(int i=0;i<this->nrOf;i++)
    {
        delete []this->matrix[i];
    }
    delete []this->matrix;
    delete []this->lines;
}

Any kind of help is appreciated.


At this point:

this->x=this->nrOf;

nrOf has not been initialised - you have undefined behavior.

And possibly you would benefit from using std::vector.


The default constructor initializes nrOf with 0, but you allocate a matrix of 10x10. Which means the destructor will not run the for loop, since nrOf is 0.

BTW, you don't have to prefix everything with this. It kinda makes the code harder to read. You should use this only to eliminate ambiguity.


You don't set nrOf to the number of allocated matrix rows in either constructor.


Try this as your only constructor

Lemon::Lemon(int n = 10)  // default constructor
{
  nrOf = n;

  matrix = new int*[nrOf];
  lines = new string[nrOf];

  for (int i = 0; i < nrOf; ++i) 
    matrix[i] = new int[nrOf];

  for(int i=0; i<nrOf; ++i)
    for(int j=0; j<nrOf; ++j)
        this->matrix[i][j] = -1;
}


You seem to have both a member variable int x (used in Lemon::Lemon(int n)) as well as a local variable int x in Lemon::Lemon(). Also, you do not consistently set nrOf in your constructors.


Note 1:

Every new-expression in your code is possible memory leak because your code is awfully exception unsafe: if any of your new expressions throw an exception all your already new-allocated storage is leaked.

Note 2:

Your first constructor does not semantically match your destructor (even if we assume that everything goes fine without throwing): constructor assigns zero value to this->nrOf but allocates 10 elements of this->matrix array and assign allocated array to each element BUT destructor expects this->nrOf elements of this->matrix (arrays) to delete so 0 elements is deleted and thus 10 elements are leaked.

Side note: your this->x (that exists according to second constructor) is not initialized by first constructor.

Note 3:

Your second constructor is simply corrupt: you never initialize this->nrOf variable but you use it like it contains something valid.

ADD:

Good advise here would be: never use arrays and new/new[]/delete/delete[] expressions. Use std::vector in your situation instead. This way you may never care about memory leaks, exception safety, etc.

Your matrix would look like this:

std::vector< std::vector<int> > matrix;
....................
....................
n = 10;
m = 20;
matrix.resize(n);

for( size_t j = 0; j < n; ++j )
{
    matrix[j].resize(m);

    for( size_t k = 0; k < m; ++k )
    {
        matrix[j][k] = j+k;
    }
}
0

精彩评论

暂无评论...
验证码 换一张
取 消