By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
435,064 Members | 1,449 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 435,064 IT Pros & Developers. It's quick & easy.

Deleting Pointers from STL Vector

P: n/a
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.

In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:

1. Before the delete, toDie and m_cdPlayer1 have a value of
0x00a72498.

2. After the delete, both toDie and m_cdPlayer1 have their __vfptr
point to memory 0xFEEEFEEE (signifying a delete)

3. After the set-to-null, the value of toDie becomes 0x00000000, while
m_cdPlayer1 remains at a value of 0x00a72498 (while still pointing to
0xFEEEFEEE).

Not the behavior I want. At this point, m_cdPlayer1 is officially
deleted, yet the test "if (m_cdPlayer)" fails, since it is technically
not NULL.

Any suggestions or recommendations on how to get the intended behavior
out of this snippet?

- Hanzo

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #1
Share this Question
Share on Google+
20 Replies


P: n/a
[Cross-post to moderated group removed. I'd rather not wait for
moderation. Generally, you should not cross-post to moderated groups.]

Hanzo wrote:
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
Why bother setting this to NULL when it's about to go out of scope and
cease to exist?

Also, there would be nothing wrong with doing this instead:

delete *i;
i = g_vecGameObjects.erase(i);
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
The original variable to which toDie was pointing is gone. It doesn't
exist. You deleted it. Once you've done that, you have no business
attempting to determine its value.

In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:
You are saying you have two different, unrelated pointers pointing to
the same dynamically created object? In that case, your problem is poor
design. The solution is to fix the design.

1. Before the delete, toDie and m_cdPlayer1 have a value of
0x00a72498.

2. After the delete, both toDie and m_cdPlayer1 have their __vfptr
point to memory 0xFEEEFEEE (signifying a delete)
I don't know what a __vfptr is, but pointers are just pointer - they
don't "have" anything called __vfptr. Theoretically, they could point to
something that does, but in this case you have deleted the thing they
point to, so you may not examine it or attempt to determine its value in
any way. If you do so, your program's behavior is undefined.

3. After the set-to-null, the value of toDie becomes 0x00000000, while
m_cdPlayer1 remains at a value of 0x00a72498 (while still pointing to
0xFEEEFEEE).
Of course.

Not the behavior I want. At this point, m_cdPlayer1 is officially
deleted, yet the test "if (m_cdPlayer)" fails, since it is technically
not NULL.
Of course.

Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


Don't share an object between two pointers like that. Or, if you do,
have one pointer "own" the object - never attempt to delete it through
the other pointer. This can get complicated, though. You have to be sure
the owner doesn't delete it while it may still be accessed through any
other pointer.

In general, you cannot know whether the object pointed to by a pointer
still exists, or has been deleted, and any attempt to refer to a deleted
object causes undefined behavior.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

Jul 19 '05 #2

P: n/a
"Hanzo" <ha***@milclan.com> wrote in message
news:om********************************@4ax.com...
http://www.cuj.com/documents/s=8890/...p0310alexandr/
[...]
Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


Yes. Use a smart pointer. Then, just destroy the copies of pointers that
you don't want any more. When the last pointer to the object is destroyed,
the object will be freed. Here's a pretty popular smart pointer:

www.boost.org/libs/smart_ptr/index.htm

Dave

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.521 / Virus Database: 319 - Release Date: 9/23/2003
Jul 19 '05 #3

P: n/a
> I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL;
After those lines toDie runs out of scope; why bother setting it to
NULL?
I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.


'toDie = NULL;' only sets the toDie pointer to NULL it does not affect
the object it was pointing to. Note that that object no longer exists at
the point toDie is being set to NULL because it was deleted just before
that line.

--
Peter van Merkerk
peter.van.merkerk(at)dse.nl

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #4

P: n/a
On Thu, 02 Oct 2003 18:58:44 GMT, Kevin Goodsell
<us*********************@neverbox.com> wrote:
[Cross-post to moderated group removed. I'd rather not wait for
moderation. Generally, you should not cross-post to moderated groups.]
Sorry about that.
Why bother setting this to NULL when it's about to go out of scope and
cease to exist?
The goal is to have m_cdPlayer1 set to NULL; my only access to it at
that point is via an iterator walking the g_vecGameObjects vector,
though.
delete *i;
i = g_vecGameObjects.erase(i);


Several people have recommended this strategy now, I just want to make
sure that, ultimately, m_cdPlayer1 is the variable that is deleted/set
to NULL, so that later on in the code, when I need to perform this
check:

if (m_cdPlayer != NULL)

That I don't erroneously evaluate to true when in fact the variable
has been deleted and *should* be NULL.

- Hanzo

Jul 19 '05 #5

P: n/a
Hanzo <ha***@milclan.com> wrote in message news:<om********************************@4ax.com>. ..
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;
Here toDie is a COPY of the pointer referenced by the iterator i.
When you later set toDie to NULL (and why not just 0?) you are only
setting that copy, and of course not the original pointer it was
copied from.

All you need to do is to make toDie be a reference to the pointer of
interest, rather than a copy of it. Simply change the above
declaration to:

CDrawableObject* &toDie = *i;

and all will work as you expect.

Randy.

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}


{ Snip of unreferenced quoting. -mod/jep }

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #6

P: n/a
Hanzo <ha***@milclan.com> wrote in message news:<om********************************@4ax.com>. ..
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.

In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:

1. Before the delete, toDie and m_cdPlayer1 have a value of
0x00a72498.

2. After the delete, both toDie and m_cdPlayer1 have their __vfptr
point to memory 0xFEEEFEEE (signifying a delete)

3. After the set-to-null, the value of toDie becomes 0x00000000, while
m_cdPlayer1 remains at a value of 0x00a72498 (while still pointing to
0xFEEEFEEE).

Not the behavior I want. At this point, m_cdPlayer1 is officially
deleted, yet the test "if (m_cdPlayer)" fails, since it is technically
not NULL.

Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


Put boost::shared_ptrs in your vector (no delete needed, just erase
from the vector), and use boost::weak_ptr as m_cdPlayer1. That should
do what you want.

http://www.boost.org.

Bob

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #7

P: n/a
On 2 Oct 2003 19:44:52 -0400, rm*****@isicns.com (Randy Maddox) wrote:
Hanzo <ha***@milclan.com> wrote in message news:<om********************************@4ax.com>. ..
All you need to do is to make toDie be a reference to the pointer of
interest, rather than a copy of it. Simply change the above
declaration to:

CDrawableObject* &toDie = *i;

and all will work as you expect.


This does not work.

1. toDie is set to 0x00000000 after the delete and assignment to NULL.
m_cdPlayer1, while pointing to 0xFEEEFEEE, is still a value of
0x00a72498. This evaluates to not NULL, and fails a test to see if it
exists.

2. Some other people have suggested I do this:

if ((*i) && (*i)->CanRemove())
{
delete *i;
*i = NULL;

This, also, does not work. After the delete, both *i and m_cdPlayer1
point to 0xFEEEFEEE (deleted), and their values are both 0x00a72498.
However, after *i is assigned to NULL, its value is 0x00000000, while
m_cdPlayer1 remains 0x00a72498.

Ultimately, this boils down to one fact: I absolutely need a way to
walk a vector of base pointers, and delete/NULL them, affecting the
original variables that were pushed onto the vector in the first
place.

Some people have also suggested smart pointers and boost, and I am
stuck with STLPort. Sorry.

- Hanzo
Jul 19 '05 #8

P: n/a
> void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
Yes, because you "delete" the object both "toDie" and your vector
element points to, but modify only the (automatic) poiter "toDie". As
usual with pointers, there's a difference between the pointer and the value /
instance it points to. So it has not much to do with "std::vector<>" nor with
C++.
In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:
Do you mean, that "m_cdPlayer1" is an instance, not a pointer? If so, you
cannot achieve your goal. So "m_cdPlayer1" _must_ be a pointer.
Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


The misconception is, that "vector<CDrawableObject*>" is the wrong type for
your usage. You should store pointers to (member) pointers instead like this:
"vector< CDrawableObjects *CGame::* > v;"
Now "delete myGame.*v[42]; myGame.*v[42]=0;"
will do the trick.

In other words, you need one further indirection in addressing.

Cheers,
Philipp.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #9

P: n/a
Hanzo wrote:
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.

In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:

1. Before the delete, toDie and m_cdPlayer1 have a value of
0x00a72498.

2. After the delete, both toDie and m_cdPlayer1 have their __vfptr
point to memory 0xFEEEFEEE (signifying a delete)

3. After the set-to-null, the value of toDie becomes 0x00000000, while
m_cdPlayer1 remains at a value of 0x00a72498 (while still pointing to
0xFEEEFEEE).

Not the behavior I want. At this point, m_cdPlayer1 is officially
deleted, yet the test "if (m_cdPlayer)" fails, since it is technically
not NULL.

Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


Try working with vector<CDrawableObject **>. This has enough
indirection to give you the "desired" effect. You'll need to
write:

delete *toDie;
*toDie = NULL;
Having said that, the real problem appears to be a misunderstanding
of pointers and indirection.

The variable toDie is a local variable, the assignment 'toDie = NULL'
merely sets that variable to NULL. A different variable, of the
same type and value (m_cdPlayer) is not affected at all. However,
due to the interpretation of the type (pointer to CDrawableObject),
and the statement 'delete toDie;' the variable m_cdPlayer holds a
value that is no longer valid.

The expectation that 'toDie = NULL' will affect any other variable,
under the conditions stated above, indicates a misunderstanding of
pointer indirection and iterators.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #10

P: n/a
> I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
{over quote snipped -mod}
Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


setting toDie = NULL is useless as it get overwritten in the next
iteration and it is unkown outside the block is is declared in anyway.

setting (*i) to NULL marks at least in your vector that the element is
deleted.
If the only pointer is in your vector, you are there. But as you
describe it, that is not the case.

Realize that you deleting an object does not change any pointer to that
object.
Probably you need to rewrite some of your program.
One possible solution would be to keep all pointers to your objects in
only one vector and using iterators to this vector to access them.

hdh,
marc



[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #11

P: n/a
On 2 Oct 2003 06:18:39 -0400, Hanzo <ha***@milclan.com> wrote:
void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);
This removes the pointer from the vector. Since we are about
to delete the pointee, ownership by the vector is assumed. The
whole point of this exercise is that

delete **i;
i = g_vecGameObjects.erase(i);

may have undefined behavior because we do not know what might be
done with the invalid pointer by erase. Paranoid code.
// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
The original variable of type pointer which had the same value as
toDie has been erased from the vector and does not exist.
In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:


If that is true, there is an ownership problem and the whole program
is a total mess. The CDrawableObjects* is a pointer to an object

allowcated on the heap and owned by that pointer. If there are any
other pointers to that heap object, it is a bug in the program.

John

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #12

P: n/a
"Hanzo" <ha***@milclan.com> a écrit:
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;
possible fix...
CDrawableObject*& toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

[snip]

cheers,

- Manuel
to reply, swap the name with the domain.

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #13

P: n/a
Hi Hanzo,

if you want to manipulate the _original_ pointer m_cdPlayer, you must use a
vector of pointers to pointers:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject**>::iterator i;

for (i = g_vecGameObjects.begin(); i != g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((**i) && (**i)->CanRemove())
{
delete **i;
**i = NULL;

i = g_vecGameObjects.erase(i);
}
else
++i;
}
}
Hope this helps
Jorge

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #14

P: n/a
Hanzo wrote:
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
}

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
Right. ToDie is just a plain pointer. You initialized it with the
value of (*i), then changed its value.

Personally, I'd rewrite it so you don't reset i immediately: if ((*i) && (*i)->CanRemove())
{
// erase...
Vector<CDrawableObject*>::iterator i2 = g_vecGameObjects.erase(i);
// ...and kill
delete *i;
*i = NULL; i=i2; }


But the way to do what you want is a reference-to-pointer:

if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject*& toDie = *i; //note ampersand

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
--
Aaron Bentley
www.aaronbentley.com

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #15

P: n/a
On 2 Oct 2003 19:44:52 -0400, rm*****@isicns.com (Randy Maddox) wrote:
Hanzo <ha***@milclan.com> wrote in message news:<om********************************@4ax.com>. ..
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i; Here toDie is a COPY of the pointer referenced by the iterator i.
When you later set toDie to NULL (and why not just 0?) you are only
setting that copy, and of course not the original pointer it was
copied from. All you need to do is to make toDie be a reference to the pointer of
interest, rather than a copy of it. Simply change the above
declaration to: CDrawableObject* &toDie = *i;

and all will work as you expect.


I doubt it.
// erase...
i = g_vecGameObjects.erase(i);
Make dangling reference.
// ...and kill
delete toDie;


Kaboom!

John

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #16

P: n/a
Hanzo <ha***@milclan.com> wrote in message news:<6f********************************@4ax.com>. ..
On 2 Oct 2003 19:44:52 -0400, rm*****@isicns.com (Randy Maddox) wrote:
Hanzo <ha***@milclan.com> wrote in message news:<om********************************@4ax.com>. ..
All you need to do is to make toDie be a reference to the pointer of
interest, rather than a copy of it. Simply change the above
declaration to:

CDrawableObject* &toDie = *i;

and all will work as you expect.


This does not work.

1. toDie is set to 0x00000000 after the delete and assignment to NULL.
m_cdPlayer1, while pointing to 0xFEEEFEEE, is still a value of
0x00a72498. This evaluates to not NULL, and fails a test to see if it
exists.

2. Some other people have suggested I do this:

if ((*i) && (*i)->CanRemove())
{
delete *i;
*i = NULL;

This, also, does not work. After the delete, both *i and m_cdPlayer1
point to 0xFEEEFEEE (deleted), and their values are both 0x00a72498.
However, after *i is assigned to NULL, its value is 0x00000000, while
m_cdPlayer1 remains 0x00a72498.

Ultimately, this boils down to one fact: I absolutely need a way to
walk a vector of base pointers, and delete/NULL them, affecting the
original variables that were pushed onto the vector in the first
place.

Some people have also suggested smart pointers and boost, and I am
stuck with STLPort. Sorry.

- Hanzo


Sorry. Let's try again. :-)

Given your original code sample, several people have suggested
variations of the same solution to the apparent problem that you were
setting the value of a local variable copy of the pointer of interest.
I am quite surprised that you now say that none of these solutions
appear to work, since it looks like any of them should be correct.

Perhaps you can provide a bit more detail on how you are determining
that the solutions offered are not working? Perhaps there is some
other detail of your code not yet mentioned that affects what is
happening. In any case, at least one problem has been found, although
it appears there may also be some other issue involved as well.

Randy.
Jul 19 '05 #17

P: n/a
Hanzo <ha***@milclan.com> wrote in message news:<6f********************************@4ax.com>. ..
On 2 Oct 2003 19:44:52 -0400, rm*****@isicns.com (Randy Maddox) wrote:
Hanzo <ha***@milclan.com> wrote in message news:<om********************************@4ax.com>. ..
All you need to do is to make toDie be a reference to the pointer of
interest, rather than a copy of it. Simply change the above
declaration to:

CDrawableObject* &toDie = *i;

and all will work as you expect.


This does not work.

1. toDie is set to 0x00000000 after the delete and assignment to NULL.
m_cdPlayer1, while pointing to 0xFEEEFEEE, is still a value of
0x00a72498. This evaluates to not NULL, and fails a test to see if it
exists.


This is fundamentally impossible using dumb pointers. An object does not
know what pointers point to it. m_cdPlayer1 is unknown to *m_cdPlayer1.
No matter what you do to *m_cdPlayer1, including deletion via toDie
will alter cdPlayer1.

The correct solution is to use smart pointers. Smart pointers can be
reset on deletion, especially when they are not mixed with dumb pointers.
In particular, when toDie is an (indirect) copy of m_cdPlayer1 the
NULLing is trivial.

If you can't use some kind of smart pointers, you can't solve
your problem. Dumb pointers are too dumb.

Regards, Michiel Salters
Jul 19 '05 #18

P: n/a
Hanzo <ha***@milclan.com> wrote in message
news:<om********************************@4ax.com>. ..
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this: void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i != g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i; // erase...
i = g_vecGameObjects.erase(i); // ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
} Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
Normal, no. You told the compiler to set toDie to NULL. You didn't tell
it to do anything else. In this case, setting toDie to NULL is for all
intents and purposes a no-op, and a any halfway good optimizing compiler
will eliminate the line completely.

I'm not sure what you mean by "the original variable". The variable
toDie contains a copy of the element which was in the vector. This
element has been erased. It doesn't exist any more, so it can't be set
to NULL.

There may, of course, be other variables designating the element. The
compiler has no way of knowing this, and it is your responsibility to
manage them. There are several alternatives, according to your needs:

- If just setting them to null is sufficient, there are different
types of smart pointers. If you use boost::shared_ptr in the array,
any boost::weak_ptr pointing to the object will appear to be NULL;
in many cases, this is a sufficient solution. If for some reason,
using boost::shared_ptr is not an alternative (although frankly, I
cannot think of one), I have a ManagedPtr at my site which will
automatically become null when the pointed to object is destructed;
it is considerably less flexible than the Boost solution, however,
since it places significant constraints on the pointed to object.

- If you are storing the pointers in other collections, say because
another object might want to refer to several GameObjects, then you
also need some way of removing the pointers from the collection
entirely, to avoid memory leaks. The simplest way to do this is
just to use weak_ptr, as above, and scan the collection periodically
(say each time you modify it) to suppress any weak_ptr whose objects
no longer exist. If the collections are really large, however, this
may have unacceptable runtime overhead. In such cases, you need
some means of informing the collection; this implies some sort of
notification mechanism, in which the collection is a listener. My
ManagedPtr uses a notification to inform the pointers to be set to
null, and could easily be adopted to handle additional
notifications. While this is definitely overkill when the first
solution works, if you do need notification, the added complexity
that ManagedPtr introduces will be necessary anyway.

- More generally, it may be that the object holding a pointer to the
deleted object needs to do more than just delete the relationship.
In this case, you need to use the Observer pattern. The destructor
of the object informs all classes which have expressed an interest
in it that it is going to die, and they do whatever they have to do.
In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens: 1. Before the delete, toDie and m_cdPlayer1 have a value of
0x00a72498. 2. After the delete, both toDie and m_cdPlayer1 have their __vfptr
point to memory 0xFEEEFEEE (signifying a delete)
I think you are speaking about something in the pointed to object
itself. If both pointers point to the same address, it is normal that
reading through either pointer will result in the same thing.
3. After the set-to-null, the value of toDie becomes 0x00000000, while
m_cdPlayer1 remains at a value of 0x00a72498 (while still pointing to
0xFEEEFEEE).
What else should (or even could) happen. The compiler has no way of
knowing which objects might or might not have pointers to the same place
as toDie. Changing toDie changes toDie, and nothing else in the
program.
Not the behavior I want. At this point, m_cdPlayer1 is officially
deleted, yet the test "if (m_cdPlayer)" fails, since it is technically
not NULL. Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


See above. The easiest solution to begin with is to use
boost::shared_ptr in the vector, and boost::weak_ptr every where else.
But sooner or later, you'll run into a case where you need full
notification; every professional I know of has some sort of change
notification mechanism in his toolbox.

--
James Kanze GABI Software mailto:ka***@gabi-soft.fr
Conseils en informatique orientée objet/ http://www.gabi-soft.fr
Beratung in objektorientierter Datenverarbeitung
11 rue de Rambouillet, 78460 Chevreuse, France, +33 (0)1 30 23 45 16

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #19

P: n/a
WW
Hanzo wrote:
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this: [SNIP] Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.

In other words, if one of the CDrawableObjects* was say, a member var
of CGame called m_cdPlayer1, when the delete/set-to-NULL happens:

1. Before the delete, toDie and m_cdPlayer1 have a value of
0x00a72498.

2. After the delete, both toDie and m_cdPlayer1 have their __vfptr
point to memory 0xFEEEFEEE (signifying a delete)

3. After the set-to-null, the value of toDie becomes 0x00000000, while
m_cdPlayer1 remains at a value of 0x00a72498 (while still pointing to
0xFEEEFEEE).

Not the behavior I want. At this point, m_cdPlayer1 is officially
deleted, yet the test "if (m_cdPlayer)" fails, since it is technically
not NULL.

Any suggestions or recommendations on how to get the intended behavior
out of this snippet?


I see no m_Player in your code snippet. So I have no idea. However I have
few information for you:

1.) std::vector stores a copy of whatever thing it stores. The pointer
inside the vector has absolutely no connection to the m_cdPlayer1 member
variable it has been copied from.

2.) toDie is a copy of what was inside the vector. It has no connection to
the pointer pointed by the iterator. It makes absolutely no sense to set it
to NULL, since in the next line (at the }) it ceases to exist.

If you want to be able to change the original pointer, you will need to use
a pointer to pointer and store that in the vector.

--
WW aka Attila

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 19 '05 #20

P: n/a
Hanzo wrote:
I'm iterating over a vector of base class pointers and deleting those
which meet a certain criteria...i'm using pretty text-book code for
the particular delete/erasure (it's straight out of Myers' Effective
STL), and reads like this:

void CGame::RemoveDeadObjects()
{
// cleanup crew!!
vector<CDrawableObject*>::iterator i;

for (i = g_vecGameObjects.begin(); i !=
g_vecGameObjects.end();)
{
// if object is not null and can be removed...
if ((*i) && (*i)->CanRemove())
{
// grab hold before erasure
CDrawableObject* toDie = *i;

// erase...
i = g_vecGameObjects.erase(i);

// ...and kill
delete toDie;
toDie = NULL;
}
else
++i;
}
} simplier:

for (i = g_vecGameObjects.begin(); i != g_vecGameObjects.end(); ++i)
{
if ((*i) && (*i)->CanRemove())
{
delete (*g_vecGameObjects.erase(i));
}
}

to eliminate any doubts about behavior of your code just add something like
.....
~CDrawableObject(void) { std::cout << "~CDrawableObject(); " << std::endl; }
.....

to CDrawableObject class declaration, and observe output when destructor
called.

Here's the problem: on the two lines that say delete toDie; toDie =
NULL; I am noticing that the original variable to which toDie is
pointing is not being set to NULL, just "toDie" itself.
..... out of this snippet?

what you inserting into vector in first place is a copy of pointer
(variable m_cdPlayer) so when assign NULL (you should be using "0"
instead, because NULL is not defined anywhere in C++ headers) you assign
it to copy of pointer stored in a vector not to m_cdPlayer member
variable itself.
Maybe instead you should use map<>, eliminate m_cdPlayer member variable
completely and address your objects like:

.....
iter = mElemMap["CD Player"];
.....

you can iterate map<> same way as you iterate vector, only difference is
in second case, that you have only _one_ single location where you keep
pointers to Drawable objects;
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]
Jul 22 '05 #21

This discussion thread is closed

Replies have been disabled for this discussion.