C++ Will this cause a memory leak?

Question

I'm trying to understand the use of delete[]. Will the following code have a memory leak?

int * getArray( )
{
    int *r = new int[10];
    for (int i = 0; i < 10; ++i)
        r[i] = i;
    return r;
}

int main ()
{
    int *array;
    for ( int i = 0; i < 10; i++ ) // main loop
        array = getArray();
    return 0;
}

The main loop seems to be allocating memory for 10 arrays, where only the last array has a valid pointer. If this is a memory leak, how do I free the memory storing the previous 9 arrays?

I can also compile using const int r[10]; in place of int *r = new int[10];. Can the use of const avoid a memory leak?

You should get into the habit of using a tool to determine if there is a memory leak like [`valgrind`](http://valgrind.org/). — IKavanagh, Aug 12 '15 at 18:48
In addition to the answers, read up about RAII to deal with memory management — Niall, Aug 12 '15 at 19:01

score 6 · Accepted Answer · edited May 23 '17 at 10:26

Each call to new must be paired with a call to delete to avoid a memory leak (or new[] and delete[]).

How to delete the memory depends on what you're doing but since you don't keep access beyond the for-loop then you could easily delete it within the body of the for-loop.

for (int i = 0; i < 10; ++i)
{
    array = getArray();
    /* do  something with 'array' */
    delete[] array;
}

Better yet, why not use std::vector<int> and avoid directly performing any memory allocation?

Edit: As @Niall mentions in the comments you should also familiarize yourself with std::unique_ptr (assuming you're compiler supports it). It uses RAII (Wikipedia link, cppreference link) to automatically handle the memory management. For example, std::unique_ptr<int[]> which will automatically call delete[] once the class goes out of scope.

Fantastic Mr Fox · Answer 2 · 2015-08-12T19:54:04.093

You have a memory leak if you dont call a delete for every new (or new[] and delete[]).
You need to keep a track of your allocated memory, this loop:

 for ( int i = 0; i < 10; i++ ) // main loop
    array = getArray();

will create memory that you cannot recover the address of (for the first 9 arrays) as you have said. There is no way to get the address back so you can properly handle this memory. You need to do something like this:

int *array[10]
...
  for ( int i = 0; i < 10; i++ ) // main loop
    array[i] = getArray();

so that you can then delete all of your arrays correctly:

   for ( int i = 0; i < 10; i++ )
        delete[] array[i];

score 1 · Answer 3 · edited May 23 '17 at 11:58

As other answers explained, you have a memory leak (and using const won't help about them).

Several systems (notably Linux) have valgrind ported to them. If possible use it (to test at runtime). So compile your C++11 code with g++ -std=c++11 -Wall -Wextra -g into a ./myprog executable, then run valgrind ./myprog; it will give you useful diagnostics at runtime.

With recent GCC you could also pass -fsanitize=address to the compiler. Read more about the address sanitizer.

BTW, you should often use smart pointers in your C++ code, e.g. std::shared_ptr or std::unique_ptr. Read also about RAII, weak references, and the rule of five.

You might also learn more about garbage collection techniques (reference counting is a quite limited one, unfriendly for cyclic graphs, but good enough for DAGs). You could consider using Boehm's conservative garbage collector. See also this.

score 1 · Answer 4 · answered Aug 12 '15 at 18:54

1 - You do have memory leaks. To solve it you can explicitly delete after you're done with each array.

for ( int i = 0; i < 10; i++ ) {
    int *array = getArray();
    // ... Do stuff ...
    delete[];
}

Or return a unique_ptr for arrays. That way you don't have to delete anything.

std::unique_ptr< int[] > getArray( )
{
    std::unique_ptr< int[] > r{ new int[10] };
    for (int i = 0; i < 10; ++i) {
        r[i] = i;
    }
    return std::move( r );
}

In both cases you can't guess the size of the arrays. You just know you're getting an array of 10 elements.

You could use an std::array to manage the memory for you and keep the length of the array visible.

2 - If you use static you'll always return the same array and there's no need to delete.

thanks, this is helpful. Sorry, on the second question I meant to write `const` rather than `static`. — Andrew, Aug 12 '15 at 19:15

C++ Will this cause a memory leak?

4 Answers4