# Гадание на пяти строчках: о чем молчит программа
Забудьте о призраках, настоящая угроза кроется в повседневных вещах, таких как static\_cast, который может неожиданно лишить вас безопасности, и assert, стремительно исчезающий в релизной сборке\. Добро пожаловать в мир ловушек, созданных собственными руками\!
## Введение
В своей недавней статье "[Игровое поле экспериментов: какие ошибки могут подстерегать программиста при создании эмулятора](https://pvs-studio.ru/ru/blog/posts/cpp/1177/)" я разбирала срабатывания анализатора PVS\-Studio в проекте Xenia\. Было рассмотрено много интересных случаев, и я уже собиралась отложить проект и перейти к другим задачам, однако решила взглянуть ещё раз на срабатывания, которые не попали в статью\. Одно из них показалось мне странным: всего пять строчек кода, но я никак не могла разгадать замысел автора\. Даже обсудив этот фрагмент с коллегами, мы не смогли его объяснить\. Поэтому я решила поделиться размышлениями в этой небольшой заметке\.
Коротко о Xenia, если вы не знаете, о чем идёт речь
Xenia — это экспериментальный эмулятор платформы Xbox 360\. Он предназначен для воспроизведения игр, изначально разработанных для этой консоли, на современных ПК\. Этот проект с [открытым исходным кодом](https://github.com/xenia-project/xenia) активно развивается благодаря поддержке сообщества\.
Как уже упоминалось, анализ проводился с использованием статического анализатора [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/)\. А для проверки я использовала состояние репозитория на момент коммита [3d30b2e](https://github.com/xenia-project/xenia/tree/3d30b2eec3ab1f83140b09745bee881fb5d5dde2)\.
Давайте вместе рассмотрим это срабатывание\.
## А вот и он
Прежде чем перейти к примеру, нужно познакомиться с небольшой иерархией классов:
```cpp
class AudioDriver
{
public:
....
virtual void DestroyDriver(AudioDriver* driver) = 0;
....
};
class XAudio2AudioDriver : public AudioDriver
{
....
void Shutdown();
virtual void DestroyDriver(AudioDriver* driver);
....
};
```
А теперь непосредственно код:
```cpp
void XAudio2AudioSystem::DestroyDriver(AudioDriver* driver)
{
assert_not_null(driver);
auto xdriver = static_cast(driver);
xdriver->Shutdown();
assert_not_null(xdriver);
delete xdriver;
}
```
Предупреждение PVS\-Studio:
V595 The 'xdriver' pointer was utilized before it was verified against nullptr\. Check lines: 48, 49\. [xaudio2\_audio\_system\.cc 48](https://github.com/xenia-project/xenia/blob/3d30b2eec3ab1f83140b09745bee881fb5d5dde2/src/xenia/apu/xaudio2/xaudio2_audio_system.cc#L48-L49)
Этот фрагмент примечателен следующими вещами:
1. Класс _XAudio2AudioSystem_ является наследником _AudioDriver_\. Значит, в функцию _XAudio2AudioSystem::DestroyDriver_ прилетает указатель на базовый тип \(_driver_\)\.
1. Макрос [_assert\_not\_null_](https://github.com/xenia-project/xenia/blob/3d30b2eec3ab1f83140b09745bee881fb5d5dde2/src/xenia/base/assert.h#L66) проверяет его состояние\. Он разворачивается в [_xenia\_assert_](https://github.com/xenia-project/xenia/blob/3d30b2eec3ab1f83140b09745bee881fb5d5dde2/src/xenia/base/assert.h#L24), а тот в свою очередь в стандартный [_assert_](https://en.cppreference.com/w/cpp/error/assert)\. Да, этот макрос удаляется под релизом, но мы опустим этот момент\. В дебаге он помогает нам узнать, что указатель не нулевой\.
1. Далее указатель _driver_ преобразуется к указателю на производный класс \(_xdriver_\) через _static\_cast_\. При таком способе не происходит проверка, какой именно объект на самом деле лежит под этим указателем\. Компилятор всего лишь проверяет, валидно ли такое преобразование согласно стандарту, и оно валидно в этом контексте\. При этом результирующий указатель также ненулевой, но не факт, что корректный\.
1. Указатель _xdriver_ разыменовывается, и вызывается нестатическая функция\-член _XAudio2AudioSystem::Shutdown_\. Если динамический тип объекта под этим указателем отличен от _XAudio2AudioSystem_ или его наследников, то поведение будет не определено \(нарушаются правила [strict aliasing](https://en.cppreference.com/w/cpp/language/reinterpret_cast#Type_aliasing)\)\.
1. После этого разработчик задумался, а не нулевой ли случаем этот указатель, и добавил проверку указателя _xdriver_\.
Буквально пять строк функции, а вопросов больше, чем ответов\.\.\. Тяжело сказать, чего именно пытался добиться разработчик, но я вижу два возможных варианта:
* последнюю проверку поставили, потому что разработчик захотел при случае отладить нулевой указатель, который может вернуться после _static\_cast_\. К сожалению, указатель всегда будет ненулевым\. Даже если представить иную ситуацию, то вместо осмысленного сообщения от макроса _assert\_not\_null_ разработчик будет иметь дело в отладчике с [segfault](https://pvs-studio.ru/ru/blog/terms/0063/);
* последнюю проверку поставили, потому что далее указатель отдаётся в оператор _delete_\. "Вдруг произойдёт что\-то нехорошее, если мы передадим ему нулевой указатель, дай\-ка подебажусь при таком случае"\. К счастью, ничего страшного не произойдёт — оператор _delete_ прекрасно обрабатывает нулевые указатели\. И как мы уже поняли, _xdriver_ всегда будет ненулевой\.
Если постараться сохранить исходный замысел автора, то лучше привести код к следующему виду:
```cpp
void XAudio2AudioSystem::DestroyDriver(AudioDriver *driver)
{
assert_not_null(driver);
auto xdriver = dynamic_cast(driver);
assert_not_null(xdriver);
xdriver->Shutdown();
delete xdriver;
}
```
Что интересно, переопределение этой функции, но в другом наследнике \([_SDLAudioDriver::DestroyDriver_](https://github.com/xenia-project/xenia/blob/3d30b2eec3ab1f83140b09745bee881fb5d5dde2/src/xenia/apu/sdl/sdl_audio_system.cc#L44-L50)\), реализовано точно таким же образом\.
Однако я бы хотела предложить исправление получше, которое позволит избавиться от преобразования к нужному производному типу\. Давайте ещё раз взглянем на код аудиосистемы и аудио драйверов\.
Иерархия аудио драйверов
```cpp
class AudioDriver
{
public:
....
virtual ~AudioDriver();
....
};
class SDLAudioDriver : public AudioDriver
{
public:
....
~SDLAudioDriver() override;
....
void Shutdown();
....
};
class XAudio2AudioDriver : public AudioDriver
{
public:
....
~XAudio2AudioDriver() override;
....
void Shutdown();
....
};
```
Иерархия аудиосистем
```cpp
class AudioSystem
{
public:
....
void UnregisterClient(size_t index);
....
protected:
....
virtual X_STATUS CreateDriver(size_t index,
xe::threading::Semaphore* semaphore,
AudioDriver** out_driver) = 0;
virtual void DestroyDriver(AudioDriver* driver) = 0;
....
static const size_t kMaximumClientCount = 8;
struct {
AudioDriver* driver;
uint32_t callback;
uint32_t callback_arg;
uint32_t wrapped_callback_arg;
bool in_use;
} clients_[kMaximumClientCount];
....
};
void AudioSystem::UnregisterClient(size_t index)
{
....
assert_true(index < kMaximumClientCount);
DestroyDriver(clients_[index].driver);
memory()->SystemHeapFree(clients_[index].wrapped_callback_arg);
clients_[index] = {0};
....
}
```
Оба производных класса аудио драйверов имеют одинаковый публичный невиртуальный интерфейс _Shutdown_\. Из\-за этого и приходится в переопределениях _AudioSystem::DestroyDriver_ производить преобразование к нужному производному классу аудио драйвера и затем вызывать эту функцию\.
Можно вынести интерфейс _Shutdown_ в базовый класс в виде чистой виртуальной функции, а _AudioSystem::DestroyDriver_ сделать невиртуальным, убрав из его наследников дублирующийся код\.
Исправление
```cpp
class AudioDriver
{
public:
....
virtual ~AudioDriver();
virtual void Shutdown() = 0;
....
};
class SDLAudioDriver : public AudioDriver
{
public:
....
~SDLAudioDriver() override;
....
void Shutdown() override;
....
};
class XAudio2AudioDriver : public AudioDriver
{
public:
....
~XAudio2AudioDriver() override;
....
void Shutdown() override;
....
};
class AudioSystem
{
protected:
....
void DestroyDriver(AudioDriver* driver);
....
};
void AudioSystem::DestroyDriver(AudioDriver* driver)
{
assert_not_null(driver);
std::unique_ptr tmp { driver };
tmp->Shutdown();
}
```
Оборачивание сырого указателя в _std::unique\_ptr_ позволит не переживать за бросок исключения из функции _Shutdown_: объект под указателем будет удалён оператором _delete_ в любом случае\.
Если потребуется, чтобы наследник _AudioSystem_ всё же мог переопределить поведение при удалении аудио драйвера, то можно воспользоваться идиомой [NVI](https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Non-Virtual_Interface) \(Non\-Virtual Interface\)\.
Исправление с использованием NVI
```cpp
class AudioSystem
{
protected:
....
void DestroyDriver(AudioDriver* driver);
....
private:
virtual void DestroyDriverImpl(AudioDriver* driver);
....
};
void AudioSystem::DestroyDriverImpl(AudioDriver* driver)
{
driver->Shutdown();
}
void AudioSystem::DestroyDriver(AudioDriver* driver)
{
assert_not_null(driver);
std::unique_ptr _ { driver };
DestroyDriverImpl(driver);
}
```
Теперь, если наследнику _AudioSystem_ требуется иное поведение при удалении драйвера, достаточно переопределить виртуальную функцию _DestroyDriverImpl_:
```cpp
class SomeAudioSystem : public AudioSystem
{
....
private:
void DestroyDriverImpl(AudioDriver* driver) override;
};
```
## Итоги
Думаю, теперь с проектом я закончила, но путь к совершенству никогда не заканчивается\. Мне интересно узнать, что вы думаете об этом фрагменте\. Поделитесь своими размышлениями в комментариях :\)
А чтобы так же легко находить подозрительные места в коде, предлагаю попробовать [пробную версию](https://pvs-studio.ru/ru/pvs-studio/try-free/) анализатора PVS\-Studio\. Давайте вместе делать код более надежным и безопасным\!