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

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

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

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

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

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

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


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

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

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

11 Окт 2017

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

0532_MusicSoftwareDefects_02_Audacity_ru/image1.png

Введение

Audacity - свободный многоплатформенный аудиоредактор звуковых файлов, ориентированный на работу с несколькими дорожками. Программа распространяется с открытым исходным кодом и работает под управлением таких операционных систем, как Microsoft Windows, Linux, macOS 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 на своём проекте очень легко, достаточно перейти на страницу загрузки.

Популярные статьи по теме
Как различить C и C++ разработчиков по их коду

Дата: 12 Май 2022

Автор: Гость

Так уж случилось, что я пишу код для разных IoT-железок, связанных с электричеством, типа зарядных станций автомобилей. Поскольку аппаратных ресурсов, как правило, вполне достаточно, то основным фоку…
Отладочный вывод на микроконтроллерах: как Concepts и Ranges отправили мой printf на покой

Дата: 06 Май 2022

Автор: Гость

Здравствуйте! Меня зовут Александр, и я работаю программистом микроконтроллеров.
Нереальный baselining или доработки PVS-Studio для Unreal Engine проектов

Дата: 26 Апр 2022

Автор: Валерий Комаров

Статический анализатор PVS-Studio постоянно развивается: улучшаются различные механизмы, происходит интеграция с игровыми движками, IDE, CI/CD и другими системами и сервисами. Благодаря этому несколь…
Разбор некоторых вредных советов для С++ программиста

Дата: 21 Апр 2022

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

Юмор юмором, но осторожность не повредит. Вдруг кому-то не до конца понятно, почему какой-то из советов является вредным. Здесь можно найти соответствующие пояснения.
Четыре причины проверять, что вернула функция malloc

Дата: 20 Апр 2022

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

Некоторые разработчики пренебрежительно относятся к проверкам: удалось ли выделить память с помощью функции malloc или нет. Их логика проста – памяти всегда должно хватить. А если не хватит, всё равн…

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

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