std::list iterator usage

G

gerg

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();
 
V

Victor Bazarov

gerg said:
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
 
G

gerg

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
 
V

Victor Bazarov

gerg said:
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
 
T

Thomas J. Gritzan

gerg said:
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
 
G

gerg

What do you mean by redundant here?

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

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top