471,585 Members | 1,227 Online
Bytes | Software Development & Data Engineering Community
Post +

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 471,585 software developers and data experts.

std::list iterator usage

I'm having to deal with some legacy code which I think may be causeing
some problems. Can STL pro's please add comments about the correctness
of the following code:

class A
{
public:
/// ... assume constructors and such are defined
char* sz; // pointer to memory buffer
};

std::list<A> g_Alist;

/// code snippet in a cleanup routine
std::list<A>::iterator iter=NULL;
for(iter = g_Alist.begin();iter != g_Alist.end(); )
{
// seems to me that erase inside this loop could cause memory
corruption ??
g_Alist.erase(iter++);
// is iter messed up now? even if it is not, is it possible that
// it is redundant?
}
g_Alist.clean();

Jan 17 '06 #1
5 22684
gerg wrote:
I'm having to deal with some legacy code which I think may be causeing
some problems. Can STL pro's please add comments about the
correctness of the following code:

class A
{
public:
/// ... assume constructors and such are defined
char* sz; // pointer to memory buffer
Public?
};

std::list<A> g_Alist;

/// code snippet in a cleanup routine
std::list<A>::iterator iter=NULL;
There is no initialisation of a list iterator with NULL. Just declare
it inside the 'for' loop's control and initialise with 'begin()' as
usual (if that's what you need).
for(iter = g_Alist.begin();iter != g_Alist.end(); )
{
// seems to me that erase inside this loop could cause memory
corruption ??
g_Alist.erase(iter++);
// is iter messed up now? even if it is not, is it possible that
// it is redundant?
}
What's the reason to call 'erase' in a loop like that? Doesn't 'clear'
do that?
g_Alist.clean();


There is no 'clean'. I suppose you mean 'clear()'.

All in all, I think you're doing too much. If you need to remove all
elements of a list, where each element upon destruction cleans what
it should all by itself, in its own destructor, then all you need to
do is

g_Alist.clear();

That's it.

V
Jan 17 '06 #2
I agree with you that too much is being done here. I want to verify
that not only is this code doing too much but also corrupting memory.
Do you think it is doing that here?

thx,

gerg

Jan 17 '06 #3
gerg wrote:
I agree with you that too much is being done here. I want to verify
that not only is this code doing too much but also corrupting memory.
Do you think it is doing that here?


No. You've unrolled the loop that happens behind the covers when
you call 'clear'. Then you call 'clear' on an empty list. It does
nothing.

V
Jan 17 '06 #4
gerg schrieb:
I'm having to deal with some legacy code which I think may be causeing
some problems. Can STL pro's please add comments about the correctness
of the following code:

class A
{
public:
/// ... assume constructors and such are defined
char* sz; // pointer to memory buffer
};

std::list<A> g_Alist;

/// code snippet in a cleanup routine
std::list<A>::iterator iter=NULL;
for(iter = g_Alist.begin();iter != g_Alist.end(); )
{
// seems to me that erase inside this loop could cause memory
corruption ??
g_Alist.erase(iter++);
// is iter messed up now? even if it is not, is it possible that
// it is redundant?
iter is not "messed up", since you post-increment here. In a list,
erase() does only invalidates iterators to the removed elements.

What do you mean by redundant here?
}
g_Alist.clean();


Thomas
Jan 17 '06 #5
>> What do you mean by redundant here?

i think the loop before the clear() is not necessary.

Jan 17 '06 #6

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

8 posts views Thread by ma740988 | last post: by
15 posts views Thread by sandwich_eater | last post: by
25 posts views Thread by Markus Svilans | last post: by
4 posts views Thread by Tim Slattery | last post: by
7 posts views Thread by TBass | last post: by
12 posts views Thread by isliguezze | last post: by
17 posts views Thread by Isliguezze | last post: by
11 posts views Thread by Juha Nieminen | last post: by
reply views Thread by leo001 | last post: by
reply views Thread by Anwar ali | last post: by

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.