C++ Properly deleting dynamically allocated memory

Question

I was wondering if one of you could confirm that I'm deleting some dynamically allocated memory properly.

The TileWrapper below is initialized as a 2D array of pointers:

private:
    TileWrapper*** mLayout;

I've simplified its initialization to show you the important parts:

void generateLayout() {
    mLayout = new TileWrapper**[mRows];
    for(int i = 0; i < mRows; i++) {
        mLayout[i] = new TileWrapper*[mColumns];
    }
    for(int i = 0; i < mRows; i++) {
        for(int j = 0; j < mColumns; j++) {
            mLayout[i][j] = new TileWrapper();
        }
    }
}

The part that I need confirmed is the destruction, shown below:

~Destructor() {
    for (int i = 0; i < mRows; i++) {
        for (int j = 0; j < mColumns; j++) {
            delete mLayout[i][j];
        }
        delete[] mLayout[i];  // CONFIRM THIS
    }
    delete[] mLayout;  // CONFIRM THIS
}

I'm especially concerned about the deletes that have // CONFIRM THIS afterwards due to the [] characters. Is my code memory-leak proof? Thanks.

Yes, it seem to be memory-leak proof. You can check it by using memory-check tools like Valgrind or DrMemory — Hugal31, Dec 22 '16 at 18:37
@molbdnilo I'm aware, I didn't want to use it since I don't need any of the functions that come with it. — , Dec 22 '16 at 18:40
You need to learn about smart pointers. In modern C++ (C++11,14 and newer) manual `delete`'s are pretty much a code smell indicating that you are probably "doing it wrong". — Jesper Juhl, Dec 22 '16 at 18:41
@Adrian Bobrowski it's not a 2D array, that would be a student error to consider it so. It's array of pointers to arrays of pointers. 2d array is array of arrays, there is a semantics difference between them — Swift - Friday Pie, Dec 22 '16 at 18:48
@AdrianBobrowski - but you **do** want to use some of the functions that come with `std::vector`, in particular, the built-in memory management. — Pete Becker, Dec 22 '16 at 19:03
I agree with Pete and Adrian. It's one of textbook signatures of a bad programmer if he discards standard, defined by language specs, library tools because "they have something he doesn't need", not because "it works not in same way as I need them" or "it doesn't work for my case", and creates his own, half-cocked version of standard library. By the way, it is inline template, the code you won't use DOES NOT EVEN EXIST. Code of template classes is inline and generated by compiler when it is called. — Swift - Friday Pie, Dec 22 '16 at 19:38
You also need to make sure you obey the rule of three. But it would be much easier to use `std::vector<>` instead and not do the memory management. — Martin York, Dec 22 '16 at 21:04
@Anthroyd: [Rule of Three](http://stackoverflow.com/documentation/c%2b%2b/1206/the-rule-of-three-five-and-zero/9867/rule-of-three#t=201612222324075718052) — Martin York, Dec 22 '16 at 23:24
@Anthroyd: Use the copy swap idiom (I am sure you can look that one up). — Martin York, Dec 22 '16 at 23:30

Swift - Friday Pie · Answer 1 · 2016-12-22T19:32:29.473

It is safe and sound as far as displayed code concerned. The confusion may arise from way the second for loop that is used. It is actually an equivalent of

for(int i = 0; i < mRows; i++) {
    for(int j = 0; j < mColumns; j++) {
        *(*(mLayout + i) + j) = new TileWrapper();
    }
}

The construction with triple pointer got one disadvantage known even in time of K&R: it isn't guaranteed to occupy monolithic area of memory because you allocate each "column" separately in your code, so it is not a multidimensional array of pointers by memory model, which should declared to users of this "array". In C++ it isn't possible to allocate whole array in one go AND use subscripts for it without creating helper template types or redefining operator[]

C++ Properly deleting dynamically allocated memory

1 Answers1