quote:Original post by civguy

{  int* x = new int[10];  ...  delete[] x;  x = 0;}    

What does x=0 help here? The variable x ceases to exist right after the assignment, so the assignment x=0 was meaningless.

No it doesn't. The *memory pointed to by x* is freed (the data doesn't even cease to exist! It's still there - though probably unreachable - until something overwrites it). The pointer is - apparently - left dangling.

Try:

{  int* x = new int[10];  ...  delete[] x;  delete[] x;}  

If I remember c++ worth a damn, the second delete[] will crash, because x is still pointing at the location where the int[] was, but the memory has already been freed.

Try:

{  int * x = new int[10];  ...  delete[] x;  x = NULL;  delete[] x;}  

Now it works, because deleting NULL is ok (as a special case).

What's happening to the OP, according to the theory people are proposing, is that multiple instances of the object have their own variable explode, but each has the same value in that variable - the address of a *shared* part2[] array - because copying/assignment is shallow by default. So, when the destructor is called for each object:
- When the first is destroyed, the memory of the part2[] array gets deleted. The value of 'explode' in the other objects is not touched, and thus still points at that memory.
- When the second is destroyed, the code tries to free the memory again, and crashes and burns.

You're right insofar as assigning NULL to 'explode' can't help matters. But I felt the need to be pedantic. If it were possible in the destructor to find every other instance of the object and set their 'explode' pointers to NULL, then things would work out ok.

So the question becomes how the part2[] array *should* be handled. There are a couple of possibilities:
- you might really want a separate copy of it for every instance of the object, so they can't write over each other's data. In that case, you need to implement (and make public) a copy constructor and an assignment operator (i.e. overload 'operator='), which clones the part2[] array and sets explode to point at the clone.
- you might want to share the data, either to save memory on constant data, or because collaboratively writing the data is the whole point of the objects. In that case you can:
- make part2[] a static member of the class.
- wrap part2[] in a class and reference-count it; in the X constructor/destructor you send addUser/removeUser() messages to a Part2ArrayWrapper instance, and when the count reaches 0, the Part2ArrayWrapper delete[]s the part2[]. deleting the Part2ArrayWrapper itself is an exercise for the reader.
- Use a garbage-collected language like Java and save yourself at least part of the trouble.


[edited by - Zahlman on January 11, 2004 9:08:28 PM]

quote:Original post by Zahlman
Try:

{  int* x = new int[10];  ...  delete[] x;  delete[] x;}   

You''re still seriously missing the point, even though I so carefully tried to explain it. There''s no second delete in the code I posted, nor in the OP''s code. The scope where ''x'' exists ends right after the delete. Setting x to 0 doesn''t help anything, but simply wastes resources (insignificantly little though).

Here''s a similar example:

int x = 42;x = 3;... 

That x=42 is most useful, because if the code was like this:

int x;x = 3;... 

And someone added a strange line there:

int x;cout << "Here is number 42: " << x << endl;x = 3;... 

The program''s output would be wrong! Now clearly, setting x first to 42 and then to 3 was the right solution even in the original case where there was no cout.. Right? Riiiiight.

quote:If it were possible in the destructor to find every other instance of the object and set their ''explode'' pointers to NULL, then things would work out ok.

BS. Then the other live objects would be accessing memory at location 0.

quote:Original post by civguy
(a bunch of correct stuff)

Yes, the second half of my post was much less brain-fart-y than the first. With the two delete[]s I was trying to construct an example of what happens when you destroy two of the objects - each tries to delete[] on its own, thus multiple calls.

And yeah, you can''t logically get rid of a shared resource until all its users are gone.

It''s been so long since I actually had to implement this kind of stuff

quote:Original post by DrPizza
If the pointer you have is merely a copy of the pointer then setting it to null is pointless, as the original still retains its (bad) value.

If the pointer is the only version of the pointer it''s still not a good thing. Double deleting is a mistake. You should not make your logic errors innocuous. You should make them blow up as loudly and quickly as possible. You want the double delete to fail. Setting the pointer to null prevents this; instead, it hides your double delete bug.

This is entirely undesirable. So the moral is, don''t set your pointers to null.

It is *far* more common to try to READ a deleted pointer then to doulbe-delete it. If you read a previously-deleted pointer, you will might get your values back, or some slightly different values, or who knows what. But the bug will be harder to track down then a simple "memory at 0x00000 could not be read."

So yes, it is a good thing if you actually READ your variables.