Error "Disallowed system call: SYS_kill" as I try to remove all consecutive duplicate elements from a string

0
votes

The following

#include <iostream>
#include <string>

void remove_duplicates ( std::string & s )
{

   for (std::string::iterator it(s.begin()), offend(s.end()); it != offend; ++it)
   {
      std::string::iterator temp = it;
      while (++it != offend && *it == *temp);
      if ((it-temp)>1) s.erase(temp, it);
   }
}


int main()
{
   std::string str = "aaabbcaaaaa";
   remove_duplicates(str);
   std::cout << str; /* Expected output: "abca" */
   return 0;
}

is producing the error

/usr/local/lib/gcc/i686-pc-linux-gnu/4.1.2/../../../../include/c++/4.1.2/bits/basic_string.h:1154: __gnu_cxx::__normal_iterator::other::pointer, std::basic_string<_CharT, _Traits, _Alloc> > std::basic_string<_CharT, _Traits, _Alloc>::erase(__gnu_cxx::__normal_iterator::other::pointer, std::basic_string<_CharT, _Traits, _Alloc> >, __gnu_cxx::__normal_iterator::other::pointer, std::basic_string<_CharT, _Traits, _Alloc> >) [with _CharT = char, _Traits = std::char_traits, _Alloc = std::allocator]: Assertion '__first >= _M_ibegin() && __first <= __last && __last <= _M_iend()' failed.

Disallowed system call: SYS_kill

when I run it on http://codepad.org/KXgHqKS2.

Is there a problem with the logic of my function? If so, what is it, and is there a cleaner way to solve the problem?

2
c++stringalgorithm
Aren't you invalidating your iterators inside that loop? Good luck in the election. – Lightness Races in Orbit
You should read the error message more carefully: "Assertion [...] failed.". In other words, something happened that represents a programming error, either in your code or, very unlikely, in the implementation of the C++ standardlibrary. – Ulrich Eckhardt

2 Answers

2
votes

Is there a problem with the logic of my function?

Yes, erasing elements invalidates the iterators. If you want to do this by steam, you'll need to make two changes:

  • don't store the end iterator between iterations, or update it after erasing;
  • update it after erasing, it = erase(temp, it)

Is there a cleaner way to solve the problem?

s.erase(std::unique(s.begin(), s.end()), s.end());
0
votes

The code s.erase(temp, it); invalidates the iterator it, and ++it in the for loop then becomes undefined behavior.

Following is my implementation of remove_duplicates for your interest: 

void remove_duplicates(std::string& str) {
  if (str.empty()) return;
  std::string::iterator it = str.begin();
  std::string::iterator itt = it + 1;
  while (itt < str.end()) {
    if (*itt == *it) {
      ++itt;
    }
    else {
      *++it = *itt++;
    }
  }
  str.resize(it - str.begin() + 1);
}