Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

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

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

11 Окт 2017

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

0532_MusicSoftwareDefects_02_Audacity_ru/image1.png

Введение

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

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

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

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

0532_MusicSoftwareDefects_02_Audacity_ru/image2.png

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

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 There are identical sub-expressions 'buffer[remaining - WindowSizeInt - 2]' to the left and to the right of the '-' operator. VoiceKey.cpp 309

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

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

bool OverlayPanel::RemoveOverlay(Overlay *pOverlay)
{
  const size_t oldSize = mOverlays.size();
  std::remove(mOverlays.begin(), mOverlays.end(), pOverlay);
  return oldSize != mOverlays.size();
}

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

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

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

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():

wxString Left (size_t count) const

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

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

0532_MusicSoftwareDefects_02_Audacity_ru/image5.gif

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

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/image6.png

Все подобные ошибки в условиях собраны в статье "Логические выражения в C/C++. Как ошибаются профессионалы".

"while" или "if"?

0532_MusicSoftwareDefects_02_Audacity_ru/image7.png

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

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/image8.png

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

std::unique_ptr<wxInputStream> mInputStream;
std::unique_ptr<wxOutputStream> 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 Ownerless expression. LoadEffects.cpp 340

void BuiltinEffectsModule::DeleteInstance(IdentInterface *instance)
{
   // Releases the resource.
   std::unique_ptr < Effect > {
      dynamic_cast<Effect *>(instance)
   };
}

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

Разное

0532_MusicSoftwareDefects_02_Audacity_ru/image9.png

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

void ToolBar::MakeRecoloredImage( teBmps eBmpOut, teBmps eBmpIn )
{
  // Don't recolour the buttons...
  MakeMacRecoloredImage( eBmpOut, eBmpIn );
  return;
  wxImage * pSrc = &theTheme.Image( eBmpIn );
  ....
}

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

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

#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 The 'clip' pointer was utilized before it was verified against nullptr. Check lines: 4094, 4095. Project.cpp 4094

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 The '?:' operator, regardless of its conditional expression, always returns one and the same value: true. TimeTrack.cpp 296

void TimeTrack::WriteXML(XMLWriter &xmlFile) const
{
  ....
  // MB: so why don't we just call Invalidate()? :)
  mRuler->SetFlip(GetHeight() > 75 ? true : true);
  ....
}

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

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

wxArrayString VampEffectsModule::FindPlugins(....)
{
  ....
  if (.... ||
      !j->hasFixedBinCount ||
      (j->hasFixedBinCount && j->binCount > 1))
 {
   ++output;
   continue;
 }
 ....
}

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

!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 это может доставлять большие неудобства.

Другие обзоры музыкального софта:

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

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

Популярные статьи по теме
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

Автор: Андрей Карпов

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Технологии, используемые в анализаторе кода PVS-Studio для поиска ошибок и потенциальных уязвимостей

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
Эффект последней строки

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

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

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

Автор: Андрей Карпов

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…

Комментарии (0)

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