2

Theoretically, is it enough if I only delete arrays and results but don't delete sub_array inside of arrays in the following code snippet, before return 0; or do I have to first delete all the sub_array before I can safely delete arrays and results.

int main() {

    int subarrays, queries;

    cin >> subarrays >> queries;

    int** arrays = new int* [subarrays]();

    int* results = new int[queries];

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

        int length;
        cin >> length;

        int* sub_array = new int[length];

        for (int j = 0; j < length; j++) {

            int element;
            cin >> element;

            sub_array[j] = element;
        }

        arrays[i] = sub_array;
    }

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

        int query_from, arr_index;
        cin >> query_from >> arr_index;

        results[i] = arrays[query_from][arr_index];
    }

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

        cout << results[i] << endl;
    }

    return 0;
}
sasieightynine
  • 434
  • 1
  • 5
  • 16
  • 1
    `for (int i = 0; i < queries; i++) { delete[] arrays[i]; } delete[] arrays; delete[] results;` – David C. Rankin Aug 20 '19 at 03:27
  • An array of arrays is very rarely what you want (despite the fact that every C++ beginner learns first about `char** argv`). If, for example, you need a data structure with variable-length elements that can be removed, you might want a linked list if they’ll be accessed in order, or a hash table if you need constant-time random access. (Although you could implement a kind of unordered map this way, if the map is very dense.) You might also be able to use a “rectangular” array of fixed dimension. If you need a sparse matrix, compressed sparse row is more efficient. – Davislor Aug 20 '19 at 03:40
  • If you do need this particular data structure, it should be a `std::vector` or something. – Davislor Aug 20 '19 at 03:41

2 Answers2

5

You shouldn't delete sub_array because the buffer it points to is stored in arrays[n]. As the only way you could delete sub_array is inside the block scope it was declared (which is the for loop), if you do delete it then arrays[n] will be a dangling pointer (a pointer that points to a freed or invalid memory block) and dangling pointers are evil.

What you should do about arrays and sub_array is when they are not needed anymore, you first iterate through all elements of arrays, deleting each one of them and after this, delete arrays itself. Something like:

// when you are done with them
for(auto i = 0; i < subarrays; i++)
{
    delete[] arrays[i];
}

delete[] arrays;

As a side-note you should consider using smart pointers in this code.

andresantacruz
  • 1,676
  • 10
  • 17
  • 1
    @DavidSchwartz OP saves that pointer at `arrays[i] = sub_array;`. Therefore deleting `arrays[n]` will free the memory allocated by `int* sub_array = new int[length];` – andresantacruz Aug 20 '19 at 03:47
1

Yes, you have to first delete all the sub_array before I can safely delete arrays and results to prevent memory leak.

You can employ a loop to go through the sub-arrays in the array.

Edward Aung
  • 3,014
  • 1
  • 12
  • 15