# Review of Music Software Code Defects\. Part 1\. MuseScore

Programming is a creative activity, that's why there are a lot of talented people with a peculiar hobby among the developers\. Despite a popular belief, it is not always a programming \(well, or not just a programming :D\)\. On the basis of my interest to arrangement/record of music and professional activity, I decided to check out the code quality of popular music open source programs\. The first selected program for a review is a program for editing sheet music \- MuseScore\. Get some popcorn\.\.\.many serious bugs are waiting for us\!

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

## Introduction

[MuseScore](https://musescore.com/) is a computer program, the editor of sheet music for Windows, macOS X and Linux operating systems\. MuseScore allows to quickly enter notes both with the computer keyboard and with an external MIDI keyboard\. You can import and export data in formats such as MIDI, MusicXML, LilyPond, as well as importing files in formats MusE, Capella and Band\-in\-a\-Box\. In addition, the program can export musical scores in PDF, SVG, and PNG, either in LilyPond documents for a further accurate modification\.

[PVS\-Studio](https://pvs-studio.com/en/pvs-studio/) is a tool for bug detection in the source code of programs, written in C, C\+\+, and C\#\.  It works in Windows and Linux environment\. 

## Problems with array indexing

[V557](https://pvs-studio.com/en/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]);
  }
  ....
}
```

The author of this code fragment made a serious error by comparing the index with the maximum size of the array\. For this reason, an array bounds exceeded of four arrays became possible\.

Corrected condition of the index check:

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

[V557](https://pvs-studio.com/en/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
  ....
}
```

Instead of trying to count the number of array elements that are in the loop, here a constant was used, which is almost four times greater that number\. In the cycle a guaranteed array overrun occurs\.

[V501](https://pvs-studio.com/en/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);
  }
  ....
}
```

The _\[i \- i\]_ index value, in this case, will always equal to zero\. Perhaps there is an error and programmers wanted, for example, to refer to the previous item of the array\.

## Memory leak

Using static analysis, you can also find memory leaks and PVS\-Studio does it\. Yes, static analyzers are weaker than dynamic in terms of finding memory leaks, but still they can find a lot of interesting things\.

In an unfamiliar project it is difficult for me to verify the validity of all warnings, but in some places I was able to ensure that there were actual errors\.

[V773](https://pvs-studio.com/en/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);       // <=
  }
  ....
}
```

In a large cascade of conditions memory allocation is performed\. In each block an object is created and a pointer to it is stored\. In the given code fragment, a saving of the pointer was commented out by adding the error in code, leading to a memory leak\.

[V773](https://pvs-studio.com/en/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
  }
  ....
}
```

This is quite a large code fragment, but there is an error that is easy to be seen\. Each marked _return_ operator causes a loss of the_ voicePtr_ pointer\. If the program is executed before the line of code with the comment "SAVE 2", then a pointer is stored in the _Track_ class\. In the destructor of this class the pointers will be freed\. In other cases, it will be a memory leak\. This is how the implementation of the _Track_ class will be performed:

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

Other similar warnings are better to be reviewed by the developers of the project\.

## Initialization errors

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

[V614](https://pvs-studio.com/en/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
    ....
  }
  ....
}
```

This code allows using the uninitialized variables _pageWidth _and _pageHeight _in the function _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;
  ....
}
```

The use of uninitialized variables leads to undefined behavior, that can create visibility that the program works correctly for a long time\.

[V730](https://pvs-studio.com/en/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;  }
....
```

The use of an uninitialized class field can lead to undefined behavior\. In this class most of the fields are initialized in the constructor and have methods to access them\. But _\_dclickValue1 _and _dclickValue2_ variables stay uninitialized, although they have methods for reading and writing\. If the first method is called to read, it will return the undefined value\. In the project code there was found about a hundred of such places, and they deserve developers reviewing\.

## Legacy errors

[V762](https://pvs-studio.com/en/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);
  ....
}
```

The analyzer found three different ways to override and overload the function _adjustCanvasPosition\(\)_ in the base class _MuseScoreView_\. It is needed to verify the code\.

## Unreachable code

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

[V517](https://pvs-studio.com/en/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
    ....
  }
  ....
}
```

In a very large conditions cascade there are two similar checks\. In case of such an error either two conditions are not executed or just the first condition is executed\. Therefore, the second condition is never executed and the code remains unreachable\.

Two more similar fragments:

* 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

Let's consider the following error:

[V547](https://pvs-studio.com/en/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());
    }
  }
  ....
}
```

Another code fragment that will never get executed\. The reason is a condition which is always false\. In a condition a pointer is verified against null that originally have been initialized by null\.  After careful consideration you see a typo: **middleMeasure **and** middleUnit **variables are messed up\.  Pay attention to the function _getMiddleUnit\(\)_\. As you can from a title and the last argument \(passed by link\), the **middleUnit** variable is being modified, it had to be verified in the condition\.



[V547](https://pvs-studio.com/en/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;
  }
  ....
}
```

It is obvious that after the first check, _error_ variable will always be less than zero\. Due to a further variable comparing with _2_, debugging information is never displayed\.

[V560](https://pvs-studio.com/en/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;
  }
  ....
}
```

Another case with an error in a conditional expression\. Code from _else_ is always executed\.

[V779](https://pvs-studio.com/en/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.com/en/docs/warnings/v779/) diagnostic is specialized on finding unreachable code, so this interesting fragment of code was found by using it\. It is not the one fragment of code, there are two more\.

* 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

## Invalid pointers/iterators

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

[V522](https://pvs-studio.com/en/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);
  }
  ....
}
```

An error in the condition is missed here\. Most likely, the author wanted to differently verify a _customDrumset_ pointer before dereferencing, but wrote code with a typo\.

[V522](https://pvs-studio.com/en/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
    ....
  }
  ....
}
```

This is not the first large cascade of conditions in this project where programmers make mistakes\. It is worth thinking about\! Here the _segment_ pointer was initially equal to null and before using it is initialized under different conditions\. In one branch a programmer forgot to do it\.

Two more dangerous places:

* 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.com/en/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 _pointer is used after freeing the memory using a _delete_ operator\.  Probably, the lines were messed up\.

[V789](https://pvs-studio.com/en/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;
    }
  }
  ....
}
```

The analyzer has detected the simultaneous reading and modification of the _oldList_ container in the range\-based for loop\. This code is erroneous\.

## Errors with arithmetic

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

[V765](https://pvs-studio.com/en/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;
  }
  ....
}
```

Here is the code the analyzer found\. The specified expression is equal to this:

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

The variable _beamYOffset_ is folded twice\. Perhaps this is a mistake\.

[V674](https://pvs-studio.com/en/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 _variable has an integer_ int_ type\. Comparison with numbers _2\.5_ and _\-2\.5_ looks very strange\. 

[V595](https://pvs-studio.com/en/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;
  ....
}
```

The analyzer complains about a dereferencing of the unchecked _sample_ pointer when in code below there is a check\. But what if a programmer has not planned to check_ sample_ in this function, but wanted to verify _sample\-\>samplerate_ variable against zero before division? If so, a serious error takes place in this fragment\.

## Other issues

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

[V523](https://pvs-studio.com/en/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);
  }
  ....
}
```

The analyzer has detected the same code execution under different conditions\. Here it is necessary to correct the error, either shorten the code twice, dropping the condition\.

[V524](https://pvs-studio.com/en/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\(\)_ and _downLine\(\)_ functions have the opposite meaning to their names, but they are implemented in the same way\. This suspicious fragment is worth checking out\.

[V766](https://pvs-studio.com/en/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"},                   // <= using of Copy-Paste
  ....
};
```

It looks like the author of this code fragment was in a hurry, so he created pairs with identical keys but different values\.

[V1001](https://pvs-studio.com/en/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_ variable is modified in code, but at the same time it is not used when exiting the function\. Perhaps, there is an error here\.

[V547](https://pvs-studio.com/en/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;
}
```

The analyzer has detected an always false condition, but the _stop\(\) _function is called in parallel code and here should be no triggering\. The reason of the warning is that the author of the code used a simple variable of the _int_ type to synchronize, which is a field of the class\. This leads to errors in synchronization\. After fixing the code diagnostic [V547](https://pvs-studio.com/en/docs/warnings/v547/) will no longer issue the false positive, i\.e\., an exception on the theme of parallel code will trigger in it\.

## Conclusion

It turns out, that a small project has many different errors\. We hope the authors of the program will pay attention to my review and carry out some corrective work\. I'll check the code of several programs that I use\. If you know an interesting soft to work with music and want to see it in review, then send me the names of the programs by [mail](mailto:razmyslov@viva64.com)\.

Other music software reviews:

* [Part 1\. MuseScore](https://pvs-studio.com/en/blog/posts/cpp/0530/)
* [Part 2\. Audacity](https://pvs-studio.com/en/blog/posts/cpp/0532/)
* [Part 3\. Rosegarden](https://pvs-studio.com/en/blog/posts/cpp/0536/)
* [Part 4\. Ardour](https://pvs-studio.com/en/blog/posts/cpp/0540/)
* [Part 5\. Steinberg SDKs](https://pvs-studio.com/en/blog/posts/cpp/0541/)

It is very easy to try PVS\-Studio analyzer on your project, just go to the [download](https://pvs-studio.com/en/pvs-studio/download/) page\.