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

Введение

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

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

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

V557 ^[3] Array overrun is possible. The value of 'cidx' index could reach 4. staff.cpp 1029

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]);
  }
  ....
}

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

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

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

V557 ^[3] Array overrun is possible. The value of 'i' index could reach 59. inspectorAmbitus.cpp 70

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 ^[4] There are identical sub-expressions to the left and to the right of the '-' operator: i — i text.cpp 1429

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 ^[5] Visibility scope of the 'beam' pointer was exited without releasing the memory. A memory leak is possible. read114.cpp 2334

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 ^[5] The function was exited without releasing the 'voicePtr' pointer. A memory leak is possible. ove.cpp 3967

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:

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();
}

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

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

V614 ^[6] Uninitialized variable 'pageWidth' used. Consider checking the third actual argument of the 'doCredits' function. importmxmlpass1.cpp 944

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():

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 ^[7] Not all members of a class are initialized inside the constructor. Consider inspecting: _dclickValue1, _dclickValue2. aslider.cpp 30

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 ^[8] 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

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. Необходимо перепроверить код.

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

V517 ^[9] 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

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 ^[10] Expression 'middleMeasure != 0' is always false. ove.cpp 7852

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 ^[10] Expression 'error == 2' is always false. mididriver.cpp 126

#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 ^[11] A part of conditional expression is always false: strack > — 1. edit.cpp 3669

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 ^[12] Unreachable code detected. It is possible that an error is present. figuredbass.cpp 1377

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 ^[12] специализирована на поиске недостижимого кода, с ее помощью нашлось вот такое забавное место. И оно не одно в коде, есть ещё два:

• 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

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

V522 ^[13] Dereferencing of the null pointer 'customDrumset' might take place. instrument.cpp 328

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 ^[13] Dereferencing of the null pointer 'segment' might take place. measure.cpp 2220

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 ^[14] The 'slur' pointer was used after the memory was released. importgtp-gp6.cpp 2072

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 ^[15] Iterators for the 'oldList' container, used in the range-based for loop, become invalid upon the call of the 'erase' function. layout.cpp 1760

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 цикле. Такой код является ошибочным.

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

V765 ^[16] A compound assignment expression 'x += x + ...' is suspicious. Consider inspecting it for a possible error. tremolo.cpp 321

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

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

beamYOffset = beamYOffset + beamYOffset + beamHalfLineWidth;

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

V674 ^[17] 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

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 ^[18] The 'sample' pointer was utilized before it was verified against nullptr. Check lines: 926, 929. voice.cpp 926

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 перед делением? Тогда здесь имеет место серьёзная ошибка.

Разное

V523 ^[19] The 'then' statement is equivalent to the 'else' statement. pluginCreator.cpp 84

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 ^[20] It is odd that the body of 'downLine' function is fully equivalent to the body of 'upLine' function. rest.cpp 667

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 ^[21] An item with the same key '«mrcs»' has already been added. importgtp-gp6.cpp 100

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 ^[22] The 'ontime' variable is assigned but is not used until the end of the function. rendermidi.cpp 1176

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 ^[10] Expression 'runState == 0' is always false. pulseaudio.cpp 206

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 ^[10] перестанет выдавать ложное срабатывание, т.е. в ней сработает исключение на тему параллельного кода.

Заключение

В небольшом проекте нашлось много разных ошибок. Надеемся, авторы программы обратят внимание на мой обзор и проведут исправительные работы. Я проверю код ещё нескольких программ, которыми пользуюсь. Если вы знаете интересный софт для работы с музыкой и хотите увидеть его в обзоре, то присылайте названия мне на почту ^[23].

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

Автор: SvyatoslavMC

Источник ^[25]