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

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

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

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

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

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

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


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

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

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

27 Ноя 2017

Я продолжаю обзор кода музыкальных приложений, и перед нами первый представитель коммерческого программного обеспечения. В комментариях к предыдущим статьям я заметил популярность программы Cubase и решил почитать о ней. Это продукт компании Steinberg, у которой есть несколько программ с закрытым исходным кодом. Случайно на их сайте я нашёл SDK для сторонних разработчиков, и, изучив его, обнаружил множество интересных ошибок.

0541_MusicSoftwareDefects_05_SteinbergSDKs_ru/image1.png

Введение

Steinberg GmbH (Steinberg Media Technologies GmbH) – немецкая компания, основанная в Гамбурге, разрабатывающая музыкальное программное обеспечение и оборудование. В большей степени она производит программное обеспечение для записи, аранжировки и редактирования музыки для использования как на цифровых аудио рабочих станциях, так и на синтезаторах с программным обеспечением VSTi формата. Steinberg - дочерняя компания Yamaha Corporation, Steinberg является полной собственностью Yamaha Corporation.

0541_MusicSoftwareDefects_05_SteinbergSDKs_ru/image2.png

Даже для небольшого количества исходников из SDK будет мало одной обзорной статьи, поэтому для просмотра полного отчёта авторы могут самостоятельно проверить проект, запросив в поддержке временный ключ для оценки возможностей статического анализатора PVS-Studio. Это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#. Работает в средах Windows и Linux.

Оператор "запятая" (,)

Оператор "запятая" (,) используется для выполнения стоящих по обе стороны от него выражений в порядке слева направо и получения значения правого выражения. Чаще всего оператор применяется в выражении изменения счётчика для цикла for. Иногда его удобно использовать в макросах отладки и тестирования. Но чаще всего этим оператором злоупотребляют и используют неправильно.

V521 Such expressions using the ',' operator are dangerous. Make sure the expression 'i < temp, i < numParams' is correct. mdaBaseProcessor.cpp 309

tresult PLUGIN_API BaseProcessor::setState (IBStream* state)
{
  ....
  // read each parameter
  for (uint32 i = 0; i < temp, i < numParams; i++)
  {
    state->read (&params[i], sizeof (ParamValue));
    SWAP64_BE(params[i])
  }
  ....
}

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

V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. mdaBaseProcessor.cpp 142

bool BaseProcessor::bypassProcessing (ProcessData& data)
{
  ....
  for (int32 bus = 0; bus < data.numInputs,   // <=
                      bus < data.numOutputs; bus++)
  {
    ....
    if (data.numInputs <= bus ||
        data.inputs[bus].numChannels <= channel)
    {
      memset(data.outputs[bus].channelBuffers32[channel], ....);
      data.outputs[bus].silenceFlags |= (uint64)1 << channel;
    }
    else
    {
      ....
    }
    ....
  }
  ....
}

Вот тут уже допущена серьёзная ошибка. В цикле обращаются к массивам data.inputs и data.outputs, но условное выражение написано с ошибкой. Выражение bus < data.numInputs хоть и вычисляется, но на результат не влияет. Следовательно, возможно обращение к памяти за пределом массива data.inputs.

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

Разные ошибки

V567 Undefined behavior. The 'p' variable is modified while being used twice between sequence points. mdaAmbienceProcessor.cpp 151

void AmbienceProcessor::doProcessing (ProcessData& data)
{
  ....
  ++p  &= 1023;
  ++d1 &= 1023;
  ++d2 &= 1023;
  ++d3 &= 1023;
  ++d4 &= 1023;
  ....
}

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

V595 The 'inputBitmap' pointer was utilized before it was verified against nullptr. Check lines: 409, 410. cbitmapfilter.cpp 409

bool run (bool replace) override
{
  CBitmap* inputBitmap = getInputBitmap ();
  uint32_t radius = static_cast<uint32_t>(static_cast<double>(
    .... * inputBitmap->getPlatformBitmap()->getScaleFactor());
  if (inputBitmap == nullptr || radius == UINT_MAX)
    return false;
  ....
}

Указатель inputBitmap сравнивается с nullptr сразу после использования. Программист хотел проверить указатель inputBitmap и переменную radius в одном условии, но так сделать невозможно, т.к. одно значение вычисляется с помощью другого. Необходимо проверять каждую переменную по отдельности.

V1004 The 'module' pointer was used unsafely after it was verified against nullptr. Check lines: 76, 84. audiohost.cpp 84

void App::startAudioClient (....)
{
  std::string error;
  module = VST3::Hosting::Module::create (path, error);
  if (!module)
  {
    std::string reason = "Could not create Module for file:";
    reason += path;
    reason += "\nError: ";
    reason += error;
    // EditorHost::IPlatform::instance ().kill (-1, reason);
  }
  auto factory = module->getFactory ();
  ....
}

Ранее, если указатель module был равен нулю, выполнение функции прерывалось с помощью вызова kill(). Сейчас вызов этой функции закомментирован, поэтому появился риск разыменования нулевого указателя.

V766 An item with the same key '0xff9b' has already been added. x11frame.cpp 51

using VirtMap = std::unordered_map<guint, uint16_t>;
const VirtMap keyMap = {
  {GDK_KEY_BackSpace, VKEY_BACK},
  {GDK_KEY_Tab, VKEY_TAB},
  {GDK_KEY_ISO_Left_Tab, VKEY_TAB},
  {GDK_KEY_Clear, VKEY_CLEAR},
  {GDK_KEY_Return, VKEY_RETURN},
  {GDK_KEY_Pause, VKEY_PAUSE},
  {GDK_KEY_Escape, VKEY_ESCAPE},
  {GDK_KEY_space, VKEY_SPACE},
  {GDK_KEY_KP_Next, VKEY_NEXT},          // <=
  {GDK_KEY_End, VKEY_END},
  {GDK_KEY_Home, VKEY_HOME},

  {GDK_KEY_Left, VKEY_LEFT},
  {GDK_KEY_Up, VKEY_UP},
  {GDK_KEY_Right, VKEY_RIGHT},
  {GDK_KEY_Down, VKEY_DOWN},
  {GDK_KEY_Page_Up, VKEY_PAGEUP},
  {GDK_KEY_Page_Down, VKEY_PAGEDOWN},
  {GDK_KEY_KP_Page_Up, VKEY_PAGEUP},
  {GDK_KEY_KP_Page_Down, VKEY_PAGEDOWN}, // <=
  ....
};

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

using VirtMap = std::unordered_map<guint, uint16_t>;
const VirtMap keyMap = {
  {0xff08, VKEY_BACK},
  {0xff09, VKEY_TAB},
  {0xfe20, VKEY_TAB},
  {0xff0b, VKEY_CLEAR},
  {0xff0d, VKEY_RETURN},
  {0xff13, VKEY_PAUSE},
  {0xff1b, VKEY_ESCAPE},
  {0x020, VKEY_SPACE},
  {0xff9b, VKEY_NEXT},     // <=
  {0xff57, VKEY_END},
  {0xff50, VKEY_HOME},

  {0xff51, VKEY_LEFT},
  {0xff52, VKEY_UP},
  {0xff53, VKEY_RIGHT},
  {0xff54, VKEY_DOWN},
  {0xff55, VKEY_PAGEUP},
  {0xff56, VKEY_PAGEDOWN},
  {0xff9a, VKEY_PAGEUP},
  {0xff9b, VKEY_PAGEDOWN}, // <=
  ....
};

Действительно, константы GDK_KEY_KP_Next и GDK_KEY_KP_PageDown имеют одинаковое значение 0xff9b. Вот только не понятно, что с этим делать, т.к. константы взяты из библиотеки GDK3.

Несколько примеров из тестов

V571 Recurring check. The 'if (vstPlug)' condition was already verified in line 170. vsttestsuite.cpp 172

bool VstTestBase::teardown ()
{
  if (vstPlug)
  {
    if (vstPlug)
    {
      vstPlug->activateBus (kAudio, kInput, 0, false);
      vstPlug->activateBus (kAudio, kOutput, 0, false);
    }
    plugProvider->releasePlugIn (vstPlug, controller);
  }
  return true;
}

Довольно часто диагностика V571 просто находит лишние проверки, но тут, видимо, настоящая ошибка. Я посмотрел подобные фрагменты в файле и, скорее всего, следует исправить код так:

bool VstTestBase::teardown ()
{
  if (plugProvider) // <=
  {
    if (vstPlug)
    {
      vstPlug->activateBus (kAudio, kInput, 0, false);
      vstPlug->activateBus (kAudio, kOutput, 0, false);
    }
    plugProvider->releasePlugIn (vstPlug, controller);
  }
  return true;
}

V773 The function was exited without releasing the 'paramIds' pointer. A memory leak is possible. vsttestsuite.cpp 436

bool PLUGIN_API VstScanParametersTest::run (....)
{
  ....
  int32* paramIds = new int32[numParameters];

  bool foundBypass = false;
  for (int32 i = 0; i < numParameters; ++i)
  {
    ParameterInfo paramInfo = {0};

    tresult result = controller->getParameterInfo (i, paramInfo);
    if (result != kResultOk)
    {
      addErrorMessage (testResult,
        printf ("Param %03d: is missing!!!", i));
      return false; // Memory Leak
    }

    int32 paramId = paramInfo.id;
    paramIds[i] = paramId;
    if (paramId < 0)
    {
      addErrorMessage (testResult,
        printf ("Param %03d: Invalid Id!!!", i));
      return false; // Memory Leak
    }
  ....
  if (paramIds)
    delete[] paramIds;

  return true;
}

Функция run() имеет более десяти точек выхода, при которых происходит утечка памяти. Только при выполнении функции до конца будет выполнено освобождение памяти для этого массива по указателю paramIds.

Замечания по коду

V523 The 'then' statement is equivalent to the 'else' statement. mdaJX10Processor.cpp 522

void JX10Processor::noteOn (....)
{
  ....
  if (!polyMode) //monophonic retriggering
  {
    voice[v].env += SILENCE + SILENCE;
  }
  else
  {
    //if (params[15] < 0.28f) 
    //{
    //  voice[v].f0 = voice[v].f1 = voice[v].f2 = 0.0f;
    //  voice[v].env = SILENCE + SILENCE;
    //  voice[v].fenv = 0.0f;
    //}
    //else 
    voice[v].env += SILENCE + SILENCE; //anti-glitching trick
  }
  ....
}

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

V573 Uninitialized variable 'oldScrollSize' was used. The variable was used to initialize itself. cscrollview.cpp 482

void CScrollView::setContainerSize (....)
{
  CRect oldSize (containerSize);
  ....
  CRect oldScrollSize = vsb->getScrollSize (oldScrollSize);
  float oldValue = vsb->getValue ();
  ....
}

Анализатор обнаружил потенциальное использование неинициализированной переменной oldScrollSize. Как оказалось, ошибки не будет, но реализация функции getScrollSize() ужасна:

CRect& getScrollSize (CRect& rect) const
{
  rect = scrollSize;
  return rect;
}

Наверняка, такой код выглядел бы лучше:

CRect oldScrollSize = vsb->getScrollSize();
....
CRect& getScrollSize () const
{
  return scrollSize;
}

Ещё несколько таких инициализаций:

  • V573 Uninitialized variable 'oldScrollSize' was used. The variable was used to initialize itself. cscrollview.cpp 503
  • V573 Uninitialized variable 'oldClip' was used. The variable was used to initialize itself. ctabview.cpp 359

V751 Parameter 'column' is not used inside function body. pitchnamesdatabrowsersource.cpp 227

void PitchNamesDataBrowserSource::dbCellTextChanged(
  int32_t row, int32_t column, ....)
{
  if (pitchnames)
  {
    UString128 str (newText);
    if (str.getLength () == 0)
      pitchnames->removePitchName (0, (int16)row);
    else
      pitchnames->setPitchName (0, (int16)row, str);
  }
}

В функции dbCellTextChanged() не используется номер столбца, который был передан в функцию. Мне сложно сказать, есть тут ошибка или нет, поэтому авторам проекта следует перепроверить код.

V570 The same value is assigned twice to the 'lpf' variable. mdaComboProcessor.cpp 274

void ComboProcessor::recalculate ()
{
  ....
  case 4: trim = 0.96f; lpf = filterFreq(1685.f);
      mix1 = -0.85f; mix2 = 0.41f;
      del1 = int (getSampleRate () / 6546.f);
      del2 = int (getSampleRate () / 3315.f);
      break;

  case 5: trim = 0.59f; lpf = lpf = filterFreq(2795.f); // <=
      mix1 = -0.29f; mix2 = 0.38f;
      del1 = int (getSampleRate () / 982.f);
      del2 = int (getSampleRate () / 2402.f);
      hpf = filterFreq(459.f); 
      break;
  ....
}

Небольшое замечание по коду: в коде присутствует лишнее присваивание переменной lpf. Скорее всего, это опечатка, случайным образом не приводящая к ошибке.

Заключение

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

Моя позиция по вопросу, какой код качественнее - открытый или закрытый - очень проста. Качество кода в большей степени зависит от руководителя проекта, чем от закрытости/открытости кода. С открытым кодом, конечно, хорошо: легко сообщить о баге, кто-то добавит функционал, кто-то исправит ошибку... но если руководитель не обеспечит использование в проекте методик контроля качества, то лучше не станет. Необходимо использовать все доступные бесплатные решения и по возможности добавлять к ним платные инструменты.

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

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

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

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

Дата: 22 Дек 2018

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

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

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 31 Май 2014

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

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

Дата: 27 Июн 2017

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

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

Дата: 30 Янв 2019

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

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

Дата: 19 Май 2017

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

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

Дата: 16 Окт 2017

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

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

Дата: 21 Ноя 2018

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

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

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

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