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

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

Audacity — свободный многоплатформенный аудиоредактор звуковых файлов, ориентированный на работу с несколькими дорожками. Программа распространяется с открытым исходным кодом и работает под управлением таких операционных систем, как Microsoft Windows, Linux, Mac OS X, FreeBSD и других.

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

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

Copy-Paste — он везде!

V523 The 'then' statement is equivalent to the 'else' statement. AButton.cpp 297

Приведённый фрагмент кода — идеальный претендент для копирования: достаточно поменять условное выражение и пару констант. К сожалению, автор не довёл дело до конца и оставил в коде две идентичных ветви кода.

Ещё несколько странных мест:

  • V523 The 'then' statement is equivalent to the 'else' statement. ASlider.cpp 394
  • V523 The 'then' statement is equivalent to the 'else' statement. ExpandingToolBar.cpp 297
  • V523 The 'then' statement is equivalent to the 'else' statement. Ruler.cpp 2422

V501 There are identical sub-expressions 'buffer[remaining — WindowSizeInt — 2]' to the left and to the right of the '-' operator. VoiceKey.cpp 309

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

Неправильное использование функций

V530 The return value of function 'remove' is required to be utilized. OverlayPanel.cpp 31

Неправильное использование функции std::remove() так распространено, что такой пример приведён в документации к этой диагностике. Поэтому, чтобы снова не копировать описание из документации, я просто приведу исправленный вариант:

V530 The return value of function 'Left' is required to be utilized. ASlider.cpp 973

Вот как выглядит прототип функции Left():

Очевидно, что строка val не изменится. Скорее всего, изменённую строку хотели сохранить обратно в val, но не прочли документацию к функции.

Страшный сон пользователей ПК

V590 Consider inspecting this expression. The expression is excessive or contains a misprint. ExtImportPrefs.cpp 600

Многие пользователи компьютерных программ когда-нибудь нажимали «не туда» и пытались отменить действие… Так вот найденная ошибка в Audacity заключается в том, что условие, проверяющее нажатую кнопку в диалоговом окне, не зависит от того, нажали на «No» или нет :D

Вот как выглядит таблица истинности для приведённого фрагмента кода:

«while» или «if»?

V612 An unconditional 'return' within a loop. Equalization.cpp 379

Вот такой цикл написал автор, который выполняется 1 или 0 итераций. Если тут нет ошибки, то нагляднее будет переписать на использование оператора if.

Использование std::unique_ptr

V522 Dereferencing of the null pointer 'mInputStream' might take place. FileIO.cpp 65

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

V607 Ownerless expression. LoadEffects.cpp 340

Пример очень интересного применения unique_ptr. Этот «однострочник» (без учёта форматирования) служит для того, чтобы создать unique_ptr и тут же его уничтожить, освободив при этом указатель instance.

Разное

V779 Unreachable code detected. It is possible that an error is present. ToolBar.cpp 706

Анализатор обнаружил недостижимый код из-за безусловного оператора return в коде.

V610 Undefined behavior. Check the shift operator '<<'. The left operand '-1' is negative. ExportFFmpeg.cpp 229

Намерено делают сдвиг отрицательного числа, что может приводить к неочевидным проблемам.

V595 The 'clip' pointer was utilized before it was verified against nullptr. Check lines: 4094, 4095. Project.cpp 4094

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

Ещё несколько опасных мест:

  • V595 The 'outputMeterFloats' pointer was utilized before it was verified against nullptr. Check lines: 5246, 5255. AudioIO.cpp 5246
  • V595 The 'buffer2' pointer was utilized before it was verified against nullptr. Check lines: 404, 409. Compressor.cpp 404
  • V595 The 'p' pointer was utilized before it was verified against nullptr. Check lines: 946, 974. ControlToolBar.cpp 946
  • V595 The 'mParent' pointer was utilized before it was verified against nullptr. Check lines: 1890, 1903. LV2Effect.cpp 1890

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

V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!j->hasFixedBinCount' and 'j->hasFixedBinCount'. LoadVamp.cpp 169

Условие является избыточным. Его можно упростить до такого варианта:

И ещё один пример такого кода:

  • V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!j->hasFixedBinCount' and 'j->hasFixedBinCount'. LoadVamp.cpp 297

Заключение

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

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

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

📎📎📎📎📎📎📎📎📎📎