# Обзор дефектов кода музыкального софта\. Часть 2\. Audacity Цикл статей про обзор дефектов кода музыкально софта продолжается\. Вторым претендентом для анализа выбран аудиоредактор Audacity\. Это программа очень популярна и широко используется, как любителями, так и профессионалами в музыкальной индустрии\. В этой статье описание фрагментов кода будет дополнительно сопровождаться популярными мемами\. Скучно не будет\! ![0532_MusicSoftwareDefects_02_Audacity_ru/image1.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image1.png) ## Введение [Audacity](http://www.audacityteam.org/home/) \- свободный многоплатформенный аудиоредактор звуковых файлов, ориентированный на работу с несколькими дорожками\. Программа распространяется с открытым исходным кодом и работает под управлением таких операционных систем, как Microsoft Windows, Linux, macOS X, FreeBSD и других\. Audacity активно пользуется сторонними библиотеками, поэтому из анализа был исключён каталог _lib\-src_, в котором содержалось ещё около тысячи файлов с исходным кодом разных библиотек\. В статью вошли только наиболее интересные ошибки\. Для просмотра полного отчёта авторы могут самостоятельно проверить проект, запросив в поддержке временный ключ\. [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/) \- это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C\+\+ и C\#\. Работает в среде Windows и Linux\. ## Copy\-Paste \- он везде\! ![0532_MusicSoftwareDefects_02_Audacity_ru/image2.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image2.png) [V523](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. AButton\.cpp 297 ```cpp AButton::AButtonState AButton::GetState() { .... if (mIsClicking) { state = mButtonIsDown ? AButtonOver : AButtonDown; //ok } else { state = mButtonIsDown ? AButtonDown : AButtonOver; //ok } } } else { if (mToggle) { state = mButtonIsDown ? AButtonDown : AButtonUp; // <= fail } else { state = mButtonIsDown ? AButtonDown : AButtonUp; // <= fail } } return state; } ``` Приведённый фрагмент кода \- идеальный претендент для копирования: достаточно поменять условное выражение и пару констант\. К сожалению, автор не довёл дело до конца и оставил в коде две идентичных ветви кода\. Ещё несколько странных мест: * 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](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'buffer\[remaining \- WindowSizeInt \- 2\]' to the left and to the right of the '\-' operator\. VoiceKey\.cpp 309 ```cpp sampleCount VoiceKey::OnBackward ( const WaveTrack & t, sampleCount end, sampleCount len) { .... int atrend = sgn(buffer[remaining - 2]-buffer[remaining - 1]); int ztrend = sgn(buffer[remaining - WindowSizeInt - 2] - buffer[remaining - WindowSizeInt - 2]); .... } ``` При втором вызове функции _sgn\(\)_ в неё передаётся разность одинаковых значений\. Скорее всего, хотели получить разницу между соседними элементами буфера, но забыли изменить двойку на единицу после копирования фрагмента строки\. ## Неправильное использование функций ![0532_MusicSoftwareDefects_02_Audacity_ru/image3.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image3.png) [V530](https://pvs-studio.ru/ru/docs/warnings/v530/) The return value of function 'remove' is required to be utilized\. OverlayPanel\.cpp 31 ```cpp bool OverlayPanel::RemoveOverlay(Overlay *pOverlay) { const size_t oldSize = mOverlays.size(); std::remove(mOverlays.begin(), mOverlays.end(), pOverlay); return oldSize != mOverlays.size(); } ``` Неправильное использование функции _std::remove\(\)_ так распространено, что такой пример приведён в документации к этой диагностике\. Поэтому, чтобы снова не копировать описание из документации, я просто приведу исправленный вариант: ```cpp bool OverlayPanel::RemoveOverlay(Overlay *pOverlay) { const size_t oldSize = mOverlays.size(); mOverlays.erase(std::remove(mOverlays.begin(), mOverlays.end(), pOverlay), mOverlays.end()); return oldSize != mOverlays.size(); } ``` ![0532_MusicSoftwareDefects_02_Audacity_ru/image4.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image4.png) [V530](https://pvs-studio.ru/ru/docs/warnings/v530/) The return value of function 'Left' is required to be utilized\. ASlider\.cpp 973 ```cpp wxString LWSlider::GetTip(float value) const { wxString label; if (mTipTemplate.IsEmpty()) { wxString val; switch(mStyle) { case FRAC_SLIDER: val.Printf(wxT("%.2f"), value); break; case DB_SLIDER: val.Printf(wxT("%+.1f dB"), value); if (val.Right(1) == wxT("0")) { val.Left(val.Length() - 2); // <= } break; .... } ``` Вот как выглядит прототип функции _Left\(\)_: ```cpp wxString Left (size_t count) const ``` Очевидно, что строка _val_ не изменится\. Скорее всего, изменённую строку хотели сохранить обратно в _val_, но не прочли документацию к функции\. ## Страшный сон пользователей ПК ![](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image5.mp4) [V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting this expression\. The expression is excessive or contains a misprint\. ExtImportPrefs\.cpp 600 ```cpp void ExtImportPrefs::OnDelRule(wxCommandEvent& WXUNUSED(event)) { .... int msgres = wxMessageBox (_("...."), wxYES_NO, RuleTable); if (msgres == wxNO || msgres != wxYES) return; .... } ``` Многие пользователи компьютерных программ когда\-нибудь нажимали "не туда" и пытались отменить действие\.\.\. Так вот найденная ошибка в Audacity заключается в том, что условие, проверяющее нажатую кнопку в диалоговом окне, не зависит от того, нажали на "No" или нет :D Вот как выглядит таблица истинности для приведённого фрагмента кода: ![0532_MusicSoftwareDefects_02_Audacity_ru/image7.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image7.png) Все подобные ошибки в условиях собраны в статье "[Логические выражения в C/C\+\+\. Как ошибаются профессионалы](https://pvs-studio.ru/ru/blog/posts/cpp/0390/)"\. ## "while" или "if"? ![0532_MusicSoftwareDefects_02_Audacity_ru/image8.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image8.png) [V612](https://pvs-studio.ru/ru/docs/warnings/v612/) An unconditional 'return' within a loop\. Equalization\.cpp 379 ```cpp bool EffectEqualization::ValidateUI() { while (mDisallowCustom && mCurveName.IsSameAs(wxT("unnamed"))) { wxMessageBox(_("...."), _("EQ Curve needs a different name"), wxOK | wxCENTRE, mUIParent); return false; } .... } ``` Вот такой цикл написал автор, который выполняется 1 или 0 итераций\. Если тут нет ошибки, то нагляднее будет переписать на использование оператора _if_\. ## Использование std::unique\_ptr ![0532_MusicSoftwareDefects_02_Audacity_ru/image9.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image9.png) [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'mInputStream' might take place\. FileIO\.cpp 65 ```cpp std::unique_ptr mInputStream; std::unique_ptr mOutputStream; wxInputStream & FileIO::Read(void *buf, size_t size) { if (mInputStream == NULL) { return *mInputStream; } return mInputStream->Read(buf, size); } wxOutputStream & FileIO::Write(const void *buf, size_t size) { if (mOutputStream == NULL) { return *mOutputStream; } return mOutputStream->Write(buf, size); } ``` Очень странный код обнаружил анализатор\. Разыменование указателей происходит в любом случае, независимо от того, равен он нулю или нет\. [V607](https://pvs-studio.ru/ru/docs/warnings/v607/) Ownerless expression\. LoadEffects\.cpp 340 ```cpp void BuiltinEffectsModule::DeleteInstance(IdentInterface *instance) { // Releases the resource. std::unique_ptr < Effect > { dynamic_cast(instance) }; } ``` Пример очень интересного применения _unique\_ptr_\. Этот "однострочник" \(без учёта форматирования\) служит для того, чтобы создать _unique\_ptr_ и тут же его уничтожить, освободив при этом указатель _instance_\. ## Разное ![0532_MusicSoftwareDefects_02_Audacity_ru/image10.png](https://import.viva64.com/docx/blog/0532_MusicSoftwareDefects_02_Audacity_ru/image10.png) [V779](https://pvs-studio.ru/ru/docs/warnings/v779/) Unreachable code detected\. It is possible that an error is present\. ToolBar\.cpp 706 ```cpp void ToolBar::MakeRecoloredImage( teBmps eBmpOut, teBmps eBmpIn ) { // Don't recolour the buttons... MakeMacRecoloredImage( eBmpOut, eBmpIn ); return; wxImage * pSrc = &theTheme.Image( eBmpIn ); .... } ``` Анализатор обнаружил недостижимый код из\-за безусловного оператора _return_ в коде\. [V610](https://pvs-studio.ru/ru/docs/warnings/v610/) Undefined behavior\. Check the shift operator '<<'\. The left operand '\-1' is negative\. ExportFFmpeg\.cpp 229 ```cpp #define AV_VERSION_INT(a, b, c) (a<<16 | b<<8 | c) ExportFFmpeg::ExportFFmpeg() : ExportPlugin() { .... int canmeta = ExportFFmpegOptions::fmts[newfmt].canmetadata; if (canmeta && (canmeta == AV_VERSION_INT(-1,-1,-1) // <= || canmeta <= avfver)) { SetCanMetaData(true,fmtindex); } .... } ``` Намерено делают сдвиг отрицательного числа, что может приводить к неочевидным проблемам\. [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'clip' pointer was utilized before it was verified against nullptr\. Check lines: 4094, 4095\. Project\.cpp 4094 ```cpp void AudacityProject::AddImportedTracks(....) { .... WaveClip* clip = ((WaveTrack*)newTrack)->GetClipByIndex(0); BlockArray &blocks = clip->GetSequence()->GetBlockArray(); if (clip && blocks.size()) { .... } .... } ``` В этом условии проверять указатель _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 [V583](https://pvs-studio.ru/ru/docs/warnings/v583/) The '?:' operator, regardless of its conditional expression, always returns one and the same value: true\. TimeTrack\.cpp 296 ```cpp void TimeTrack::WriteXML(XMLWriter &xmlFile) const { .... // MB: so why don't we just call Invalidate()? :) mRuler->SetFlip(GetHeight() > 75 ? true : true); .... } ``` Похоже один из разработчиков догадался, что такой код не имеет смысла, но вместо исправления кода, решил его прокомментировать\. [V728](https://pvs-studio.ru/ru/docs/warnings/v728/) An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!j\-\>hasFixedBinCount' and 'j\-\>hasFixedBinCount'\. LoadVamp\.cpp 169 ```cpp wxArrayString VampEffectsModule::FindPlugins(....) { .... if (.... || !j->hasFixedBinCount || (j->hasFixedBinCount && j->binCount > 1)) { ++output; continue; } .... } ``` Условие является избыточным\. Его можно упростить до такого варианта: ```cpp !j->hasFixedBinCount || j->binCount > 1 ``` И ещё один пример такого кода: * V728 An excessive check can be simplified\. The '\|\|' operator is surrounded by opposite expressions '\!j\-\>hasFixedBinCount' and 'j\-\>hasFixedBinCount'\. LoadVamp\.cpp 297 ## Заключение Вряд ли такие ошибки будут заметны конечному слушателю, но вот пользователям Audacity это может доставлять большие неудобства\. Другие обзоры музыкального софта: * [Часть 1\. MuseScore](https://pvs-studio.ru/ru/blog/posts/cpp/0530/) * [Часть 2\. Audacity](https://pvs-studio.ru/ru/blog/posts/cpp/0532/) * [Часть 3\. Rosegarden](https://pvs-studio.ru/ru/blog/posts/cpp/0536/) * [Часть 4\. Ardour](https://pvs-studio.ru/ru/blog/posts/cpp/0540/) * [Часть 5\. Steinberg SDKs](https://pvs-studio.ru/ru/blog/posts/cpp/0541/) Если вы знаете интересный софт для работы с музыкой и хотите увидеть его в обзоре, то присылайте названия мне на [почту](mailto:razmyslov@viva64.com)\. Попробовать анализатор PVS\-Studio на своём проекте очень легко, достаточно перейти на страницу [загрузки](https://pvs-studio.ru/ru/pvs-studio/download/)\.