Обзор дефектов кода музыкального софта. Часть 3. Rosegarden

• Введение
• Пример выявления ошибки, где помогает Data-flow analysis
• Повесть о недостижимом коде
• Без комментариев
• Ошибки неправильной работы с типами данных
• Ошибки с итераторами
• Ошибки с указателями
• Редкая ошибка
• Разное
• Заключение

Программы для работы с музыкой имеют маленький объём кода и, поначалу, я сомневался в возможности находить достаточное количество ошибок для статей. Тематику музыкального софта всё равно хотелось затронуть, поэтому я был готов объединять несколько проектов в статье. И вот я пишу уже третью статью, стараясь хоть как-то вместить интересные ошибки в одну статью. Третьим проектом для анализа выбран MIDI-секвенсор и нотный редактор - Rosegarden. Внимание! Прочтение статьи вызывает "Facepalm"!

Введение

Rosegarden — свободный MIDI-секвенсор, нотный редактор для Linux, использующий ALSA и JACK, программа для создания и редактирования музыки наподобие Apple Logic Pro, Cakewalk Sonar и Steinberg Cubase.

В статью вошли только наиболее интересные ошибки, найденные мною с помощью PVS-Studio. Для просмотра полного отчёта авторы могут самостоятельно проверить проект, запросив в поддержке временный ключ.

PVS-Studio - это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#. Работает в среде Windows и Linux.

Пример выявления ошибки, где помогает Data-flow analysis

Ложные предупреждения всегда составляют некую часть отчёта профессионального статического анализатора кода. Обидно, когда люди просто не хотят написать код лучше и просто списывают предупреждения как ложные. Иногда код написан так запутанно, что другому программисту не под силу разобраться без отладки. Так или иначе, мы стараемся учитывать такие ситуации, чтобы анализатор не выдавал ложные предупреждения. Для этого сейчас активно развивается Data-flow analysis, что, кроме уменьшения количества ложных предупреждений, позволяет находить интересные ошибки.

V560 A part of conditional expression is always false: singleStaff. NotationScene.cpp 1707

void NotationScene::layout(....)
{
  ....
  bool full = (singleStaff == 0 && startTime == endTime);

  m_hlayout->setViewSegmentCount(m_staffs.size());

  if (full) {
    Profiler profiler("....", true);

    m_hlayout->reset();
    m_vlayout->reset();

    bool first = true;

    for (unsigned int i = 0; i < m_segments.size(); ++i) {

      if (singleStaff &&  // <= Always False
          m_segments[i] != &singleStaff->getSegment()) {
        continue;
      }

      timeT thisStart = m_segments[i]->getClippedStartTime();
      timeT thisEnd = m_segments[i]->getEndMarkerTime();

      if (first || thisStart < startTime) startTime = thisStart;
      if (first || thisEnd > endTime) endTime = thisEnd;

      first = false;
    }
  }
  ....
}

Из-за логической ошибки в цикле for никогда не выполняется оператор continue, что, вероятно, приводит к выполнению лишних итераций цикла. Причиной этого является проверка указателя singleStaff в условии с оператором '&&'. Значение указателя singleStff всегда равно нулю. Весь этот код находится под условием "if (full)", при вычислении которого анализатор увидел зависимость от переменной singleStaff:

bool full = (singleStaff == 0 && startTime == endTime);

Значение переменной full будет равно true только в том случае, если указатель singleStaff будет нулевым.

Повесть о недостижимом коде

В этом разделе я собрал разные примеры ошибок, так или иначе приводящих к невыполнению кода. Всё это относится к CWE-571: Expression is Always True, CWE-570: Expression is Always False, CWE-561: Dead Code и их вариациям.

V547 Expression '!beamedSomething' is always true. SegmentNotationHelper.cpp 1405

void SegmentNotationHelper::makeBeamedGroupAux(....)
{
  int groupId = segment().getNextId();
  bool beamedSomething = false;             // <=

  for (iterator i = from; i != to; ++i) {
  ....
  if ((*i)->isa(Note::EventType) &&
    (*i)->getNotationDuration() >= Note(....).getDuration()) {
    if (!beamedSomething) continue;         // <=
    iterator j = i;
    bool somethingLeft = false;
    while (++j != to) {
      if ((*j)->getType() == Note::EventType &&
        (*j)->getNotationAbsoluteTime() > (*i)->get....() &&
        (*j)->getNotationDuration() < Note(....).getDuration()) {
        somethingLeft = true;
        break;
      }
    }
    if (!somethingLeft) continue;
  }
  ....
}

Этот пример очень похож на код, приведённый в предыдущем разделе, но немного проще. Переменная beamedSomething инициализируется значением false и больше не изменяется. Вследствие чего в цикле for всегда выполняется оператор continue, из-за чего никогда не выполняется большой фрагмент кода.

V547 Expression 'i > 5' is always false. SegmentParameterBox.cpp 323

void SegmentParameterBox::initBox()
{
  ....
  for (int i = 0; i < 6; i++) {
    timeT time = 0;
    if (i > 0 && i < 6) {
        time = Note(Note::Hemidemisemiquaver).get.... << (i - 1);
    } else if (i > 5) {
        time = Note(Note::Crotchet).getDuration() * (i - 4);
    }
  ....
}

Счётчик цикла принимает значения в диапазоне от 0 до 5. Первое условное выражение выполняется для всех значений счётчика, кроме нулевого, в то время как второе условное выражение не выполняется никогда, т.к. ожидает, что переменная i будет иметь значение от 6 и более.

V547 Expression 'adjustedOctave < 8' is always false. NotePixmapFactory.cpp 1920

QGraphicsPixmapItem* NotePixmapFactory::makeClef(....)
{
  ....
  int oct = clef.getOctaveOffset();
  if (oct == 0) return plain.makeItem();

  int adjustedOctave = (8 * (oct < 0 ? -oct : oct));
  if (adjustedOctave > 8)
      adjustedOctave--;
  else if (adjustedOctave < 8)
      adjustedOctave++;
  ....
}

Начнём разбирать пример по порядку. Переменная oct сначала инициализируется неким значением знакового типа, а потом из этого диапазона исключается нулевое значение. Далее вычисляется модуль переменной oct и умножается на 8. Результирующее значение в adjustedOctave будет иметь диапазон [8..N), что делает проверку (adjustedOctave < 8) бессмысленной.

V547 Expression '""' is always true. LilyPondOptionsDialog.cpp 64

LilyPondOptionsDialog::LilyPondOptionsDialog(....)
{
  setModal(true);
  setWindowTitle((windowCaption = "" ?
    tr("LilyPond Export/Preview") : windowCaption));
  ....
}

Интересная ошибка с формированием заголовка модального окна. Видимо хотели задавать новый заголовок окна, если текущее значение отсутствует, но допустили ошибку в операторе.

Для тех, кто сразу не заметил опечатку, подскажу. Должен был использоваться оператор '==', а не оператор '='.

Такой же код используется при показе другого окна:

• V547 Expression '""' is always true. MusicXMLOptionsDialog.cpp 60

Примечание. Если автор кода хотел в одной строке задать новый заголовок и стереть старый таким образом, то нет — так писать не круто.

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 223, 239. IntervalDialog.cpp 223

QString IntervalDialog::getIntervalName(....)
{
  ....
  if (deviation == -1)
    textIntervalDeviated += tr("a minor");
  else if (deviation == 0)                               // <=
    textIntervalDeviated += tr("a major");
  else if (deviation == -2)
    textIntervalDeviated += tr("a diminished");
  else if (deviation == 1)
    textIntervalDeviated += tr("an augmented");
  else if (deviation == -3)
    textIntervalDeviated += tr("a doubly diminished");
  else if (deviation == 2)
    textIntervalDeviated += tr("a doubly augmented");
  else if (deviation == -4)
    textIntervalDeviated += tr("a triply diminished");
  else if (deviation == 3)
    textIntervalDeviated += tr("a triply augmented");
  else if (deviation == 4)
    textIntervalDeviated += tr("a quadruply augmented");
  else if (deviation == 0)                               // <=
    textIntervalDeviated += tr("a perfect");
  ....
}

Одно из условий лишнее или записано с ошибкой. Значение 0 уже обработано в самом начале.

Без комментариев

В этом разделе я приведу несколько интересных фрагментов кода для работы с файлами. Похоже, программист вдохновлялся такими языками программирования, как C# и Java. В противном случае непонятно, почему не создать экземпляр типа ifstream просто как переменную на стеке. Динамическое выделение памяти здесь явно избыточно и, вдобавок, стало поводом для ошибки.

V773 The function was exited without releasing the 'testFile' pointer. A memory leak is possible. RIFFAudioFile.cpp 561

AudioFileType
RIFFAudioFile::identifySubType(const QString &filename)
{
  std::ifstream *testFile =
    new std::ifstream(filename.toLocal8Bit(),
                      std::ios::in | std::ios::binary);

  if (!(*testFile))
    return UNKNOWN;
  ....
  testFile->close();
  delete testFile;
  delete [] bytes;

  return type;
}

Если с файлом возникают проблемы, то указатель testFile не освобождается при выходе из функции. Это распространённый паттерн, приводящий к утечке памяти.

V773 The function was exited without releasing the 'midiFile' pointer. A memory leak is possible. MidiFile.cpp 1531

bool
MidiFile::write(const QString &filename)
{
  std::ofstream *midiFile =
    new std::ofstream(filename.toLocal8Bit(),
                        std::ios::out | std::ios::binary);

  if (!(*midiFile)) {
    RG_WARNING << "write() - can't write file";
    m_format = MIDI_FILE_NOT_LOADED;
    return false;
  }
  ....
  midiFile->close();

  return true;
}

Вы могли подумать, что этот фрагмент кода такой же, как предыдущий, но это не совсем так. В отличии от первого примера, в этой функции вообще нет освобождения памяти. Утечка памяти происходит всегда.

V668 There is no sense in testing the 'file' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. SF2PatchExtractor.cpp 94

SF2PatchExtractor::Device
SF2PatchExtractor::read(string fileName)
{
  Device device;

  ifstream *file = new ifstream(fileName.c_str(), ios::in |....);
  if (!file)
    throw FileNotFoundException();
  ....
}

Вот список проблем этого фрагмента кода:

• Код написан избыточно сложно;
• Проверка указателя здесь не имеет смысла (оператор new сгенерирует исключение, если не сможет выделить память под объект);
• Ситуация с отсутствием файла не обрабатывается;
• Утечка памяти, т.к. указатель далее нигде не освобождается.

Причём такое место не одно:

• V668 There is no sense in testing the 'statstream' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. RosegardenMainWindow.cpp 4672
• V668 There is no sense in testing the 'file' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. SF2PatchExtractor.cpp 67

Ошибки неправильной работы с типами данных

V601 The integer type is implicitly cast to the char type. MidiEvent.cpp 181

QDebug &
operator<<(QDebug &dbg, const MidiEvent &midiEvent)
{
  timeT tempo;
  int tonality;
  std::string sharpflat;
  ....
  tonality = (int)midiEvent.m_metaMessage[0];

  if (tonality < 0) {
    sharpflat = -tonality + " flat"; // <=
  } else {
    sharpflat = tonality;            // <=
    sharpflat += " sharp";
  }
  ....
}

Предположим, значение переменной tonality было '42', тогда в указанных в коде местах хотели получить такие строки: "42 flat" или "42 sharp". Но это работает совсем не так, как ожидает программист. Конвертации числа в строку не происходит, вместо этого в строку сохраняется смещённый указатель, образуя мусор в буфере. Или случится access violation. Или вообще произойдет что угодно, так как доступ за границу массива приводит к неопределённому поведению программы.

Исправить ошибку можно следующим образом:

if (tonality < 0) {
  sharpflat = to_string(-tonality) + " flat";
} else {
  sharpflat = to_string(tonality);
  sharpflat += " sharp";
}

V674 The '0.1' literal of the 'double' type is compared to a value of the 'int' type. Consider inspecting the 'm_connectingLineLength > 0.1' expression. StaffLayout.cpp 1028

class StaffLayout
{
  ....
protected:
  int m_connectingLineLength;
  ....
}

int m_connectingLineLength;

void
StaffLayout::resizeStaffLineRow(int row, double x, double length)
{
  ....
  if (m_pageMode != LinearMode && m_connectingLineLength > 0.1) {
  ....
}

Сравнение переменой типа int со значением 0.1 выполнять бессмысленно. Возможно, здесь задумывалось что-то другое. Авторам проекта стоит внимательно изучить этот код.

V601 The string literal is implicitly cast to the bool type. FileSource.cpp 902

bool
FileSource::createCacheFile()
{
  {
    QMutexLocker locker(&m_mapMutex);

#ifdef DEBUG_FILE_SOURCE
    std::cerr << "...." << m_refCountMap[m_url] << std::endl;
#endif

    if (m_refCountMap[m_url] > 0) {
      m_refCountMap[m_url]++;
      m_localFilename = m_remoteLocalMap[m_url];
#ifdef DEBUG_FILE_SOURCE
      std::cerr << "...." << m_refCountMap[m_url] << std::endl;
#endif
      m_refCounted = true;
      return true;
    }
  }

  QDir dir;
  try {
      dir = TempDirectory::getInstance()->....;
  } catch (DirectoryCreationFailed f) {
#ifdef DEBUG_FILE_SOURCE
      std::cerr << "...." << f.what() << std::endl;
#endif
      return "";  // <=
  }
  ....
}

В одном месте, вместо значений true/false, функция возвращает пустую строку, что всегда интерпретируется как true.

Ошибки с итераторами

Использование итераторов в этом проекте выглядит не менее странным, чем работа с файлами.

V783 Dereferencing of the invalid iterator 'i' might take place. IconStackedWidget.cpp 126

void
IconStackedWidget::slotPageSelect()
{
  iconbuttons::iterator i = m_iconButtons.begin();
  int index = 0;
  while (((*i)->isChecked() == false) &&
         (i != m_iconButtons.end())) {
    ++i;
    index++;
  }
  m_pagePanel->setCurrentIndex(index);
}

В цикле while перепутан порядок проверки итератора i. В этом коде ничего необычного нет, классическая ошибка.

V783 Dereferencing of the invalid iterator 'beatTimeTs.end()' might take place. CreateTempoMapFromSegmentCommand.cpp 119

void
CreateTempoMapFromSegmentCommand::initialise(Segment *s)
{
 ....
 std::vector<timeT> beatTimeTs;
 ....
 for (int i = m_composition->...At(*beatTimeTs.begin() - 1) + 1;
          i <= m_composition->...At(*beatTimeTs.end() - 1); ++i){
 ....
}

Анализатор обнаружил ещё одно обращение к итератору end(). Возможно, хотели получить примерно такой код:

...At(*(beatTimeTs.end() - 1))

но забыли скобочки.

Похожий код есть и в другом файле:

• V783 Dereferencing of the invalid iterator 'm_segments.end()' might take place. StaffHeader.cpp 250

Ошибки с указателями

V1004 The 'track' pointer was used unsafely after it was verified against nullptr. Check lines: 319, 329. MatrixView.cpp 329

void
MatrixView::slotUpdateWindowTitle(bool m)
{
  ....
  Track *track =
    m_segments[0]->getComposition()->getTrackById(trackId);

  int trackPosition = -1;
  if (track)
      trackPosition = track->getPosition();                // <=

  QString segLabel = strtoqstr(m_segments[0]->getLabel());
  if (segLabel.isEmpty()) {
      segLabel = " ";
  } else {
      segLabel = QString(" \"%1\" ").arg(segLabel);
  }

  QString trkLabel = strtoqstr(track->getLabel());         // <=
  ....
}

Стрелочками я отметил два места, в которых разыменовывается указатель track. Первое место является безопасным, т.к. указатель точно ненулевой. Второе место может приводить к неопределённому поведению программы. В приведённом фрагменте кода нет косвенных проверок. Код выполняется последовательно и содержит потенциальную ошибку.

Другие опасные разыменования указателей:

• V1004 The 'track' pointer was used unsafely after it was verified against nullptr. Check lines: 2528, 2546. RosegardenDocument.cpp 2546
• V1004 The 'inst' pointer was used unsafely after it was verified against nullptr. Check lines: 392, 417. ManageMetronomeDialog.cpp 417
• V1004 The 'controller' pointer was used unsafely after it was verified against nullptr. Check lines: 75, 84. ControllerEventsRuler.cpp 84

V595 The 'm_scene' pointer was utilized before it was verified against nullptr. Check lines: 1001, 1002. NotationWidget.cpp 1001

void
NotationWidget::slotEnsureTimeVisible(timeT t)
{
  m_inMove = true;
  QPointF pos = m_view->mapToScene(0,m_view->height()/2);
  pos.setX(m_scene->getRulerScale()->getXForTime(t));     // <=
  if (m_scene) m_scene->constrainToSegmentArea(pos);      // <=
  m_view->ensureVisible(QRectF(pos, pos));
  m_inMove = false;
}

Диагностика V595 находит похожий тип ошибок. Здесь указатель s_scene разыменовывается в одной строке, а сразу на следующей проверяется, является ли он валидным.

V595 The 'm_hideSignatureButton' pointer was utilized before it was verified against nullptr. Check lines: 248, 258. TimeSignatureDialog.cpp 248

TimeSignature
TimeSignatureDialog::getTimeSignature() const
{
  QSettings settings;
  settings.beginGroup( GeneralOptionsConfigGroup );

  settings.setValue("timesigdialogmakehidden",
    m_hideSignatureButton->isChecked());                    // <=
  settings.setValue("timesigdialogmakehiddenbars",
    m_hideBarsButton->isChecked());                         // <=
  settings.setValue("timesigdialogshowcommon",
    m_commonTimeButton->isChecked());                       // <=
  settings.setValue("timesigdialognormalize",
    m_normalizeRestsButton->isChecked());

  TimeSignature ts(m_timeSignature.getNumerator(),
                   m_timeSignature.getDenominator(),
                   (m_commonTimeButton &&
                    m_commonTimeButton->isEnabled() &&
                    m_commonTimeButton->isChecked()),
                   (m_hideSignatureButton &&                // <=
                    m_hideSignatureButton->isEnabled() &&
                    m_hideSignatureButton->isChecked()),
                   (m_hideBarsButton &&
                    m_hideBarsButton->isEnabled() &&
                    m_hideBarsButton->isChecked()));

  settings.endGroup();

  return ts;
}

Похожая на предыдущий пример ошибка, но я решил всё равно привести этот фрагмент кода. Здесь выполняется сразу три разыменования потенциально нулевых указателей.

Все остальные похожие места приведу списком:

• V595 The 'm_timeT' pointer was utilized before it was verified against nullptr. Check lines: 690, 696. TimeWidget.cpp 690
• V595 The 'm_scene' pointer was utilized before it was verified against nullptr. Check lines: 526, 538. NoteRestInserter.cpp 526
• V595 The 'item' pointer was utilized before it was verified against nullptr. Check lines: 318, 320. TempoView.cpp 318
• V595 The 'm_scene' pointer was utilized before it was verified against nullptr. Check lines: 902, 903. MatrixWidget.cpp 902
• V595 The 'm_seqManager' pointer was utilized before it was verified against nullptr. Check lines: 1029, 1058. RosegardenMainWindow.cpp 1029
• V595 The 'm_seqManager' pointer was utilized before it was verified against nullptr. Check lines: 5690, 5692. RosegardenMainWindow.cpp 5690
• V595 The 'm_seqManager' pointer was utilized before it was verified against nullptr. Check lines: 5701, 5704. RosegardenMainWindow.cpp 5701
• V595 The 'm_controller' pointer was utilized before it was verified against nullptr. Check lines: 553, 563. ControllerEventsRuler.cpp 553
• V595 The 'e' pointer was utilized before it was verified against nullptr. Check lines: 418, 420. MarkerRuler.cpp 418
• V595 The 'm_doc' pointer was utilized before it was verified against nullptr. Check lines: 490, 511. SequenceManager.cpp 490
• V595 The 'm_groupLocalEventBuffers' pointer was utilized before it was verified against nullptr. Check lines: 329, 335. DSSIPluginInstance.cpp 329
• V595 The 'm_instrumentMixer' pointer was utilized before it was verified against nullptr. Check lines: 699, 709. AudioProcess.cpp 699

Редкая ошибка

Ошибка с порядком инициализации членов класса является очень редкой. В нашей базе ошибок есть всего двенадцать упоминаний об этой ошибке.

V670 The uninitialized class member 'm_intervals' is used to initialize the 'm_size' member. Remember that members are initialized in the order of their declarations inside a class. Tuning.cpp 394

class Tuning {
  ....
  int m_size;                      // line 138
  const IntervalList *m_intervals; // line 139
  ....
}

Tuning::Tuning(const Tuning *tuning) :
  m_name(tuning->getName()),
  m_rootPitch(tuning->getRootPitch()),
  m_refPitch(tuning->getRefPitch()),
  m_size(m_intervals->size()),
  m_intervals(tuning->getIntervalList()),
  m_spellings(tuning->getSpellingList())
{
  ....
}

Поля класса инициализируются в том порядке, в каком определены в классе. В приведённом примере кода поле m_size проинициализируется первым и будет иметь некорректное значение.

Разное

V557 Array overrun is possible. The value of 'submaster' index could reach 64. SequencerDataBlock.cpp 325

#define SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS 64

class SequencerDataBlock
{
  ....
protected:
  int m_submasterLevelUpdateIndices[64];
  ....
}

bool
SequencerDataBlock::getSubmasterLevel(int submaster, ....) const
{
 ....int lastUpdateIndex[SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS];

 if (submaster < 0 ||
     submaster > SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS) {
   info.level = info.levelRight = 0;
   return false;
 }

 int currentUpdateIndex=m_submasterLevelUpdateIndices[submaster];
 info = m_submasterLevels[submaster];

 if (lastUpdateIndex[submaster] != currentUpdateIndex) {
   lastUpdateIndex[submaster] = currentUpdateIndex;
   return true;
 } else {
   return false; // no change
 }
}

Эта ошибка уже стала классикой. При сравнении индекса массива с максимальным значением, почему-то всегда путают оператор '>' и '>='. Такая ошибка приводит к обращению за пределы массива. В данном случае, даже к двум массивам.

Правильная проверка должна выглядеть так:

if (submaster < 0 ||
    submaster >= SEQUENCER_DATABLOCK_MAX_NB_SUBMASTERS) {

Такой код скопирован в ещё две функции:

• V557 Array overrun is possible. The value of 'submaster' index could reach 64. SequencerDataBlock.cpp 343
• V557 Array overrun is possible. The value of 'submaster' index could reach 64. SequencerDataBlock.cpp 344

V612 An unconditional 'break' within a loop. Fingering.cpp 83

Fingering::Barre
Fingering::getBarre() const
{
  int lastStringStatus = m_strings[getNbStrings() - 1];
  Barre res;
  res.fret = lastStringStatus;

  for(unsigned int i = 0; i < 3; ++i) {
    if (m_strings[i] > OPEN && m_strings[i] == lastStringStatus)
      res.start = i;
      break;
  }

  res.end = 5;
  return res;
}

Я уже приводил примеры кода, стили которых были похожи на C# или Java. Вот тут явное сходство с языком Python. К сожалению (для автора кода), в C++ это так не работает. Оператор break не находится в условии, а выполняется всегда на первой итерации цикла.

V746 Object slicing. An exception should be caught by reference rather than by value. MupExporter.cpp 197

timeT MupExporter::writeBar(....)
{
  ....
  try {
      // tuplet compensation, etc
      Note::Type type = e->get<Int>(NOTE_TYPE);
      int dots = e->get
                 <Int>(NOTE_DOTS);
      duration = Note(type, dots).getDuration();
  } catch (Exception e) { // no properties
      RG_WARNING << "WARNING: ...: " << e.getMessage();
  }
  ....
}

Перехват исключения по значению может приводить к нескольким типам шибок. В коде проекта я нашёл такой класс:

class BadSoundFileException : public Exception

При перехвате исключения по значению, будет создан новый объект класса Exception, а информация об унаследованном классе BadSoundFileException будет утеряна.

Таких мест в проекте около пятидесяти.

V523 The 'then' statement is equivalent to the 'else' statement. HydrogenXMLHandler.cpp 476

bool
HydrogenXMLHandler::characters(const QString& chars)
{
 bool rc=false;

 if (m_version=="") {
   /* no version yet, use 093 */
   rc=characters_093(chars);
 }
 else {
   /* select version dependant function */
   rc=characters_093(chars);
 }
 return rc;
}

Подозрительное место. Наличие разных комментариев предполагает разный код, но здесь это не так.

Два похожих предупреждения:

• V523 The 'then' statement is equivalent to the 'else' statement. HydrogenXMLHandler.cpp 182
• V523 The 'then' statement is equivalent to the 'else' statement. HydrogenXMLHandler.cpp 358

Заключение

Пока это проект с самым низким качеством кода из всех в обзоре. Будем продолжать исследование дальше.

Другие обзоры музыкального софта:

• Часть 1. MuseScore
• Часть 2. Audacity
• Часть 3. Rosegarden
• Часть 4. Ardour
• Часть 5. Steinberg SDKs

Если вы знаете интересный софт для работы с музыкой и хотите увидеть его в обзоре, то присылайте названия мне на почту.

Попробовать анализатор PVS-Studio на своём проекте очень легко, достаточно перейти на страницу загрузки.