Tukukkk

I have some pointers that I allocate in the constructor of a class and then attempt to delete in its destructor:

TileMap::TileMap(int x, int y) {



    mapSize.x = x;

    mapSize.y = y;



    p_p_map = new Tile*[x];



    for(int i = 0; i < x; i++) {



        p_p_map[i] = new Tile[y];



    }



    randomize();



}



TileMap::~TileMap() {



    for(int i = 0; i < mapSize.x; i++) {



        delete p_p_map[i];



    }



    delete p_p_map;



}



void TileMap::randomize() {



    for(int i = 0; i < mapSize.x; i++) {



        for(int j = 0; j < mapSize.y; j++) {



            p_p_map[i][j] = *new Tile(Tile::TileSize * i, Tile::TileSize * j, TileType::randomType());



        }



    }



}

At the end of the program the destructor is called to free the memory of the pointers I allocated, but when it reaches "delete p_p_map[i];" in the destructor, XCode informs me that the pointer was not allocated. I am new to C++, but I feel that I pretty explicitly allocated memory to the pointers in the randomize() function.

What error am I making?

You have to match delete with new and delete with new. Mixing one up with the other leads to issues. So if you do:

p_p_map = new Tile*[x];

you have to delete it like:

delete p_p_map;

and same with

delete p_p_map[i];

If you create something like:

pSomething = new Type;

then you delete it like:

delete pSomething;

What error am I making?

A few:

First, as @uesp pointed out, you mismatch new and delete calls

Second, you are using the "memory leak operator":

p_p_map[i][j] = *new Tile(Tile::TileSize * i, Tile::TileSize * j, TileType::randomType());

The construct new Tile(...) allocates memory. Then, this memory (not stored anywhere) is dereferenced, and the result is assigned to p_p_map[i][j].

Because the pointer is not stored anywhere, it is leaked.

Third, you are not respecting RAII. While this is not technically an error in itself, the way you write the code is unsafe, and in low memory conditions, you will get UB.

For example, here's what happens if you construct a Tile instance with large values for x and y:

TileMap::TileMap(int x, int y) { // e.g. (x = 1024 * 1024, y = 1024 * 1024 * 1024)



    mapSize.x = x;

    mapSize.y = y;



    p_p_map = new Tile*[x]; // allocate 1049600 pointers block



    for(int i = 0; i < x; i++) {



        p_p_map[i] = new Tile[y]; // run out of memory (for example) half way through the loop



    }



    randomize();

}

Depending where your allocations fail, your constructor will not finish executing, meaning your TileMap instance is "half-constructed" (i.e. in an invalid state) and the destructor will not be called.

In this case, everything the class allocated is leaked, and (especially if you allocated a large size) your application is left in low memory conditions.

To fix this, make sure each pointer is managed by a different instance of a class (part of RAII). This ensures that if an allocation fails, the allocated resources are released before exitting the scope, as part of stack unwinding (as @CaptainObvlious said, use std::vector for the array and std::unique_ptr for each element).