std::list iterator usage

Discussion in 'C++' started by gerg, Jan 17, 2006.

  1. gerg

    gerg Guest

    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();
    gerg, Jan 17, 2006
    #1
    1. Advertising

  2. 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
    Victor Bazarov, Jan 17, 2006
    #2
    1. Advertising

  3. gerg

    gerg Guest

    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
    gerg, Jan 17, 2006
    #3
  4. 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
    Victor Bazarov, Jan 17, 2006
    #4
  5. 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
    Thomas J. Gritzan, Jan 17, 2006
    #5
  6. gerg

    gerg Guest

    >> What do you mean by redundant here?

    i think the loop before the clear() is not necessary.
    gerg, Jan 17, 2006
    #6
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. greg
    Replies:
    6
    Views:
    444
    Dietmar Kuehl
    Jul 17, 2003
  2. Vivi Orunitia
    Replies:
    11
    Views:
    4,448
    Martijn Lievaart
    Feb 4, 2004
  3. Replies:
    6
    Views:
    628
    Jim Langston
    Oct 30, 2005
  4. David Bilsby
    Replies:
    5
    Views:
    2,025
    David Bilsby
    Oct 9, 2007
  5. Juha Nieminen
    Replies:
    22
    Views:
    989
    Kai-Uwe Bux
    Oct 12, 2007
Loading...

Share This Page