Code sample: KDElibs

A small bug fix, providing a small sample of both code and bug-handling comments.

This part of KDE is under the Gnu GPL (code) and Gnu GFDL (documentation); only two short excerpts are reproduced here.

Bug overview

Link to Debian's bug tracking system

KDE has a standard text-field component. The search-and-replace function could lock up in a infinite-loop, forcing the user to kill the application and lose any unsaved changes.

When handling a replacement string containing an escaped backslash followed by a number, such as "\\0", the old code first found the "\0", then checked for a preceeding backslash, and became stuck if there was one.

A two-part fix, the first part being to decide and document how it should handle escape sequences.

Documentation addition

Use Placeholders
If selected, the replacement string may use backreferences (see Regular Expressions). Backslashes, newlines and tabs may be inserted with the sequences "\\", "\n" and "\t" respectively.

Code change

The code is using Qt's QtString class, which handles ref-counting and deallocation of the underlying character arrays

The existing code that follows the changed section checked for and handled the addition of newlines, which is why I added support for "\n" in the replacement string.

void KateSearch::replaceOne()
{
  QString replaceWith = m_replacement;
  if ( s.flags.regExp && s.flags.useBackRefs ) {
    // replace each "(?!\)\d+" with the corresponding capture
 
 
 
    QRegExp br("\\\\(\\d+)");
    int pos = br.search( replaceWith );
    int ncaps = m_re.numCaptures();
    while ( pos >= 0 ) {
      QString sc;
      if ( !pos ||  replaceWith.at( pos-1) != '\\' ) {
        int ccap = br.cap(1).toInt();
 
 
        if (ccap <= ncaps ) {
          sc = m_re.cap( ccap );
          replaceWith.replace( pos, br.matchedLength(), sc );
        }
        else {
          kdDebug()<<"same message as in the new version
 
        }
 
 
 
 
 
 
 
 
      }
      pos = br.search( replaceWith, pos + (int)sc.length() );
    }
  }
 
  //... rest of function, including check for added newlines
}
void KateSearch::replaceOne()
{
  QString replaceWith = m_replacement;
  if ( s.flags.regExp && s.flags.useBackRefs ) {
    // Replace each "\0"..."\9" with the corresponding capture,
    // "\n" and "\t" with newline and tab,
    // "\\" with "\",
    // and remove the "\" for any other sequence.
    QRegExp br("\\\\(.)");
    int pos = br.search( replaceWith );
    int ncaps = m_re.numCaptures();
    while ( pos >= 0 ) {
      QString substitute;
      QChar argument = br.cap(1).at(0);
      if ( argument.isDigit() ) {
        // the second character is a digit, this is a backreference
        int ccap = argument.digitValue();
        if (ccap <= ncaps ) {
          substitute = m_re.cap( ccap );
 
 
        } else {
          kdDebug()<<"KateSearch::replaceOne(): you don't have "<<ccap<<" backreferences in regexp '"<<m_re.pattern()<<"'"<<endl;
          break;
        }
      } else if ( argument == 'n' ) {
        substitute = '\n';
      } else if ( argument == 't' ) {
        substitute = '\t';
      } else {
        // handle a validly escaped backslash, or an invalid escape.
        substitute = argument;
      }
      replaceWith.replace( pos, br.matchedLength(), substitute );
      pos = br.search( replaceWith, pos + substitute.length() );
    }
  }
 
  //... rest of function, including check for added newlines
}

Thanks to Lorenzo Bettini for GNU source-highlight