neel
neel

Reputation: 9071

Segmentation fault in C++ program

I am using STL list for my linked list implementation but when I am using erase function inside a loop it is giving segmentation fault. Can someone tell me why is it happening?

void remove(list<int> &myList,int N){
    int k = 1;
    list<int>::iterator it;
    for(it = myList.begin(); it != myList.end();it++){
        if(k == N){
            myList.erase(it);
            k = 1;
        }
        else
            k++;
    }
}

Upvotes: 0

Views: 243

Answers (3)

Jasper
Jasper

Reputation: 11908

If you erase an element, its iterator becomes invalid. In other words, when you get to the next iteration, you do it++ which no longer has a meaning because it no longer points to an element of a list.

Upvotes: 2

bbg
bbg

Reputation: 321

Do it like this,

void remove(list<int> &myList,int N){
    int k = 1;
    list<int>::iterator it;
    for(it = myList.begin(); it != myList.end();){
        if(k == N){
            myList.erase(it++);
            k = 1;
        } else{
            ++it;
            k++;
        }
    }
}

When the code myList.erase(it++) was executed, the object than iterator "it" represent is invalid.So , that is undefined to execute "it++"

Upvotes: 1

Benjamin Lindley
Benjamin Lindley

Reputation: 103741

When you call erase on an iterator, it invalidates that iterator. But you continue to use it. You need to capture the return value of erase, and assign that back to your iterator, like this:

it = myList.erase(it);

But this will necessitate a slight change in your loop. If you erase, then you don't want to increment, because then you will be skipping one element. This is especially bad if you end up erasing the last element, because then you will be moving past the end iterator. So, you should only increment if you don't erase:

for(it = myList.begin(); it != myList.end(); ){
    if(k == N){
        it = myList.erase(it);
        k = 1;
    }
    else
    {
        k++;
        ++it;
    }
}

Upvotes: 6

Related Questions