C++ map container erase() segmentation fault

Question

I use this code to remove elements from map container that equal to some int.

for(auto x:m){
  if((x.second)==element)m.erase(x.first);
}

As result Segmentation fault. I also tried this:

for(map<int,int>::iterator i=m.begin();i!=m.end();i++){
  if((i->second)==element)m.erase(i);
}

Same result. If you put i++ into if/else program will freeze/loop or something. How can I fix this?

Step 0: show us a [mcve]. Not just code snippets. – Jesper Juhl May 21 '20 at 17:22 — Jesper Juhl, May 21 '20 at 17:22

john · Accepted Answer · 2020-05-21T19:24:18.307

5

erase() invalidates the iterator being used by the for loop. Fortunately, erase() itself returns an iterator to the next entry, so the correct loop would look like this instead:

for (map<int,int>::iterator i = m.begin(); i != m.end(); )
{
    if (i->second == element)
        i = m.erase(i);
    else
        ++i;
}

edited May 21 '20 at 19:24

answered May 21 '20 at 17:22

john

85,011
4
57
81

Note in particular the absence of `++i` on the first line. Very important. – Asteroids With Wings May 21 '20 at 17:32

score 2 · Answer 2 · answered May 21 '20 at 17:33

2

In addition to @john's answer, if your C++ Standard Library implementation supports it, you can invoke the std::erase_if(map, condition) helper:

std::erase_if(m, [](const auto& item) {
    auto const& [key, value] = item;
    // Write your erasing condition here, e.g.:
    // return value == element;
});

answered May 21 '20 at 17:33

Mr.C64

41,637
14
86
162

It's a shame you can't put the structured binding in the parameter list. Kind of. – Asteroids With Wings May 21 '20 at 17:35
1

Structured bindings is great, but here, I would simply prefer `return item.second == element;`. It's simply shorter. – Daniel Langr May 21 '20 at 17:37
@DanielLangr: `key` and `value` (or even more context-specific names) are in general more readable than `first` and `second`. – Mr.C64 May 21 '20 at 17:38
1

I like it how it is in the answer. It is free to the computer, and well worth the extra line in terms of cognitive load. `.first` and `.second` are just horrible. – Asteroids With Wings May 21 '20 at 17:40

C++ map container erase() segmentation fault

2 Answers2