﻿# Обзор дефектов кода музыкального софта\. Часть 1\. MuseScore

Программирование \- занятие творческое, поэтому среди разработчиков встречается много талантливых людей, имеющих своеобразное хобби\. Вопреки распространённому мнению, это не всегда программирование \(ну или не только оно :D\)\. На основе своего увлечения записью/обработкой музыки и профессиональной деятельности, я решил проверить качество кода популярных музыкальных программ с открытым исходным кодом\. Первой для обзора выбрана программа для редактирования нот \- MuseScore\. Запасайтесь попкорном\.\.\. серьёзных багов будет много\!

![0530_MusicSoftwareDefects_01_MuseScore_ru/image1.png](https://import.viva64.com/docx/blog/0530_MusicSoftwareDefects_01_MuseScore_ru/image1.png)

## Введение

[MuseScore](https://musescore.com/) — компьютерная программа, редактор нотных партитур для операционных систем Windows, macOS X и Linux\. MuseScore позволяет быстро вводить ноты как с клавиатуры компьютера, так и с внешней MIDI\-клавиатуры\. Поддерживается импорт и экспорт данных в форматах MIDI, MusicXML, LilyPond, а также импорт файлов в форматах MusE, Capella и Band\-in\-a\-Box\. Кроме того, программа может экспортировать партитуры в файлы PDF, SVG и PNG, либо в документы LilyPond для дальнейшей точной доработки партитуры\.

[PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) \- это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C\+\+ и C\#\. Работает в среде Windows и Linux\. 

## Проблемы с индексацией массивов

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'cidx' index could reach 4\. staff\.cpp 1029

```cpp
ClefTypeList clefTypes[MAX_STAVES];
int staffLines[MAX_STAVES];
BracketType bracket[MAX_STAVES];
int bracketSpan[MAX_STAVES];
int barlineSpan[MAX_STAVES];
bool smallStaff[MAX_STAVES];

void Staff::init(...., const StaffType* staffType, int cidx)
{
  if (cidx > MAX_STAVES) { // <=
    setSmall(0, false);
  }
  else {
    setSmall(0,       t->smallStaff[cidx]);
    setBracketType(0, t->bracket[cidx]);
    setBracketSpan(0, t->bracketSpan[cidx]);
    setBarLineSpan(t->barlineSpan[cidx]);
  }
  ....
}
```

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

Исправленное условие проверки индекса:

```cpp
if (cidx >= MAX_STAVES) {
  setSmall(0, false);
}
```

[V557](https://pvs-studio.ru/ru/docs/warnings/v557/) Array overrun is possible\. The value of 'i' index could reach 59\. inspectorAmbitus\.cpp 70

```cpp
class NoteHead : public Symbol {
  ....
public:
  enum class Group : signed char {
    HEAD_NORMAL = 0,
    HEAD_CROSS,
    HEAD_PLUS,
    ....
    HEAD_GROUPS,              // <= 59
    HEAD_INVALID = -1
    };
  ....
}

InspectorAmbitus::InspectorAmbitus(QWidget* parent)
   : InspectorElementBase(parent)
{
  r.setupUi(addWidget());
  s.setupUi(addWidget());

  static const NoteHead::Group heads[] = {
    NoteHead::Group::HEAD_NORMAL,
    NoteHead::Group::HEAD_CROSS,
    NoteHead::Group::HEAD_DIAMOND,
    NoteHead::Group::HEAD_TRIANGLE_DOWN,
    NoteHead::Group::HEAD_SLASH,
    NoteHead::Group::HEAD_XCIRCLE,
    NoteHead::Group::HEAD_DO,
    NoteHead::Group::HEAD_RE,
    NoteHead::Group::HEAD_MI,
    NoteHead::Group::HEAD_FA,
    NoteHead::Group::HEAD_SOL,
    NoteHead::Group::HEAD_LA,
    NoteHead::Group::HEAD_TI,
    NoteHead::Group::HEAD_BREVIS_ALT
    };
  ....
  for (int i = 0; i < int(NoteHead::Group::HEAD_GROUPS); ++i)
    r.noteHeadGroup->setItemData(i, int(heads[i]));//out of bound
  ....
}
```

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

[V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\-' operator: i \- i text\.cpp 1429

```cpp
void Text::layout1()
{
  ....
  for (int i = 0; i < rows(); ++i) {
    TextBlock* t = &_layout[i];
    t->layout(this);
    const QRectF* r = &t->boundingRect();

    if (r->height() == 0)
      r = &_layout[i-i].boundingRect(); // <=
    y += t->lineSpacing();
    t->setY(y);
    bb |= r->translated(0.0, y);
  }
  ....
}
```

Значение индекса _\[i \- i\]_, в данном случае, всегда будет равно нулю\. Возможно, тут ошибка и хотели, к примеру, обращаться к предыдущему элементу массива\.

## Утечки памяти

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

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

[V773](https://pvs-studio.ru/ru/docs/warnings/v773/) Visibility scope of the 'beam' pointer was exited without releasing the memory\. A memory leak is possible\. read114\.cpp 2334

```cpp
Score::FileError MasterScore::read114(XmlReader& e)
{
  ....
  else if (tag == "Excerpt") {
    if (MScore::noExcerpts)
          e.skipCurrentElement();
    else {
      Excerpt* ex = new Excerpt(this);
      ex->read(e);
      _excerpts.append(ex);
    }
  }
  else if (tag == "Beam") {
    Beam* beam = new Beam(this);
    beam->read(e);
    beam->setParent(0);
    // _beams.append(beam);       // <=
  }
  ....
}
```

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

[V773](https://pvs-studio.ru/ru/docs/warnings/v773/) The function was exited without releasing the 'voicePtr' pointer\. A memory leak is possible\. ove\.cpp 3967

```cpp
bool TrackParse::parse() {
  ....
  Track* oveTrack = new Track();
  ....
  QList<Voice*> voices;
  for( i=0; i<8; ++i ) {
    Voice* voicePtr = new Voice();

    if( !jump(5) ) { return false; }                    // <=

    // channel
    if( !readBuffer(placeHolder, 1) ) { return false; } // <=
    voicePtr->setChannel(placeHolder.toUnsignedInt());

    // volume
    if( !readBuffer(placeHolder, 1) ) { return false; } // <=
    voicePtr->setVolume(placeHolder.toInt());

    // pitch shift
    if( !readBuffer(placeHolder, 1) ) { return false; } // <=
    voicePtr->setPitchShift(placeHolder.toInt());

    // pan
    if( !readBuffer(placeHolder, 1) ) { return false; } // <=
    voicePtr->setPan(placeHolder.toInt());

    if( !jump(6) ) { return false; }                    // <=

    // patch
    if( !readBuffer(placeHolder, 1) ) { return false; } // <=
    voicePtr->setPatch(placeHolder.toInt());

    voices.push_back(voicePtr);                       //SAVE 1
  }

  // stem type
  for( i=0; i<8; ++i ) {
    if( !readBuffer(placeHolder, 1) ) { return false; } // <=
    voices[i]->setStemType(placeHolder.toUnsignedInt());

    oveTrack->addVoice(voices[i]);                    //SAVE 2
  }
  ....
}
```

Достаточно большой фрагмент, но в наличии ошибки легко убедиться\. Каждый отмеченный оператор _return_ приводит к потере указателя _voicePtr_\. Если всё же программа выполняется до строки кода с комментарием "SAVE 2", то указатель сохраняется в класс _Track_\. В деструкторе этого класса и будет выполнено освобождение указателей\. В остальных случаях будет утечка памяти\. Вот как выглядит реализация класса _Track_:

```cpp
class Track{
  ....
  QList<Voice*> voices_;
  ....
}

void Track::addVoice(Voice* voice) {
  voices_.push_back(voice);
}

Track::~Track() {
  clear();
}

void Track::clear(void) {
  ....
  for(int i=0; i<voices_.size(); ++i){
    delete voices_[i];
  }
  voices_.clear();
}
```

Другие аналогичные предупреждения анализатора лучше посмотреть разработчикам проекта\.

## Ошибки инициализации

![0530_MusicSoftwareDefects_01_MuseScore_ru/image2.png](https://import.viva64.com/docx/blog/0530_MusicSoftwareDefects_01_MuseScore_ru/image2.png)

[V614](https://pvs-studio.ru/ru/docs/warnings/v614/) Uninitialized variable 'pageWidth' used\. Consider checking the third actual argument of the 'doCredits' function\. importmxmlpass1\.cpp 944

```cpp
void MusicXMLParserPass1::scorePartwise()
{
  ....
  int pageWidth;
  int pageHeight;

  while (_e.readNextStartElement()) {
    if (_e.name() == "part")
      part();
    else if (_e.name() == "part-list") {
      doCredits(_score, credits, pageWidth, pageHeight);// <= USE
      partList(partGroupList);
    }
    ....
    else if (_e.name() == "defaults")
      defaults(pageWidth, pageHeight);                 // <= INIT
    ....
  }
  ....
}
```

Такой код допускает использование неинициализированных переменных _pageWidth_ и _pageHeight_ в функции _doCredits\(\)_:

```cpp
static
void doCredits(Score* score,const CreditWordsList& credits,
               const int pageWidth, const int pageHeight)
{
  ....
  const int pw1 = pageWidth / 3;
  const int pw2 = pageWidth * 2 / 3;
  const int ph2 = pageHeight / 2;
  ....
}
```

Использование неинициализированных переменных приводит к неопределённому поведению, что долгое время может создавать видимость правильной работы программы\.

[V730](https://pvs-studio.ru/ru/docs/warnings/v730/) Not all members of a class are initialized inside the constructor\. Consider inspecting: \_dclickValue1, \_dclickValue2\. aslider\.cpp 30

```cpp
AbstractSlider::AbstractSlider(QWidget* parent)
   : QWidget(parent), _scaleColor(Qt::darkGray),
     _scaleValueColor(QColor("#2456aa"))
{
  _id         = 0;
  _value      = 0.5;
  _minValue   = 0.0;
  _maxValue   = 1.0;
  _lineStep   = 0.1;
  _pageStep   = 0.2;
  _center     = false;
  _invert     = false;
  _scaleWidth = 4;
  _log        = false;
  _useActualValue = false;
  setFocusPolicy(Qt::StrongFocus);
}

double lineStep() const    { return _lineStep; }
void setLineStep(double v) { _lineStep = v;    }
double pageStep() const    { return _pageStep; }
void setPageStep(double f) { _pageStep = f;    }
double dclickValue1() const      { return _dclickValue1; }
double dclickValue2() const      { return _dclickValue2; }
void setDclickValue1(double val) { _dclickValue1 = val;  }
void setDclickValue2(double val) { _dclickValue2 = val;  }
....
```

К неопределённому поведению может приводить использование неинициализированного поля класса\. В этом классе большинство полей инициализируются в конструкторе и имеют методы для доступа к ним\. А вот переменные _\_dclickValue1_ и\__dclickValue2_ остаются не инициализированными, хотя имеют методы для чтения и записи\. Если первым будет вызван метод для чтения, то он вернёт неопределённое значение\. В коде проекта найдено около ста таких мест, и они заслуживают изучения разработчиками\.

## Ошибки наследования

[V762](https://pvs-studio.ru/ru/docs/warnings/v762/) It is possible a virtual function was overridden incorrectly\. See third argument of function 'adjustCanvasPosition' in derived class 'PianorollEditor' and base class 'MuseScoreView'\. pianoroll\.h 92

```cpp
class MuseScoreView {
  ....
  virtual void adjustCanvasPosition(const Element*,
    bool /*playBack*/, int /*staffIdx*/ = 0) {};
  ....
}

class PianorollEditor : public QMainWindow, public MuseScoreView{
  ....
  virtual void adjustCanvasPosition(const Element*, bool);
  ....
}

class ScoreView : public QWidget, public MuseScoreView {
  ....
  virtual void adjustCanvasPosition(const Element* el,
    bool playBack, int staff = -1) override;
  ....
}

class ExampleView : public QFrame, public MuseScoreView {
  ....
  virtual void adjustCanvasPosition(const Element* el,
    bool playBack);
  ....
}
```

Анализатор нашёл целых три разных способа переопределить и перегрузить функцию _adjustCanvasPosition\(\)_ у базового класса _MuseScoreView_\. Необходимо перепроверить код\.

## Недостижимый код

![0530_MusicSoftwareDefects_01_MuseScore_ru/image3.png](https://import.viva64.com/docx/blog/0530_MusicSoftwareDefects_01_MuseScore_ru/image3.png)

[V517](https://pvs-studio.ru/ru/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 1740, 1811\. scoreview\.cpp 1740

```cpp
static void readNote(Note* note, XmlReader& e)
{
  ....
  while (e.readNextStartElement()) {
    const QStringRef& tag(e.name());
    if (tag == "Accidental") {
      ....
    }
    ....
    else if (tag == "offTimeType") {        // <= line 651
      if (e.readElementText() == "offset")
        note->setOffTimeType(2);
      else
        note->setOffTimeType(1);
    }
    ....
    else if (tag == "offTimeType")          // <= line 728
      e.skipCurrentElement();               // <= Dead code
    ....
  }
  ....
}
```

В очень большом каскаде условий присутствуют две одинаковые проверки\. При такой ошибке либо не выполняются оба условия, либо выполнится только первое\. Таким образом, второе условие никогда не выполняется и код остаётся недостижим\.

Ещё два похожих места в коде:

* V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 645, 726\. read114\.cpp 645
* V517 The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 1740, 1811\. scoreview\.cpp 1740

Рассмотрим следующую ошибку\.

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'middleMeasure \!\= 0' is always false\. ove\.cpp 7852

```cpp
bool
getMiddleUnit(...., Measure* middleMeasure, int& middleUnit) {
  ....
}

void OveOrganizer::organizeWedge(....) {
  ....
  Measure* middleMeasure = NULL;
  int middleUnit = 0;

  getMiddleUnit(
    ove_, part, track,
    measure, ove_->getMeasure(bar2Index),
    wedge->start()->getOffset(), wedge->stop()->getOffset(),
    middleMeasure, middleUnit);

  if( middleMeasure != 0 ) {                            // <=
    WedgeEndPoint* midStopPoint = new WedgeEndPoint();
    measureData->addMusicData(midStopPoint);

    midStopPoint->setTick(wedge->getTick());
    midStopPoint->start()->setOffset(middleUnit);
    midStopPoint->setWedgeStart(false);
    midStopPoint->setWedgeType(WedgeType::Cres_Line);
    midStopPoint->setHeight(wedge->getHeight());

    WedgeEndPoint* midStartPoint = new WedgeEndPoint();
    measureData->addMusicData(midStartPoint);

    midStartPoint->setTick(wedge->getTick());
    midStartPoint->start()->setOffset(middleUnit);
    midStartPoint->setWedgeStart(true);
    midStartPoint->setWedgeType(WedgeType::Decresc_Line);
    midStartPoint->setHeight(wedge->getHeight());
    }
  }
  ....
}
```

Ещё очень большой фрагмент кода, который никогда не выполняется\. Причиной является условие, которое всегда ложно\. В условии сравнивается с нулём указатель, который изначально инициализирован нулём\. При внимательном рассмотрении можно увидеть опечатку: перепутаны переменные **middleMeasure** и **middleUnit**\. Обратите внимание на функцию _getMiddleUnit\(\)_\. По названию и последнему аргументу \(передан по ссылке\) видно, что модифицируется переменная **middleUnit**, которую и надо было проверять в условии\.



[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'error \=\= 2' is always false\. mididriver\.cpp 126

```cpp
#define ENOENT 2

bool AlsaMidiDriver::init()
{
  int error = snd_seq_open(&alsaSeq, "hw", ....);
  if (error < 0) {
    if (error == ENOENT)
      qDebug("open ALSA sequencer failed: %s",
        snd_strerror(error));
    return false;
  }
  ....
}
```

Очевидно, что после первой проверки переменная _error_ всегда будет меньше нуля\. Из\-за дальнейшего сравнения переменной с числом _2_ никогда не выводится отладочная информация\.

[V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always false: strack \> \- 1\. edit\.cpp 3669

```cpp
void Score::undoAddElement(Element* element)
{
  QList<Staff* > staffList;
  Staff* ostaff = element->staff();
  int strack = -1;
  if (ostaff) {
    if (ostaff->score()->excerpt() && strack > -1)
     strack = ostaff->score()->excerpt()->tracks().key(...);
    else
     strack = ostaff->idx() * VOICES + element->track() % VOICES;
  }
  ....
}
```

Ещё один случай с ошибкой в условном выражении\. Всегда выполняется код из _else_\.

[V779](https://pvs-studio.ru/ru/docs/warnings/v779/) Unreachable code detected\. It is possible that an error is present\. figuredbass\.cpp 1377

```cpp
bool FiguredBass::setProperty(P_ID propertyId, const QVariant& v)
{
  score()->addRefresh(canvasBoundingRect());
  switch(propertyId) {
    default:
      return Text::setProperty(propertyId, v);
    }
  score()->setLayoutAll();
  return true;
}
```

Диагностика [V779](https://pvs-studio.ru/ru/docs/warnings/v779/) специализирована на поиске недостижимого кода, с ее помощью нашлось вот такое забавное место\. И оно не одно в коде, есть ещё два:

* V779 Unreachable code detected\. It is possible that an error is present\. fingering\.cpp 165
* V779 Unreachable code detected\. It is possible that an error is present\. chordrest\.cpp 1127

## Невалидные указатели/итераторы

![0530_MusicSoftwareDefects_01_MuseScore_ru/image4.png](https://import.viva64.com/docx/blog/0530_MusicSoftwareDefects_01_MuseScore_ru/image4.png)

[V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'customDrumset' might take place\. instrument\.cpp 328

```cpp
bool Instrument::readProperties(XmlReader& e, Part* part,
  bool* customDrumset)
{
  ....
  else if (tag == "Drum") {
    // if we see on of this tags, a custom drumset will
    // be created
    if (!_drumset)
      _drumset = new Drumset(*smDrumset);
    if (!customDrumset) {                        // <=
      const_cast<Drumset*>(_drumset)->clear();
      *customDrumset = true;                     // <=
    }
    const_cast<Drumset*>(_drumset)->load(e);
  }
  ....
}
```

Тут допущена ошибка в условии\. Скорее всего, автор хотел иначе проверить указатель _customDrumset_ перед разыменованием, но сделал опечатку\.

[V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'segment' might take place\. measure\.cpp 2220

```cpp
void Measure::read(XmlReader& e, int staffIdx)
{
  Segment* segment = 0;
  ....
  while (e.readNextStartElement()) {
    const QStringRef& tag(e.name());

    if (tag == "move")
      e.initTick(e.readFraction().ticks() + tick());
    ....
    else if (tag == "sysInitBarLineType") {
      const QString& val(e.readElementText());
      BarLine* barLine = new BarLine(score());
      barLine->setTrack(e.track());
      barLine->setBarLineType(val);
      segment = getSegmentR(SegmentType::BeginBarLine, 0); //!!!
      segment->add(barLine);                           // <= OK
    }
    ....
    else if (tag == "Segment")
      segment->read(e);                                // <= ERR
    ....
  }
  ....
}
```

Это уже не первый большой каскад условий в этом проекте, где допускают ошибки\. Стоит задуматься\! Здесь указатель _segment_ изначально равен нулю, а перед использованием инициализируется при разных условиях\. В одной ветви это сделать забыли\.

Ещё два опасных места:

* V522 Dereferencing of the null pointer 'segment' might take place\. read114\.cpp 1551
* V522 Dereferencing of the null pointer 'segment' might take place\. read206\.cpp 1879

[V774](https://pvs-studio.ru/ru/docs/warnings/v774/) The 'slur' pointer was used after the memory was released\. importgtp\-gp6\.cpp 2072

```cpp
void GuitarPro6::readGpif(QByteArray* data)
{
  if (c) {
    slur->setTick2(c->tick());
    score->addElement(slur);
    legatos[slur->track()] = 0;
  }
  else {
    delete slur;
    legatos[slur->track()] = 0;
  }
}
```

Указатель _slur_ используется уже после освобождения памяти с помощью оператора _delete_\. Вероятно, тут перепутали строки местами\.

[V789](https://pvs-studio.ru/ru/docs/warnings/v789/) Iterators for the 'oldList' container, used in the range\-based for loop, become invalid upon the call of the 'erase' function\. layout\.cpp 1760

```cpp
void Score::createMMRest(....)
{
  ElementList oldList = mmr->takeElements();

  for (Element* ee : oldList) {    // <=
    if (ee->type() == e->type()) {
      mmr->add(ee);
      auto i = std::find(oldList.begin(), oldList.end(), ee);
      if (i != oldList.end())
        oldList.erase(i);          // <=
      found = true;
      break;
    }
  }
  ....
}
```

Анализатор обнаружил одновременное чтение и модификацию контейнера _oldList_ в range\-based for цикле\. Такой код является ошибочным\.

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

![0530_MusicSoftwareDefects_01_MuseScore_ru/image5.png](https://import.viva64.com/docx/blog/0530_MusicSoftwareDefects_01_MuseScore_ru/image5.png)

[V765](https://pvs-studio.ru/ru/docs/warnings/v765/) A compound assignment expression 'x \+\= x \+ \.\.\.' is suspicious\. Consider inspecting it for a possible error\. tremolo\.cpp 321

```cpp
void Tremolo::layout()
{
  ....
  if (_chord1->up() != _chord2->up()) {
    beamYOffset += beamYOffset + beamHalfLineWidth; // <=
  }
  else if (!_chord1->up() && !_chord2->up()) {
    beamYOffset = -beamYOffset;
  }
  ....
}
```

Вот такой код нашёл анализатор\. Указанное выражение равносильно такому:

```cpp
beamYOffset = beamYOffset + beamYOffset + beamHalfLineWidth;
```

Переменная _beamYOffset_ складывается два раза\. Возможно, это является ошибкой\.

[V674](https://pvs-studio.ru/ru/docs/warnings/v674/) The '\-2\.5' literal of the 'double' type is compared to a value of the 'int' type\. Consider inspecting the 'alter < \- 2\.5' expression\. importmxmlpass2\.cpp 5253

```cpp
void MusicXMLParserPass2::pitch(int& step, int& alter ....)
{
  ....
  alter = MxmlSupport::stringToInt(strAlter, &ok);
  if (!ok || alter < -2.5 || alter > 2.5) {
    logError(QString("invalid alter '%1'").arg(strAlter));
    ....
    alter = 0;
  }
  ....
}
```

Переменная _alter_ имеет целочисленный тип _int_\. И сравнение с числами _2\.5_ и _\-2\.5_ выглядит очень странным\. 

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'sample' pointer was utilized before it was verified against nullptr\. Check lines: 926, 929\. voice\.cpp 926

```cpp
void Voice::update_param(int _gen)
{
 ....
 if (gen[GEN_OVERRIDEROOTKEY].val > -1) {
  root_pitch = gen[GEN_OVERRIDEROOTKEY].val * 100.0f - ....
 }
 else {
  root_pitch = sample->origpitch * 100.0f - sample->pitchadj;
 }
 root_pitch = _fluid->ct2hz(root_pitch);
 if (sample != 0)
  root_pitch *= (float) _fluid->sample_rate / sample->samplerate;
 break;
  ....
}
```

Анализатор ругается на разыменование непроверенного указателя _sample_, когда ниже по коду проверка присутствует\. Но что если указатель _sample_ не планировали проверять в этой функции, а с нулём хотели сравнить переменную _sample\-\>samplerate_ перед делением? Тогда здесь имеет место серьёзная ошибка\.

## Разное

![0530_MusicSoftwareDefects_01_MuseScore_ru/image6.png](https://import.viva64.com/docx/blog/0530_MusicSoftwareDefects_01_MuseScore_ru/image6.png)

[V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. pluginCreator\.cpp 84

```cpp
PluginCreator::PluginCreator(QWidget* parent)
   : QMainWindow(parent)
{
  ....
  if (qApp->layoutDirection() == Qt::LayoutDirection::....) {
    editTools->addAction(actionUndo);
    editTools->addAction(actionRedo);
  }
  else {
    editTools->addAction(actionUndo);
    editTools->addAction(actionRedo);
  }
  ....
}
```

Анализатор обнаружил выполнение одинакового кода при разных условиях\. Тут следует исправить ошибку, либо сократить код вдвое, убрав условие\.

[V524](https://pvs-studio.ru/ru/docs/warnings/v524/) It is odd that the body of 'downLine' function is fully equivalent to the body of 'upLine' function\. rest\.cpp 667

```cpp
int Rest::upLine() const
{
  qreal _spatium = spatium();
  return lrint((pos().y() + bbox().top() +
    _spatium) * 2 / _spatium);
}

int Rest::downLine() const
{
  qreal _spatium = spatium();
  return lrint((pos().y() + bbox().top() +
    _spatium) * 2 / _spatium);
}
```

Функции _upLine\(\)_ и _downLine\(\)_ имеют противоположные по смыслу названия, но при этом реализованы одинаково\. Стоит проверить это подозрительное место\.

[V766](https://pvs-studio.ru/ru/docs/warnings/v766/) An item with the same key '"mrcs"' has already been added\. importgtp\-gp6\.cpp 100

```cpp
const static std::map<QString, QString> instrumentMapping = {
  ....
  {"e-piano-gs", "electric-piano"},
  {"e-piano-ss", "electric-piano"},
  {"hrpch-gs", "harpsichord"},
  {"hrpch-ss", "harpsichord"},
  {"mrcs", "maracas"},                // <=
  {"mrcs", "oboe"},                   // <=
  {"mrcs", "oboe"},                   // <= явный Copy-Paste
  ....
};
```

Похоже автор этого фрагмента кода торопился и насоздавал пар с одинаковыми ключами, но разными значениями\.

[V1001](https://pvs-studio.ru/ru/docs/warnings/v1001/) The 'ontime' variable is assigned but is not used until the end of the function\. rendermidi\.cpp 1176

```cpp
bool renderNoteArticulation(....)
{
  int ontime    = 0;
  ....
  // render the suffix
  for (int j = 0; j < s; j++)
    ontime = makeEvent(suffix[j], ontime, tieForward(j,suffix));
  // render graceNotesAfter
  ontime = graceExtend(note->pitch(), ...., ontime);
  return true;
}
```

В коде модифицируется переменная _ontime_, но при этом не используется при выходе из функции\. Возможно, тут допущена ошибка\.

[V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'runState \=\= 0' is always false\. pulseaudio\.cpp 206

```cpp
class PulseAudio : public Driver {
  Transport state;
  int runState;           // <=
  ....
}

bool PulseAudio::stop()
{
  if (runState == 2) {
    runState = 1;
    int i = 0;
    for (;i < 4; ++i) {
      if (runState == 0)  // <=
        break;
      sleep(1);
    }
    pthread_cancel(thread);
    pthread_join(thread, 0);
    }
  return true;
}
```

Анализатор обнаружил всегда ложное условие, но функция _stop\(\)_ вызывается в параллельном коде и срабатывания быть не должно\. Причина появления предупреждения заключается в том, что автор кода воспользовался для синхронизации простой переменой типа _int_, являющейся полем класса\. А это приводит к ошибкам синхронизации\. После исправления кода, диагностика [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) перестанет выдавать ложное срабатывание, т\.е\. в ней сработает исключение на тему параллельного кода\.

## Заключение

В небольшом проекте нашлось много разных ошибок\. Надеемся, авторы программы обратят внимание на мой обзор и проведут исправительные работы\. Я проверю код ещё нескольких программ, которыми пользуюсь\. Если вы знаете интересный софт для работы с музыкой и хотите увидеть его в обзоре, то присылайте названия мне на [почту](mailto:razmyslov@viva64.com)\.

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

* [Часть 1\. MuseScore](https://pvs-studio.ru/ru/blog/posts/cpp/0530/)
* [Часть 2\. Audacity](https://pvs-studio.ru/ru/blog/posts/cpp/0532/)
* [Часть 3\. Rosegarden](https://pvs-studio.ru/ru/blog/posts/cpp/0536/)
* [Часть 4\. Ardour](https://pvs-studio.ru/ru/blog/posts/cpp/0540/)
* [Часть 5\. Steinberg SDKs](https://pvs-studio.ru/ru/blog/posts/cpp/0541/)

Попробовать анализатор PVS\-Studio на своём проекте очень легко, достаточно перейти на страницу [загрузки](https://pvs-studio.ru/ru/pvs-studio/download/)\.