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

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

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

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

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

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

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

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

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

V501 There are identical sub-expressions to the left and to the right of the '-' operator: i — i text.cpp 1429

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

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

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

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

V773 Visibility scope of the 'beam' pointer was exited without releasing the memory. A memory leak is possible. read114.cpp 2334

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

V773 The function was exited without releasing the 'voicePtr' pointer. A memory leak is possible. ove.cpp 3967

Достаточно большой фрагмент, но в наличии ошибки легко убедиться. Каждый отмеченный оператор return приводит к потере указателя voicePtr. Если всё же программа выполняется до строки кода с комментарием «SAVE 2», то указатель сохраняется в класс Track. В деструкторе этого класса и будет выполнено освобождение указателей. В остальных случаях будет утечка памяти. Вот как выглядит реализация класса Track:

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

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

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

Такой код допускает использование неинициализированных переменных pageWidth и pageHeight в функции doCredits():

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

V730 Not all members of a class are initialized inside the constructor. Consider inspecting: _dclickValue1, _dclickValue2. aslider.cpp 30

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

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

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

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

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

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

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

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

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

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

V547 Expression 'error == 2' is always false. mididriver.cpp 126

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

V560 A part of conditional expression is always false: strack > — 1. edit.cpp 3669

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

V779 Unreachable code detected. It is possible that an error is present. figuredbass.cpp 1377

Диагностика 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

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

V522 Dereferencing of the null pointer 'customDrumset' might take place. instrument.cpp 328

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

V522 Dereferencing of the null pointer 'segment' might take place. measure.cpp 2220

Это уже не первый большой каскад условий в этом проекте, где допускают ошибки. Стоит задуматься! Здесь указатель 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

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

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

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

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

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

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

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

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

Переменная alter имеет целочисленный тип int. И сравнение с числами 2.5 и -2.5 выглядит очень странным.

V595 The 'sample' pointer was utilized before it was verified against nullptr. Check lines: 926, 929. voice.cpp 926

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

Разное

V523 The 'then' statement is equivalent to the 'else' statement. pluginCreator.cpp 84

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

V524 It is odd that the body of 'downLine' function is fully equivalent to the body of 'upLine' function. rest.cpp 667

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

V766 An item with the same key '«mrcs»' has already been added. importgtp-gp6.cpp 100

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

V1001 The 'ontime' variable is assigned but is not used until the end of the function. rendermidi.cpp 1176

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

V547 Expression 'runState == 0' is always false. pulseaudio.cpp 206

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

Заключение

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

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Review of Music Software Code Defects Part 1. MuseScore