0

I have a vector of integers which I am traversing in reverse. I need to search a element in it and delete its first occurrence while traversing the vector. I am using the following function and getting segmentation fault

for(i=num.end()-1;i!=num.begin()-1;i--)
{
    tmp=binary(*i);
    if(tmp!=num.end())
        {num.erase(i);num.erase(tmp);res++;}
}

the function binary here returns an iterator whose value is surely to be less than i if the statements of erase function is to be used.

I used the following links :- How to traverse vector from end to start? and Erasing elements from a vector

but they suggest use of std::remove and std::remove_if functions which erase all the occurrences of the value.

Community
  • 1
  • 1
Tilak Raj Singh
  • 353
  • 2
  • 5
  • 15

1 Answers1

0

Edit: I was wrong, and your design is flawed.

The problem I overlooked: note that num.erase(x) invalidates references and iterators to elements after x. That means that num.erase(i);num.erase(tmp); is valid if tmp < i, but also means that i will not be valid afterwards. So, in the next iteration of the loop, your loop iterator will be invalid!

If you only intend to erase those two elements, you should exit the loop after erasing them. If you intend to delete multiple items in an end-to-beginning sweep, you need to find some way to advance your loop pointer to a point before the earliest deletion point. (although, you should also note that deleting multiple items out of the middle of a std::vector<> is inefficient -- generally taking O(N) per deletion from a vector of length N). You can also exit the loop and start again from the end (which is what I think @SHR meant by "reset the iterator").

If you can't do any of the above, you need some other kind of container, like a std::list<>, for which erase() does not invalidate iterators to the remaining elements. Also, these deletions are O(1) (although you lose the fast random access...)

If binary() behaves the way you say it does, your intention seems sound: erase()ing elements from the std::vector<> should leave references and iterators to elements before the deleted one intact.

The problem is likely elsewhere. If the code you posted is verbatim from your program, you have a formatting error in your if statement:

if(tmp!=num.end())
    num.erase(i);num.erase(tmp);res++;

Note that the only statement actually affected by the if condition is num.erase(i); -- the remaining statements will be executed unconditionally. Specifically, if tmp==num.end(), it will still execute num.erase(tmp), potentially causing a segmentation fault....

The likely minimal fix is to add curly-braces around the three statements:

if(tmp!=num.end())
    {num.erase(i);num.erase(tmp);res++;}
comingstorm
  • 25,557
  • 3
  • 43
  • 67