Review of Music Software Code Defects Part 1. MuseScore

Introduction

Problems with array indexing

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

Memory leak

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

Initialization errors

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

Legacy errors

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

Unreachable code

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
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());
}
}
....
}
#define ENOENT 2bool 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;
}
....
}
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;
}
....
}
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 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

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

Errors with arithmetic

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

Other issues

PluginCreator::PluginCreator(QWidget* parent)
: QMainWindow(parent)
{
....
if (qApp->layoutDirection() == Qt::LayoutDirection::....) {
editTools->addAction(actionUndo);
editTools->addAction(actionRedo);
}
else {
editTools->addAction(actionUndo);
editTools->addAction(actionRedo);
}
....
}
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);
}
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
....
};
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;
}
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;
}

Conclusion

--

--

--

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

Love podcasts or audiobooks? Learn on the go with our new app.

Recommended from Medium

How to migrate Applications into Cloud?

How to Validate a Last Name in Python

5 must-read books if you work in IT

Outsystems | How Low Code Development changed me

Omnisphere 2 Buy

Introduce Product 1: MassBit Route

Then the ball went farther out into the ocean, and I felt shitty

Configure a running ActionHero with Docker as simple as possible

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store
Unicorn Developer

Unicorn Developer

The developer, the debugger, the unicorn. I know all about static analysis and how to find bugs and errors in C, C++, C#, and Java source code.

More from Medium

A better alternative than Signals in Godot with C# + Dependency Injection

The S.O.L.I.D Principles

MuditaOS: Will your alarm clock go off? Part I

Compilation in C