views:

310

answers:

11

I am writing a template class that takes as an input a pointer and stores it. The pointer is meant to point to an object allocated by another class, and handed to the this containing class.

Now I want to create a destructor for this container. How should I free the memory pointed to by this pointer? I have no way of knowing a priori whether it is an array or a single element.

I'm sort of new to C++, so bear with me. I've always used C, and Java is my OO language of choice, but between wanting to learn C++ and the speed requirements of my project, I've gone with C++.

Would it be a better idea to change the container from a template to a container for an abstract class that can implement its own destructor?

+4  A: 

Short answer:

If you use [] with new you want to use [] with delete.

//allocate some memory
myObject* m = new myObject[100];

//later on...destructor...
delete m; //wrong
delete[] m; //correct

That was the bare bones, the other thing you could look at is boost. Also quite difficult to answer considering you are not sure if its an array or single object. You could check this though via a flag telling your app whether to use delete or delete[].

JonH
Doesn't really answer his question. He presumably understands it's important to use the correct `delete` function, or he wouldn't be asking his question. :P
GMan
@GMan I will edit to fit needs :).
JonH
boost seems to solve everything..do you ever sometimes have that thought ? :)
JonH
No, if Boost solved *everything*, I wouldn't be going bald. But for C++ coding, it's pretty handy.
Steven Sudit
@Steven Sudit - Now that was funny!
JonH
+6  A: 

If you don't know whether it was allocated with new or new[], then it is not safe to delete it.

Your code may appear to work. For example, on one platform I work on, the difference only matters when you have an array of objects that have destructors. So, you do this:

// by luck, this works on my preferred platform
// don't do this - just an example of why your code seems to work
int *ints = new int[20];
delete ints;

but then you do this:

// crashes on my platform
std::string *strings = new std::string[10];
delete strings;
R Samuel Klatchko
It's not luck, it's that `int` has no destructor.
Steven Sudit
@Steven: It's luck. Mis-matching `new` and `delete`'s leads to undefined behavior, period. Not defined even if a type has a trivial destructor. It's entirely reasonable for me to replace `operator new`'s and `operator delete`'s as a see fit; and I can expect pointers in my `delete` function's to have been outputted by the corresponding `new`.
GMan
@Steven - it's luck because the platform *chooses* to optimize array allocations when the type has no destructor. But another platform could handle this differently and that code would then break on this other platform.
R Samuel Klatchko
I wouldn't want to make it sound like I'm suggesting we can *count on* simple types always working, so let me rephrase my answer: It's luck, but the source of your luck is that it happens to work so long as there are no destructors. Better?
Steven Sudit
@Steven: It's more than that; new[]/delete[] are allowed to use a completely different implementation than new/delete (easily done if you replace with your own). Types having trivial dtors is only half the story.
Roger Pate
@Roger Pate: You're right. The example that comes to mind is the use of fixed-sized allocation for individual instances while still going to malloc for variable-sized blocks. The point I was trying to make is that, for objects that need to be destructed, calling the non-array delete is very likely to destruct only the first instance (if it doesn't just plain crash). In this way, primitive types are less vulnerable. Having said that, this changes nothing: we should never mix array and non-array allocations/deletions. Code should work intentionally, not by luck.
Steven Sudit
A: 

A smart pointer like boost shared_pointer already has this covered, could you use it? linky

disown
+3  A: 

As a general development rule, you should stick to a design where the class which calls new should also call delete

alemjerus
Not really, this is exactly the opposite of most current smart pointer libraries: `std::auto_ptr<int> ap(new int()); boost::shared_ptr<int> sp(new int());` Both call delete for you.
Roger Pate
+5  A: 

You must document how this class expects to be used, and always allocate as expected. You can also pass a flag to the object specifying how it should destroy. Also look at boost's smart pointers, which can handle this distinction for you.

Roger Pate
This is basically correct. If you're passing a pointer in, there must be either A: only one "right" way to delete it, or B: Another "flag" variable which says which way to delete it, or C: A custom deleter, which is probably way beyond a beginner's understanding anyways. Go with "B" in your case IMO.
Kevin
@Kevin: Only basically? Did I miss anything? The option I go with isn't one you mentioned: get that pointer into an appropriate smart pointer ASAP, and let the smart pointer handle knowing how/if/when to delete. The basics of *using* auto_ptr or any of the boost smart pointers isn't very hard (understanding all the customizations and esp. implementation details is, though).
Roger Pate
What was the downvote for?
Roger Pate
As the object seems to have ownership symantics he may also want to check std::auto_ptr<>
Martin York
+1 It's really nothing but another form of matching up correct allocation and deallocation routines, which is entirely the responsibility of the user. Just as `free()` must be matched with `malloc()` (and `free()` cannot check it itself), for example `std::auto_ptr<X>` must be matched with `new`. - I wouldn't bother about the flag, though: why should the decision of deallocation routine be done at run-time, if the use of `new` vs `new[]` is going to be compile-time anyway?
UncleBens
@UncleBens: It's not about making the decision at run-time, it's about the same code/function/class/etc. being versatile. I guess it wasn't apparent, but boost::shared_ptr's custom deleters are one type of flag that I meant to include along with others.
Roger Pate
+1  A: 

You shouldn't delete it at all. If your class takes an already initialized pointer, it is not safe to delete it. It might not even point to an object on the heap; calling either delete or delete[] could be disastrous.

The allocation and deallocation of memory should happen in the same scope. Which ever code owns and initializes the instance of your class is also presumably responsible for initializing and passing in the pointer, and that is where your delete should be.

meagar
So I can't use `auto_ptr`/`unique_ptr` to pass on ownership?
GMan
@GMan There are obvious exceptions when the class you're passing a pointer into is *designed* to take ownership of it...
meagar
+1  A: 

Since pointer in C++ does not tell us how it was allocated, yes, there's no way to decide what deallocation method to use. The solution is to give the choice to the user that hopefully knows how the memory was allocated. Take a look at Boost smart ptr library, especially at shared_ptr constructor with second parameter, for a great example.

Nikolai N Fetissov
Give the choice to the user? That doesn't make much sense ?
JonH
@JonH, to the *user of the container*, i.e. the code using it, not the *user at the keyboard* :)
Nikolai N Fetissov
@Nikolai N Fetissov - ahh I got you, was confused for a second.
JonH
+2  A: 
  • Use delete if you allocated with new.
  • Use delete[] if you allocated with new[].

After these statements, if you still have a problem (maybe you want to delete an object that was created by someone else), then you are breaking the third rule:

  • Always delete what you created. Corollary, never delete what you did not create.
Vincent Robert
That third rule needs to be broken on occasion, although I agree with it in general.
Steven Sudit
The corollary from the third rule is - 'never create smart pointers' :-)
Tadeusz Kopec
Actually no. If you create an object, stores it in a smart pointer and ensure that only the smart pointer will be published to the outside world, then you took responsibility of creating and deleting the object in the same place, which is good, even if the actual deletion is not done by yourself.
Vincent Robert
I mean that each smart pointer class breaks the rule 'never delete what you did not create'. And question seems to be about something that behaves like smart pointer (takes ownership of an allocated object). And in this case Michael Burr is right - either say 'I accept this kind of pointer and no other' - standard smart pointers behave this way, or add deallocation policy to be passed with the pointer.
Tadeusz Kopec
+2  A: 

(Moving my comment into an answer, by request.)

JonH's answer is right (about using array destruction only when you used array construction), so perhaps you should offer templates: one for arrays, one not.

The other answer is to avoid arrays and instead expect a single instance that may or may not be a proper collection that cleans up after itself, such as vector<>.

edit

Stealing blatantly from Roger Pate, I'll add that you could require the use of a smart pointer, which amounts to a single-item collection.

Steven Sudit
A: 

Put simply, given only a pointer to dynamically allocated memory there is no way of determining how to de-allocate it safely. The pointer could have been allocated in any of the the following ways:

  • using new
  • using new []
  • using malloc
  • using a user defined function
  • etc.

In all cases before you can deallocate the memory you have to know how it was allocated.

anon
You'd basically decide if your class represents a smart_ptr or a smart_array and leave it up to the caller to give a suitably allocated pointer to your class.
UncleBens
@UncleBens Not sure what your comment means WRT my answer - despite its name a "smart pointer" isn't a pointer.
anon
I mean it's right that if all you have is a pointer, then you can't tell how it was allocated. I don't think it ends there. You simply say, I have a class here which is going to call `delete`/`delete[]`/`free()`/user-defined deallocation function, and it is up to the user of your class to provide a suitably allocated pointer. Basically you do something that makes sense, and the rest is not your problem.
UncleBens
@UncleBens Not to criticise your style too much, but in comments try to differentiate between the answerer (the default "you") and the questioner.
anon
Ok. In my comments "you" refers to the designer/programmer of a class in general. As I suppose in your reply where you probably don't mean: "... before you, Alex, can deallocate memory..." :)
UncleBens
@UncleBens My use of "you" was in the answer to a question written Alex. Your use of "you" was in comment to an answer written by me.
anon
So everybody else other than Alex *can* deallocate without knowing how the object was allocated? (just kidding) More often than not, one chooses how to allocate an object, knowing in advance how a particular class is going to deallocate it. One simply doesn't do: `std::auto_ptr<int> p((int*) malloc(sizeof int));`. The problem is at the end of the person doing *that*. A class that automatically figures out a proper way to deallocate (or even whether to deallocate) is indeed unthinkable.
UncleBens
+2  A: 

If you have a class that takes a pointer it's going assume ownership of, then the contract for the use of the class needs to include one of a couple things. Either:

  • the interface needs to indicate how the object the pointer is pointing to was allocated so the new owner can know how to safely deallocate the object. This option has the advantage of keeping things simple (on one level anyway), but it's not flexible - the class can't handle taking ownership of static objects as well as dynamically allocated objects.

or

  • the interface needs to include a mechanism where a deallocation policy can be specified by whatever is giving the pointer to the class. This can be as simple as providing a mechanism to pass in a functor (or even a plain old function pointer) that will be called to deallocate the object (preferably in the same function/constructor that passes in the pointer itself). This makes the class arguably more complicated to use (but having a default policy of calling delete on the pointer, for example, might make it as easy to use as option 1 for the majority of uses). Now if someone wants to give the class a pointer to a statically allocated object, they can pass in a no-op functor so nothing happens when the class wants to deallocates it, or a functor to a delete[] operation if the object was allocated by new[], etc.
Michael Burr
+1 This is the correct answer. I'm surprised it's not upvoted.
Tadeusz Kopec