Why this multiset print code results forever loop?

Question

I want to print out every duplicates from a multiset, but somehow iterators behave strangely for me. How can fix this code? This code results a forever loop, that surprise me.

#include <set>
#include <iostream>
#include <sstream>

static void print_duplicate(const std::multiset<int>& mset)
{
  std::stringstream error_msg;
  for (auto it = mset.begin(); it != mset.end(); ++it)
    {
      unsigned count = mset.count(*it);
      if (count < 2)
        continue;

      error_msg << "Duplicated numbers found:\n";

      for (unsigned i = 0; i < count; ++it, ++i)
        error_msg << "\tNum:" << *it << "\n";
    }

  std::cout << error_msg.str();
}

int main()
{
  std::multiset<int> mset;

  // fill it
  mset.insert(1);
  mset.insert(1);
  mset.insert(1);

  print_duplicate(mset);
}

EDIT I added a --it at he end of the cycle

  for (unsigned i = 0; i < count; ++it, ++i)
    error_msg << "\tNum:" << *it << "\n";
  --it; // this line fix it
}

Answer 1

for (unsigned i = 0; i < count; ++it, ++i) When this loop ends, it will be equal to mset.end() and since you still have the other ++it from the main loop, you are getting something different to mset.end() hence the program never ends.

Answer 2

Inside your outer loop you might increment it more than once. Thus, the condition it != mset.end() might never be true, because the end has already been skipped. Incrementing a past-the-end iterator is undefined behavior, meaning it might also fail silently.

A possible solution might be to also check it != mset.end() in the inner for-loop:

for (unsigned i = 0; (i < count) && (it != mset.end()); ++it, ++i)
    error_msg << "\tNum:" << *it << "\n";

Answer 3

The problem seems to be that you slip past the end without detecting it. In the mset you create, there are only 1s. This means count will be 3 and the nested for (the one over i) will execute 3 times; when it finishes, it will be equal to mset.end(). Then control reaches the end of the outer for and it is incremented, becoming illegal and, more importantly, different from mset.end(). Which means the outer loop never terminates.

Answer 4

Quick Fix: Add an extra line after the inner loop: it--;

Reason is explained in Angew's answer.

Answer 5

Change the for loop to this:

for(std::container::iterator it=mset.begin(), it_next=it; it!=mset.end(); it=it_next) {
  ++it_next;
  ...
}

Essentially, you are skipping past the end of the loop because you are checking the variable against your iterator. Incrementing only the secondary iterator will prevent this from happening.