Crash while erasing a member in a vector

Discussion in 'C++' started by eeykay@gmail.com, Dec 21, 2005.

  1. Guest

    Hello,

    I am facing a starnge problem while erasing the last member in a
    vector. I am using VC++ .NET 2002 complier. I have vector of
    CComPtr<..> (irrelevant here), and then I iterate over the vector. If
    it is the iterator, then I remove the element from the vector using
    vecObjects.erase(it). It works fine till the last element. While
    removing the last element it throws exception and fails. But the same
    vecObject.clear() works with out any problem. Can somebody there,
    please help me to indentify this problem and solution to this

    Thanks in advance.
    EK
     
    , Dec 21, 2005
    #1
    1. Advertising

  2. E. Mark Ping Guest

    In article <>,
    <> wrote:
    >I am facing a starnge problem while erasing the last member in a
    >vector. I am using VC++ .NET 2002 complier. I have vector of
    >CComPtr<..> (irrelevant here), and then I iterate over the vector. If
    >it is the iterator, then I remove the element from the vector using
    >vecObjects.erase(it). It works fine till the last element. While
    >removing the last element it throws exception and fails. But the same
    >vecObject.clear() works with out any problem. Can somebody there,
    >please help me to indentify this problem and solution to this


    You are most likely going off the end of the vector and erasing an
    invalid iterator. Please post a code snippet that can compile and
    you'll get much better responses.
    --
    Mark Ping
     
    E. Mark Ping, Dec 21, 2005
    #2
    1. Advertising

  3. Jordan Guest

    Yes, the 'last' position in a vector is one beyond the last element.

    If you are progressing through your vectory array, check if the address
    of the iterator has reached the address of the last position. See the
    code example below.

    vector<int>::iterator iter = 0;
    for( iter = vecList.begin(); iter != vecList.end(); iter++ )
    {
    delete *iter;
    }
     
    Jordan, Dec 21, 2005
    #3
  4. Howard Guest

    "Jordan" <> wrote in message
    news:...
    > Yes, the 'last' position in a vector is one beyond the last element.
    >
    > If you are progressing through your vectory array, check if the address
    > of the iterator has reached the address of the last position. See the
    > code example below.
    >
    > vector<int>::iterator iter = 0;
    > for( iter = vecList.begin(); iter != vecList.end(); iter++ )
    > {
    > delete *iter;
    > }
    >


    That's how to delete dynamically allocated objects from a vector, but the
    iterator you've declared refers to a vector that does not have pointers, it
    has just int's. You don't want to delete an int, or anything that was not
    created via new. (I know, it's just an example, but it's a bad example.
    :))

    The OP asked about using erase() to remove items from a vector. That's
    different from deleting the dynamically allocated data pointed to by
    pointers stored in the vector. And without seeing his code, we can't say
    _for_sure_ what the problem is. My "guess" is that clear() will work fine,
    but it's just a guess at this point. Only if he also wants to delete
    dynamically allocated objects will he first want to have a loop like the one
    you've shown.

    -Howard
     
    Howard, Dec 21, 2005
    #4
  5. BobR Guest

    wrote in message
    <>...
    >Hello,
    >
    >I am facing a starnge problem while erasing the last member in a
    >vector. I am using VC++ .NET 2002 complier. I have vector of
    >CComPtr<..> (irrelevant here), and then I iterate over the vector. If
    >it is the iterator, then I remove the element from the vector using
    >vecObjects.erase(it). It works fine till the last element. While
    >removing the last element it throws exception and fails. But the same
    >vecObject.clear() works with out any problem. Can somebody there,
    >please help me to indentify this problem and solution to this
    >Thanks in advance.
    >EK


    You forgot to show the code! Are you trying to do this in a loop?

    If you do:
    vecObjects.erase(it);
    ....then iterator 'it' may no longer be valid [1]. Reset it.

    >... It works fine till the last element.

    Use:
    vecObjects.pop_back(); // Removes the last element.

    [1] " A vector's iterators are invalidated when its memory is reallocated.
    Additionally, inserting or deleting an element in the middle of a vector
    invalidates all iterators that point to elements following the insertion or
    deletion point. ..."

    --
    Bob R
    POVrookie
     
    BobR, Dec 21, 2005
    #5
  6. Heinz Ozwirk Guest

    <> schrieb im Newsbeitrag
    news:...
    > Hello,
    >
    > I am facing a starnge problem while erasing the last member in a
    > vector. I am using VC++ .NET 2002 complier. I have vector of
    > CComPtr<..> (irrelevant here),


    Perhaps it is irrelevant for your problem, but you should not put instances
    of CComPtr into C++ containers. Some of these containers (or some
    implementations of them) do not work well with objects overloading
    operator&.

    > and then I iterate over the vector. If
    > it is the iterator, then I remove the element from the vector using
    > vecObjects.erase(it).


    When you iterate through a container and try to erase its last element, make
    sure not to increment the iterator after erasing. Actually you should never
    increment an iterator after erasing the element it refers to. For all
    containers, all iterators refering to the erased element become invalid. So
    don't do something like this

    for (SomeContainer::iterator it = myContainer.begin(); it !=
    myContainer.end(); ++it)
    {
    if (RemoveThisElement(*it)) myContainer.erase(it);
    }

    If SomeContainer is an std::vector, this seems to work until you try to
    erase the last element, but actually it will not examine those elements
    imediately following an element, that will be removed. Now if you erase the
    last element, end() will change (in some implementations end() will become
    equal to the iterator for the erased elemet) and due to the final increment
    the test for equality to end() will fail and the loop will continue until it
    reaches the end of allocated memory. (Or it will behave entirely different,
    after all that's the problem with undefined behaviour.)

    HTH
    Heinz
     
    Heinz Ozwirk, Dec 21, 2005
    #6
  7. E. Mark Ping Guest

    In article <43a9be0f$0$9637$-online.net>,
    Heinz Ozwirk <> wrote:
    > for (SomeContainer::iterator it = myContainer.begin(); it !=
    >myContainer.end(); ++it)
    > {
    > if (RemoveThisElement(*it)) myContainer.erase(it);
    > }
    >
    >If SomeContainer is an std::vector, this seems to work until you try to
    >erase the last element, but actually it will not examine those elements
    >imediately following an element, that will be removed.


    This is actually worse, because as you remove the elements you
    invalidate your iterator, so your "++it" is a very bad thing.

    Better is:

    vector<int> container;
    vector<int>::iterator iter;
    for (iter=container.begin(); iter != container.end(); )
    {
    if (RemoveThisElement(*iter)) {
    iter = container.erase(iter);
    } else {
    ++iter
    }
    }

    Note the assignment of "iter" from erase, and the lack of ++iter in
    the for loop. Either the iter is manually incremented or the erase
    operation leaves it pointing to the next position.
    --
    Mark Ping
     
    E. Mark Ping, Dec 21, 2005
    #7
  8. Guest

    Hello All,

    Thanks for the huge response, I have got. I am yet to try the
    suggestions. In the mean time I am adding the code snippet here for
    better clarity of the question I have asked.

    Though I cannot reproduce the code, it looks similar to this.

    I have a vector vecObjects with CComPtr<Interfaces I..>. Now I have to
    check the contents for this vector for some conditions. Hence I iterate
    the container as follows.

    std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
    std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()

    for(;it != itEnd; ++it); // I tried post increment also
    {
    // Check the iterator for some conditions
    if (! IsValid(*it))
    {
    vecObjects.erase(it); // This causes the crash with the last
    element
    }
    ++it;
    }

    // At the same time the following code works

    for(;it != itEnd; ++it); // I tried post increment also
    {
    // Check the iterator for some conditions
    if (! IsValid(*it))
    {
    if(vecObjects.size() == 1)
    vecObjects.clear();
    else
    vecObjects.erase(it);
    }
    ++it;
    }

    Thank you for all the help.
    EK
     
    , Dec 22, 2005
    #8
  9. E. Mark Ping Guest

    In article <>,
    <> wrote:
    >std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
    >std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()
    >
    >for(;it != itEnd; ++it); // I tried post increment also
    >{
    > // Check the iterator for some conditions
    > if (! IsValid(*it))
    > {
    > vecObjects.erase(it); // This causes the crash with the last
    >element
    > }
    > ++it;
    >}


    Yeah, this is broken.

    >// At the same time the following code works

    No it doesn't.

    >for(;it != itEnd; ++it); // I tried post increment also
    >{
    > // Check the iterator for some conditions
    > if (! IsValid(*it))
    > {
    > if(vecObjects.size() == 1)
    > vecObjects.clear();
    > else
    > vecObjects.erase(it);
    > }
    > ++it;
    >}


    the 'erase' invalidates all the iterators. You can't cache the 'end'
    iterator. Furthermore, you need to use the return value of erase,
    since you just invalidated 'it' when you erased it (and hence
    incrementing it is unsafe).
    --
    Mark Ping
     
    E. Mark Ping, Dec 22, 2005
    #9
  10. writes:

    > Hello,
    >
    > I am facing a starnge problem while erasing the last member in a
    > vector. I am using VC++ .NET 2002 complier. I have vector of
    > CComPtr<..> (irrelevant here), and then I iterate over the vector. If
    > it is the iterator, then I remove the element from the vector using
    > vecObjects.erase(it). It works fine till the last element. While
    > removing the last element it throws exception and fails. But the same
    > vecObject.clear() works with out any problem. Can somebody there,
    > please help me to indentify this problem and solution to this


    Difficult to say without seeing your code. One possibility is that
    you somewhere use an iterator which was earlier invalidated by
    the call to vector<CComPtr>::erase. When an element in a vector
    is erased, every iterator to that element and all elements following
    it are invalidated.

    If that is your problem, it could be possible that you could
    get around it by iterating from end towards begin instead.

    /Niklas Norrthon
     
    Niklas Norrthon, Dec 22, 2005
    #10
  11. Zara Guest

    On 21 Dec 2005 17:01:17 -0800, wrote:

    >Hello All,
    >

    (...)
    >
    >std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
    >std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()
    >
    >for(;it != itEnd; ++it); // I tried post increment also
    >{
    > // Check the iterator for some conditions
    > if (! IsValid(*it))
    > {
    > vecObjects.erase(it); // This causes the crash with the last
    >element
    > }
    > ++it;
    >}
    >

    (...)

    From the code you show us, I think the best soultion is to use the
    algorithm std::remove_if using as predicte a functor written around
    IsValid.

    std::vector<...>::iterator lastPosition=
    std::remove_if(vecObjectsbegin(),vecObjects.end(),wrapIsValid());
    vecObjects.erase(lastPosition,vecObjects.end());

    Or something similar.

    Best regards,

    Zara
     
    Zara, Dec 22, 2005
    #11
  12. writes:

    > Hello All,
    >
    > Thanks for the huge response, I have got. I am yet to try the
    > suggestions. In the mean time I am adding the code snippet here for
    > better clarity of the question I have asked.
    >
    > Though I cannot reproduce the code, it looks similar to this.
    >
    > I have a vector vecObjects with CComPtr<Interfaces I..>. Now I have to
    > check the contents for this vector for some conditions. Hence I iterate
    > the container as follows.
    >
    > std::vector< CComPtr<I..> >::iterator it = vecObjects.begin();
    > std::vector< CComPtr<I..> >::iterator itEnd = vecObjects.end()
    >
    > for(;it != itEnd; ++it); // I tried post increment also
    > {
    > // Check the iterator for some conditions
    > if (! IsValid(*it))
    > {
    > vecObjects.erase(it); // This causes the crash with the last
    > element
    > }
    > ++it;
    > }


    Consider including <algorithm> and <functional> and then do this
    instead:

    using std::not1;
    using std::ptr_fun;
    using std::remove_if;

    typedef std::vector<CComPtr<I...> >::iterator Iter;

    Iter it = remove_if(vecObjects.begin(), vecObjects.end(),
    not1(ptr_fun(IsValid)));
    vecObjects.erase(it, vecObjects.end());

    /Niklas Norrthon
     
    Niklas Norrthon, Dec 22, 2005
    #12
    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. ding feng
    Replies:
    8
    Views:
    874
    Dhruv
    Jul 2, 2003
  2. ma740988
    Replies:
    5
    Views:
    367
  3. Replies:
    8
    Views:
    2,003
    Csaba
    Feb 18, 2006
  4. Replies:
    13
    Views:
    705
    Noah Roberts
    Jul 11, 2006
  5. =?iso-8859-1?q?Erik_Wikstr=F6m?=

    Erasing in a vector while iterating through it

    =?iso-8859-1?q?Erik_Wikstr=F6m?=, Jun 11, 2007, in forum: C++
    Replies:
    3
    Views:
    825
    Christopher Dearlove
    Jun 11, 2007
Loading...

Share This Page