CODEWITHSUNDEEP
c++memory-managementdynamic-memory-allocation
RectangleEquals
RectangleEquals

Reputation: 1825

Will using delete[] like this end in a memory leak?

I'm sure this has been asked before, but the search terms are failing me for my specific situation.
Essentially, I'm creating a 2D tile engine. For overall efficiency and to reduce a bit of overhead, I have decided to implement my own simple memory management container...

The following code is what I'm using to do so:

Tile.h:

//Tile class
class CTile
{
public:
    CTile();
    ~CTile();
    unsigned int row;
    unsigned int column;
};

//Container class
class CTileLayer
{
public:
    CTileLayer(unsigned int nRows, unsigned int nCols, float w, float h);
    ~CTileLayer();

protected:
    CTile** m_tiles;
    unsigned long count;

    void Allocate(unsigned int nRows, unsigned int nCols, float w, float h);
    void Free();
};

Tile.cpp:

#include "Tile.h"


CTile::CTile() : row(0), column(0)
{
}

CTile::~CTile()
{
}


CTileLayer::CTileLayer(unsigned int nRows, unsigned int nCols, float w, float h) : m_tiles(NULL)
{
    Allocate(nRows, nCols, w, h);
}

CTileLayer::~CTileLayer()
{
    if(m_tiles != NULL) Free();
}

void CTileLayer::Allocate(unsigned int nRows, unsigned int nCols, float w, float h)
{
    unsigned int column = 0, row = 0;

    if(m_tiles != NULL) Free();
    m_tiles = new CTile*[count = (nRows * nCols)];

    for( unsigned long l = 0; l < count; l++ ) {
        m_tiles[l] = new CTile[count];
        m_tiles[l]->column = column + 1;
        m_tiles[l]->row = row + 1;
        //
        //...
        //
        if(++column > nCols) {
            column = 0;
            row++;
        }
    }
}

void CTileLayer::Free()
{
    delete [] *m_tiles;
    delete [] m_tiles;
    m_tiles = NULL;
    count = 0;
}

So by seeing how each tile is allocated/freed, is it safe to say this won't produce any memory leaks?

If it will, what would be the most appropriate way to free this array, while making sure each object's destructor gets called? Should I loop through the array, manually deleting each object instead?

Upvotes: 1

Views: 119

Answers (2)

Ben Voigt
Ben Voigt

Reputation: 283614

You called new CTile[count] count times, once for each m_tiles[l].

You need to therefore call delete[] count times, for each m_tiles[l].

It isn't clear what count is good for, however. You should be using nRows and nColumns for the two layers of the array. Right now you have nRows * nRows * nColumns * nColumns instances of CTile allocated, which is probably too many.

Instead try

m_tiles = new CTile*[nRows];

m_tiles[l] = new CTile[nColumns];

Upvotes: 2

huysentruitw
huysentruitw

Reputation: 28091

Why not do this:

CTile* m_tiles;

m_tiles = new CTile[nRows*nColumns];

delete[] m_tiles;

Upvotes: 3

Related Questions