2

In my class I have a dynamically allocated array of pointers. My declaration:

 array = new Elem* [size];
 for (int i = 0; i < size; i++) {
    array[i] = NULL;
 }

So there is an array of pointers, where each pointer points to a simple Elem struct.

The main question is, how should I properly deallocate the array. If I use only:

for (int i = 0; i < size; i++) {
   delete array[i];
}

Valgrind reports 1 not-freed block, which is traced to the line where 'array = new Elem* [size];' states.

On the other hand if I add to the previous code:

delete array;

Which I thought is correct, valgrind reports 0 not-freed blocks, which is perfect, BUT it reports 

Mismatched free() / delete / delete []

exactly on the line where 'delete array;' is. I tried 'delete []array' too, but that's just "1 not-freed blocks" too then! If somebody could explain me the proper way it would be much appreciated.

EDIT: So using:

for (int i = 0; i < size; i++) {
   delete array[i];
}
delete[] array;

is working probably fine. It is working in one of my classes (I have two similar) the other still reports some small leak. I would think it's just a minor bug somewhere, but valgrind still points to the line where 

array = new Elem* [size];

stands.

EDIT2: I solved this as well, thank you for your exhausting contribution!!

Improve this question
6
  • 2
    Where does it say that not freed block got allocateD? Commented Nov 3, 2011 at 19:36
  • Make your life easier: delete all of the above, and use Boost ptr_vector. Commented Nov 3, 2011 at 19:53
  • I know this doesn't answer your question, but is there some reason you can't use a std::vector<Elem> to greatly simplify the memory management? Commented Nov 3, 2011 at 19:54
  • Indeed there is, it's a school work and we are not allowed to use that, probably the point is to learn how to do this properly. Commented Nov 3, 2011 at 19:56
  • @aGr: Would "homework" tag be appropriate in this case? It might keep others from pushing the vector angle as well. :) Commented Nov 3, 2011 at 19:59

4 Answers 4

Reset to default
8

You need:

delete [] array;

Because it's an array.

I just noticed your note that you tried this too - it's the proper thing to do so I don't know why you'd still be getting an error.

Edit: This deserves a more thorough explanation.

When you create a pointer using new, the pointer may be to a single element or an array of elements depending on the syntax you use. But the pointer type is the same in both cases! The compiler relies on you to know what the pointer points to and treat it accordingly.

Elem ** single = new Elem*;    // pointer to one pointer
single[0] = new Elem;          // OK
single[1] = new Elem;          // runtime error, but not compile time

Elem ** array = new Elem* [2]; // pointer to array of pointers
array[0] = new Elem;           // OK
array[1] = new Elem;           // OK

When you delete a pointer, the destructor is called for the object it points to or for each element of the array. But since the pointer type is the same in each case, the compiler relies on you to give it the proper syntax so it knows what to do.

delete single;
delete [] array;

In your case the elements of the array are pointers also, and pointers don't have destructors. That means those pointers won't be deleted and will become memory leaks if you don't delete them first. You were correct to have a loop to delete them individually before the final delete.

Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

The OP does need delete [] but I wouldn't call it an array.
@cnicutar, if it's not an array then what would you call it? The new [] syntax allocates an array.
@cnicutar, yes, a pointer to an array. I hope we're in agreement now.
It's an array (well, a pointer to one). Although it differs from the "array type"
@cnicutar, I must correct myself based on your comment - the pointer type is in fact a pointer to a single element, but the memory it points to is still an array. Somehow under the covers the memory subsystem tracks the difference.
|
7

You should free everything in the array (if dynamically allocated) and then free the array itself.

for (int i = 0; i < size; i++) { // only free inside if dynamically allocated - not if just storing pointers
   delete array[i];
}
delete[] array; // necesarry
Improve this answer

18 Comments

You mean he should delete all the objects the pointers in the array point to, then use delete[] which deletes the objects in the array (which are pointers) and the array itself.
@DavidSchwartz, it's not accurate to say that the objects in the array are deleted, rather they are destroyed. Pointers don't have destructors, so nothing happens in that step.
@MarkRansom Pointers have destructors. C++ has them so templates work with builtin types.
The semantics of delete[] are that it deletes the objects in the array. In this case, that doesn't do much -- but the point is that it's a mistake to say that delete[] doesn't "free everything in the array". The OP's issue is that he has an array of pointers.
@DavidSchwartz: Mark is correct. Each element is destroyed (which is a no-op for raw pointers, due to the trivial destructor), then the entire block is freed at once. Each element is NOT individually freed.
|
5

The syntax for deleting an array is like this:

delete[] array;

Your for loop to delete the objects pointed to by the elements of the array is fine. The deletion of the array itself is the only problem. You need both the for loop and then the delete[] to dispose of the array itself.

for (int i = 0; i < size; i++) {
   delete array[i];
}
delete[] array;

I suspect that you have tried using the for loop, or the delete[], but not both together. And if when you do that you still have leaks or errors, then you would need to show us the code that allocates the pointers that are elements of the array.

Using std::vector<> instead of an array would mean that you could stop worrying about these nitty gritty details and move to higher level of abstraction.

Improve this answer

1 Comment

@David OK, I edited it. It's just amazing how the signal gets lots in all the noise.
2

In this case, you need both.

for (int i = 0; i < size; i++) {
   delete array[i];
}
delete[] array;

You call delete exactly once for each time you called new.

Note that although you need to call delete[] array here (because you allocated it with new[]), the delete[] operator does not call the destructors on the objects pointed to by elements of the array. This is because the delete[] operator calls destructors on objects in the array, and your array contains pointers but not objects. Pointers do not themselves have destructors.

Improve this answer

1 Comment

I think that formally, pointer types have trivial destructors, which do nothing.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.