C++ Segmentation when using erase on std::list

Question

I'm trying to remove items from a C++ linked list using erase and a list iterator:

#include <iostream>
#include <string>
#include <list>

class Item
{
  public:
    Item() {}
    ~Item() {}
};

typedef std::list<Item> list_item_t;


int main(int argc, const char *argv[])
{

  // create a list and add items
  list_item_t newlist;
  for ( int i = 0 ; i < 10 ; ++i )
  {
    Item temp;
    newlist.push_back(temp);
    std::cout << "added item #" << i << std::endl;
  }

  // delete some items
  int count = 0;
  list_item_t::iterator it;

  for ( it = newlist.begin(); count < 5 ; ++it )
  {
    std::cout << "round #" << count << std::endl;
    newlist.erase( it );
    ++count;
  }
  return 0;
}

I get this output and can't seem to trace the reason:

added item #0
added item #1
added item #2
added item #3
added item #4
added item #5
added item #6
added item #7
added item #8
added item #9
round #0
round #1
Segmentation fault

I'm probably doing it wrong, but would appreciate help anyway. thanks.

score 24 · Accepted Answer · edited May 23 '17 at 11:45

24

The core problem here is you're using at iterator value, it, after you've called erase on it. The erase method invalidates the iterator and hence continuing to use it results in bad behavior. Instead you want to use the return of erase to get the next valid iterator after the erased value.

it = newList.begin();
for (int i = 0; i < 5; i++) {
  it = newList.erase(it);
}

It also doesn't hurt to include a check for newList.end() to account for the case where there aren't at least 5 elements in the list.

it = newList.begin();
for (int i = 0; i < 5 && it != newList.end(); i++) {
  it = newList.erase(it);
}

As Tim pointed out, here's a great reference for erase

http://www.cplusplus.com/reference/stl/list/erase/

edited May 23 '17 at 11:45

Community

1
1

answered Feb 28 '11 at 17:58

JaredPar

733,204
149
1,241
1,454

1

Better answer than mine. You don't want to combine it++ and erase, and this neatly sidesteps that. But I do like sharing links to some of my favorite reference pages: [list::erase](http://www.cplusplus.com/reference/stl/list/erase/) – Tim Feb 28 '11 at 18:05
@Tim, added the reference to my answer. It's my goto page as well for C++ STL questions. – JaredPar Feb 28 '11 at 18:09

score 3 · Answer 2 · answered Feb 28 '11 at 18:00

3

When you erase an element at position it, the iterator it gets invalidated - it points to a piece of memory that you just freed.

The erase(it) function returns another iterator pointing to the next element to the list. Use that one!

answered Feb 28 '11 at 18:00

CygnusX1

20,968
5
65
109

Azaral · Answer 3 · 2012-04-29T02:14:57.937

2

I do this:

for(list<type>::iterator i = list.begin(); i != list.end(); i++)
{
     if(shouldErase)
     { 
        i = list.erase(i);
        i--;
     }
}

Edited because I'm a bonehead that can't read apparently lol.

edited Apr 29 '12 at 02:14

answered Apr 28 '12 at 15:50

Azaral

140
9

There is no skipping problem with the accepted answer: the loop doesn't increment the iterator, **it**, but an integer counter, **i**. – alexisdm Apr 29 '12 at 01:05
o yeah, I missed them being different lol. stupid t. editing my answer – Azaral Apr 29 '12 at 02:07
Though I don't see how the code in the answer would work either. Unless I'm an idiot again, it looks like the code would try to erase the beginning of the list, and then the next item, and the next item, until i gets to 5. – Azaral Apr 29 '12 at 02:20
Because the question was about deleting the first 5 items of the list. But anyway, removing the `i++` from the loop statement and putting it as an else clause would be better. – alexisdm Apr 30 '12 at 06:38
Yeah, that would be better I think. The question was about fixing the seg fault though, not just removing the first five elements of the list. That just happened to be what he was trying to do. – Azaral Apr 30 '12 at 11:42

Fred Larson · Answer 4 · 2011-02-28T18:06:05.777

2

You're invalidating your iterator when you erase() within the loop. It would be simpler to do something like this in place of your erase loop:

list_item_t::iterator endIter = newlist.begin();
std::advance(endIter, 5);
newList.erase(newlist.begin(), endIter);

You might also be interested in the erase-remove idiom.

edited Feb 28 '11 at 18:06

answered Feb 28 '11 at 18:00

Fred Larson

60,987
18
112
174

C++ Segmentation when using erase on std::list

4 Answers4

Linked