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

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

![0541_MusicSoftwareDefects_05_SteinbergSDKs_ru/image1.png](https://import.viva64.com/docx/blog/0541_MusicSoftwareDefects_05_SteinbergSDKs_ru/image1.png)

## Введение

[Steinberg GmbH](https://www.steinberg.net/en/home.html) \(Steinberg Media Technologies GmbH\) – немецкая компания, основанная в Гамбурге, разрабатывающая музыкальное программное обеспечение и оборудование\. В большей степени она производит программное обеспечение для записи, аранжировки и редактирования музыки для использования как на цифровых аудио рабочих станциях, так и на синтезаторах с программным обеспечением VSTi формата\. Steinberg \- дочерняя компания [Yamaha Corporation](https://www.yamaha.com/), Steinberg является полной собственностью Yamaha Corporation\.

![0541_MusicSoftwareDefects_05_SteinbergSDKs_ru/image2.png](https://import.viva64.com/docx/blog/0541_MusicSoftwareDefects_05_SteinbergSDKs_ru/image2.png)

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

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

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

[V521](https://pvs-studio.ru/ru/docs/warnings/v521/) Such expressions using the ',' operator are dangerous\. Make sure the expression 'i < temp, i < numParams' is correct\. mdaBaseProcessor\.cpp 309

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v521/) Such expressions using the ',' operator are dangerous\. Make sure the expression is correct\. mdaBaseProcessor\.cpp 142

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v567/) Undefined behavior\. The 'p' variable is modified while being used twice between sequence points\. mdaAmbienceProcessor\.cpp 151

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

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

[V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'inputBitmap' pointer was utilized before it was verified against nullptr\. Check lines: 409, 410\. cbitmapfilter\.cpp 409

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v1004/) The 'module' pointer was used unsafely after it was verified against nullptr\. Check lines: 76, 84\. audiohost\.cpp 84

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v766/) An item with the same key '0xff9b' has already been added\. x11frame\.cpp 51

```cpp
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}, // <=
  ....
};
```

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

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'if \(vstPlug\)' condition was already verified in line 170\. vsttestsuite\.cpp 172

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v571/) просто находит лишние проверки, но тут, видимо, настоящая ошибка\. Я посмотрел подобные фрагменты в файле и, скорее всего, следует исправить код так:

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v773/) The function was exited without releasing the 'paramIds' pointer\. A memory leak is possible\. vsttestsuite\.cpp 436

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v523/) The 'then' statement is equivalent to the 'else' statement\. mdaJX10Processor\.cpp 522

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v573/) Uninitialized variable 'oldScrollSize' was used\. The variable was used to initialize itself\. cscrollview\.cpp 482

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

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

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

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

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v751/) Parameter 'column' is not used inside function body\. pitchnamesdatabrowsersource\.cpp 227

```cpp
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](https://pvs-studio.ru/ru/docs/warnings/v570/) The same value is assigned twice to the 'lpf' variable\. mdaComboProcessor\.cpp 274

```cpp
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 содержит разные исходники, включая примеры плагинов\. Найденные ошибки могут отражать состояние кода других продуктов компании с закрытым исходным кодом\.

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

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

* [Часть 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/)

Если вы знаете интересный софт для работы с музыкой и хотите увидеть его в обзоре, то присылайте названия мне на [почту](mailto:razmyslov@viva64.com)\.

Попробовать анализатор PVS\-Studio на своём проекте очень легко, достаточно перейти на страницу [загрузки](https://pvs-studio.ru/ru/pvs-studio/download/)\.