confusing delete problem

[Joerg Toellner]

Hi Group,

i stumbled over a delete problem which confuses me. I'm not very experienced
with C++ programming and so i am not able to understand what's going on
here.

I have the following situation:

-------------SNIP---------------------
// File: map.cpp

class tile;

class map
{
map();
~map();
// ... some other unrelated stuff here
tile *m_MyTiles[144];
}

void map::map()
{
int x;
for(x = 0; x < 144; x++)
m_MyTiles[x] = new tile;
}

void map::~map()
{
for(x = 0; x < 144; x++)
delete m_MyTiles[x];
}
-------------SNIP---------------------

When i construct a new instance of class map and later delete it again my
program crash at the delete command in the destructor. It seems that the (in
the constructor of class map) created 144 instances of class Tile will be
deleted somehow automagically. When i comment out the code in the destructor
of class map, all works fine. But i assume that then i will leave some
memory leaks behind, right?

Can s.o. please direct me in the right direction what i'm doing wrong? Must
i delete my with new created instances of the tiles in the destructor of map
(where the pointers are member variables)? Or who will do this for me behind
the scene? Or when this will leave memory leaks, how do i delete my
tiles-objects correctly without a program crash?

Sorry for the dumb question, but i can't figure it out myself.

TIA very much
Joerg Toellner

 

[Clark Cox]

Hi Group,

i stumbled over a delete problem which confuses me. I'm not very experienced
with C++ programming and so i am not able to understand what's going on
here.

I have the following situation:

-------------SNIP---------------------
// File: map.cpp

class tile;

class map
{
map();
~map();
// ... some other unrelated stuff here
tile *m_MyTiles[144];
}

void map::map()
{
int x;
for(x = 0; x < 144; x++)
m_MyTiles[x] = new tile;
}

void map::~map()
{
for(x = 0; x < 144; x++)
delete m_MyTiles[x];
}
-------------SNIP---------------------

The code as you have posted it is fine (assuming that tile is defined
somewhere before map::map(), and you actually define a variable 'x' in
map::~map(). Your problem must be elsewhere. Maybe some other code is
overwriting the m_MyTiles array, or maybe there is a problem in tile's
destructor, but I can't really tell from what you've posted

 

[John Harrison]

Hi Group,

i stumbled over a delete problem which confuses me. I'm not very experienced
with C++ programming and so i am not able to understand what's going on
here.

I have the following situation:

-------------SNIP---------------------
// File: map.cpp

class tile;

class map
{
map();
~map();
// ... some other unrelated stuff here
tile *m_MyTiles[144];
}

void map::map()
{
int x;
for(x = 0; x < 144; x++)
m_MyTiles[x] = new tile;
}

void map::~map()
{
for(x = 0; x < 144; x++)
delete m_MyTiles[x];
}
-------------SNIP---------------------

When i construct a new instance of class map and later delete it again my
program crash at the delete command in the destructor. It seems that the (in
the constructor of class map) created 144 instances of class Tile will be
deleted somehow automagically.

That's not true, you new them so you delete them.

When i comment out the code in the destructor
of class map, all works fine. But i assume that then i will leave some
memory leaks behind, right?

Right.

Can s.o. please direct me in the right direction what i'm doing wrong? Must
i delete my with new created instances of the tiles in the destructor of map
(where the pointers are member variables)? Or who will do this for me behind
the scene? Or when this will leave memory leaks, how do i delete my
tiles-objects correctly without a program crash?

What you've forgotten to do is write a copy constructor and assignment
operator for your class.

If you have two copies of the same object, then at present you will have two
copies of the same pointers. When the first object gets destructed
everything is OK, but when the second gets destructed you are deleteing
pointers that have been deleted already, so your program crashes.

Sorry for the dumb question, but i can't figure it out myself.

This is *the* classic newbie C++ gotcha. You need a good C++ book, any C++
book that doesn't explain copy constructors is not worth reading.

TIA very much
Joerg Toellner

john

 

[Joerg Toellner]

Hi John and Clark,

thx for your replies.

I haven't my code right by hand now. So i only wrote the situation out of
the head. Of course x is defined in the real code destructor.

I'm glad that my suggestion that i have to delete it, is right.

To John:
Can't understand your comments about copy constructor. There will be
definitely ONE instance/object of type class map. map is a map (or say a
playfield) for a game that will hold the parts of the playfield (the single
tiles of the map). So there is only one map consisting of maaaaany tiles.

Do i need the copy constructor in this case too? And why do i need it when i
never want to copy the map-object? Even if it is a good fashion or
recommended by default.

Of course i'm thankful for your hint. I will remember this for the objects
where i need more than one instance.

I'll check my code further to find who is deleting my tiles before i get
hand on it. Your comments are important for me that it is worth searching
for the bug further and not hunting C++ ghosts which every experienced
C++-Guru already knows.

Thx. again to you both and to John for maybe another little comment about
the copy constructor.

CU
Joerg Toellner

PS:
I already own Stroustroup C++ Language and Effective C++ and More effective
C++ as books. But you will understand, nobody can remember all the details
and s.t. it is like chasing an elephant with an microscope. You overseas the
obvious.

Hi Group,

i stumbled over a delete problem which confuses me. I'm not very experienced
with C++ programming and so i am not able to understand what's going on
here.

I have the following situation:

-------------SNIP---------------------
// File: map.cpp

class tile;

class map
{
map();
~map();
// ... some other unrelated stuff here
tile *m_MyTiles[144];
}

void map::map()
{
int x;
for(x = 0; x < 144; x++)
m_MyTiles[x] = new tile;
}

void map::~map()
{
for(x = 0; x < 144; x++)
delete m_MyTiles[x];
}
-------------SNIP---------------------

When i construct a new instance of class map and later delete it again my
program crash at the delete command in the destructor. It seems that the (in
the constructor of class map) created 144 instances of class Tile will be
deleted somehow automagically.

That's not true, you new them so you delete them.

When i comment out the code in the destructor
of class map, all works fine. But i assume that then i will leave some
memory leaks behind, right?

Right.

Can s.o. please direct me in the right direction what i'm doing wrong? Must
i delete my with new created instances of the tiles in the destructor of map
(where the pointers are member variables)? Or who will do this for me behind
the scene? Or when this will leave memory leaks, how do i delete my
tiles-objects correctly without a program crash?

What you've forgotten to do is write a copy constructor and assignment
operator for your class.

If you have two copies of the same object, then at present you will have two
copies of the same pointers. When the first object gets destructed
everything is OK, but when the second gets destructed you are deleteing
pointers that have been deleted already, so your program crashes.

Sorry for the dumb question, but i can't figure it out myself.

This is *the* classic newbie C++ gotcha. You need a good C++ book, any C++
book that doesn't explain copy constructors is not worth reading.

TIA very much
Joerg Toellner

john

 

[John Harrison]

Hi John and Clark,

thx for your replies.

I haven't my code right by hand now. So i only wrote the situation out of
the head. Of course x is defined in the real code destructor.

I'm glad that my suggestion that i have to delete it, is right.

To John:
Can't understand your comments about copy constructor. There will be
definitely ONE instance/object of type class map. map is a map (or say a
playfield) for a game that will hold the parts of the playfield (the single
tiles of the map). So there is only one map consisting of maaaaany tiles.

Do i need the copy constructor in this case too? And why do i need it when i
never want to copy the map-object? Even if it is a good fashion or
recommended by default.

If you really don't think a map object will ever be copied then do this

class map
{
public:
...
private:
map(const map&);
map& operator=(const map&);
};

I.e. declare the copy constructor and assignment operator private, this will
stop the compiler copying them when you don't expect it to.

You problem does sound exactly like a problem caused by a lack of copy
constructor, are you sure the map is never copied, for instance to you use
the map as a parameter to a function, or a return value from a function?
Either of those could cause the map to be copied.

john

 

[Chris Theis]

Hi Group,

i stumbled over a delete problem which confuses me. I'm not very experienced
with C++ programming and so i am not able to understand what's going on
here.

I have the following situation:

-------------SNIP---------------------
// File: map.cpp

class tile;

class map
{
map();
~map();
// ... some other unrelated stuff here
tile *m_MyTiles[144];
}

void map::map()
{
int x;
for(x = 0; x < 144; x++)
m_MyTiles[x] = new tile;
}

void map::~map()
{
for(x = 0; x < 144; x++)
delete m_MyTiles[x];
}
-------------SNIP---------------------

The code as you have posted it is fine (assuming that tile is defined
somewhere before map::map(), and you actually define a variable 'x' in
map::~map(). Your problem must be elsewhere. Maybe some other code is
overwriting the m_MyTiles array, or maybe there is a problem in tile's
destructor, but I can't really tell from what you've posted

It's rather what the OP did not write than what he's written. The trouble is
that the delete statement might be called twice on the pointers which will
result in undefined behavior. This might be caused for example by the
omittance of the copy constructor. Hence, the compiler synthesizes a binary
copy and two map objects will contain pointers to the same elements.
Consequently, the elements will be deleted as soon as one of those two
objects gets destroyed. As soon as the other one attempts to say good bye
the program will end up in nirvana as the delete statement is called again
for the already deleted elements.

As a guideline one should remember always to take care of the copy ctor (and
assignment op) if one needs to implement a dtor.

Cheers
Chris

 

[Karl Heinz Buchegger]

Hi John and Clark,

thx for your replies.

I haven't my code right by hand now. So i only wrote the situation out of
the head. Of course x is defined in the real code destructor.

I'm glad that my suggestion that i have to delete it, is right.

To John:
Can't understand your comments about copy constructor. There will be
definitely ONE instance/object of type class map. map is a map (or say a
playfield) for a game that will hold the parts of the playfield (the single
tiles of the map). So there is only one map consisting of maaaaany tiles.

Do i need the copy constructor in this case too? And why do i need it when i
never want to copy the map-object?

You may create a copy by accident and never notice.

eg.

int main()
{
map tiles;

...

foo( tiles );
}

and now your function foo is declared like this

void foo( map AllTiles )
{
...
}

You didn't want this, you wanted to pass per reference, but as you are
human you made an error and forgot the &.
So what happens now? When function foo is called, a copy of tiles is made.
Both, the original tiles and the copy contain identical pointers. When
the function returns the copy is detroyed, meaning it deletes all the memory
where the pointer points to. But wait: tiles also has this pointer to the very
same memory. Boom.

Of course i'm thankful for your hint. I will remember this for the objects
where i need more than one instance.

The only thing you should remember is the simple rule:
Don't allocate dynamically if you don't have to.

class map
{
map();
~map();

...

tile m_MyTiles[144];
}

Bingo. No allocation in the constructor, no deallocationn in the destrustor
and creating copies automatically does the right thing: it copies the tiles
and not only the pointer to the tiles.

 

[Thomas Matthews]

Hi John and Clark,

thx for your replies.

I haven't my code right by hand now. So i only wrote the situation out of
the head. Of course x is defined in the real code destructor.

I'm glad that my suggestion that i have to delete it, is right.

To John:
Can't understand your comments about copy constructor. There will be
definitely ONE instance/object of type class map. map is a map (or say a
playfield) for a game that will hold the parts of the playfield (the single
tiles of the map). So there is only one map consisting of maaaaany tiles.

Do i need the copy constructor in this case too? And why do i need it when i
never want to copy the map-object? Even if it is a good fashion or
recommended by default.

Of course i'm thankful for your hint. I will remember this for the objects
where i need more than one instance.

I'll check my code further to find who is deleting my tiles before i get
hand on it. Your comments are important for me that it is worth searching
for the bug further and not hunting C++ ghosts which every experienced
C++-Guru already knows.

Thx. again to you both and to John for maybe another little comment about
the copy constructor.

CU
Joerg Toellner

PS:
I already own Stroustroup C++ Language and Effective C++ and More effective
C++ as books. But you will understand, nobody can remember all the details
and s.t. it is like chasing an elephant with an microscope. You overseas the
obvious.

Hi Group,

i stumbled over a delete problem which confuses me. I'm not very
experienced

with C++ programming and so i am not able to understand what's going on
here.

I have the following situation:

-------------SNIP---------------------
// File: map.cpp

class tile;

class map
{
map();
~map();
// ... some other unrelated stuff here
tile *m_MyTiles[144];
}

void map::map()
{
int x;
for(x = 0; x < 144; x++)
m_MyTiles[x] = new tile;
}

void map::~map()
{
for(x = 0; x < 144; x++)
delete m_MyTiles[x];
}
-------------SNIP---------------------

When i construct a new instance of class map and later delete it again

my

program crash at the delete command in the destructor. It seems that the
(in

the constructor of class map) created 144 instances of class Tile will

be

deleted somehow automagically.

That's not true, you new them so you delete them.

When i comment out the code in the destructor
of class map, all works fine. But i assume that then i will leave some
memory leaks behind, right?

Right.


Can s.o. please direct me in the right direction what i'm doing wrong?
Must

i delete my with new created instances of the tiles in the destructor of
map

(where the pointers are member variables)? Or who will do this for me
behind

the scene? Or when this will leave memory leaks, how do i delete my
tiles-objects correctly without a program crash?

What you've forgotten to do is write a copy constructor and assignment
operator for your class.

If you have two copies of the same object, then at present you will have
two

copies of the same pointers. When the first object gets destructed
everything is OK, but when the second gets destructed you are deleteing
pointers that have been deleted already, so your program crashes.

Sorry for the dumb question, but i can't figure it out myself.

This is *the* classic newbie C++ gotcha. You need a good C++ book, any C++
book that doesn't explain copy constructors is not worth reading.

TIA very much
Joerg Toellner

john

If you only want one instance of the board / map,
I suggest you search the web and newsgroups for
the "Singleton Design Pattern". The Singleton pattern
is when you only want one instance of an object.

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.learn.c-c++ faq:
http://www.raos.demon.uk/acllc-c++/faq.html
Other sites:
http://www.josuttis.com -- C++ STL Library book

 

[zoopy]

void map::map()
This should be:
map::map()

void map::~map()

Same here:
map::~map()


Regards,
planetzoom

 

[Joerg Toellner]

Dear John and Chris,

OOOps! Of course i use the map object (better to say a pointer to the
object) as function parameter and i store the pointer to the map-object in
my application-object which owns a map* GetMap() method to be able to get
access to the map from within other classes.

Ok i saw it is neccesary to go for a little reading *WhereAreMyBooks?* about
copy constructors.

Thx. again for your patience and kindness. I really appreciate to learn from
you and i hope i will be welcome later if there are other problems coming
up.

For now....happy coding *TurnThePageInMyBook-where to hell is the chapter
about operators?* )))))

CU
Joerg Toellner

 

[John Harrison]

Dear John and Chris,

OOOps! Of course i use the map object (better to say a pointer to the
object) as function parameter and i store the pointer to the map-object in
my application-object which owns a map* GetMap() method to be able to get
access to the map from within other classes.

Ok i saw it is neccesary to go for a little reading *WhereAreMyBooks?* about
copy constructors.

Well if all you are passing is a pointer to the map, then that is not a
problem (at least as far as copy constructors go).

There is a simple way to tell however. Do what I said and declare the copy
constructor and assignment operator private (you don't have to define them,
just declare them). If you do that and then you get compile or link errors
that is because somewhere you are copying the map object, if not, you
aren't.

john