Как вы, наверное, уже догадались из заголовка, сегодняшняя статья будет посвящена ошибкам в исходном коде программного обеспечения. Но не только этому. Если вас интересует не только C++ и чтение о багах в коде других разработчиков, но и необычные видеоигры, а также то, что такое «рогалики» и как в них играть, тогда добро пожаловать!
В поисках необычных игр я наткнулся на Cataclysm Dark Days Ahead , которая выделяется среди других игр благодаря графике, основанной на ASCII-символах разных цветов, расположенных на черном фоне.
Одна из вещей, которая поражает в этой и других подобных играх, — это то, насколько много в них встроенного функционала. В частности, в Cataclysm , например, вы даже не можете создать персонажа, не почувствовав непреодолимого желания поискать в Google руководства, потому что доступны десятки параметров, характеристик и начальных сценариев, не говоря уже о множестве вариаций событий, происходящих на протяжении игры.
Поскольку это игра с открытым исходным кодом, написанная на C++, мы не могли пройти мимо, не проверив её с помощью нашего статического анализатора кода PVS-Studio, в разработке которого я принимаю активное участие. Код проекта оказался на удивление качественным, но всё же содержит некоторые незначительные дефекты, о некоторых из которых я расскажу в этой статье.
Уже довольно много игр было протестировано с помощью PVS-Studio. Некоторые примеры вы можете найти в нашей статье « Статический анализ в разработке видеоигр: 10 самых распространенных программных ошибок ».
Логика
Пример 1:
Этот пример демонстрирует классическую ошибку копирования и вставки.
V501 Слева и справа от оператора '||' находятся идентичные подвыражения: rng(2, 7) < abs(z) || rng(2, 7) < abs(z) overmap.cpp 1503
bool overmap::generate_sub( const int z )
{
....
если( rng( 2, 7 ) < abs( z ) || rng( 2, 7 ) < abs( z ) )
{
....
}
....
}
Одно и то же условие проверяется дважды. Программист скопировал выражение, но забыл его изменить. Я не уверен, является ли это критической ошибкой, но факт в том, что проверка не работает так, как должна.
Ещё одна похожая ошибка:
- V501 Слева и справа от оператора '&&' находятся идентичные подвыражения 'one_in(100000 / to_turns <int> (dur))'. player_hardcoded_effects.cpp 547
Пример 2:
V728 Избыточную проверку можно упростить. Выражение '(A && B) || (!A && !B)' эквивалентно выражению 'bool(A) == bool(B)'. inventory_ui.cpp 199
bool inventory_selector_preset::sort_compare( .... ) const
{
....
const bool left_fav = g->u.inv.assigned.count( lhs.location->invlet );
const bool right_fav = g->u.inv.assigned.count( rhs.location->invlet );
if( ( left_fav && right_fav ) || ( !left_fav && !right_fav ) ) {
возвращаться ....
}
....
}
Это условие логически корректно, но оно слишком сложное. Тому, кто написал этот код, следовало бы пожалеть своих коллег-программистов, которым предстоит его поддерживать. Его можно было бы переписать в более простой форме: if( left_fav == right_fav ) .
Ещё одна похожая ошибка:
- V728 Избыточную проверку можно упростить. Выражение '(A && !B) || (!A && B)' эквивалентно выражению 'bool(A) != bool(B)'. iuse_actor.cpp 2653
Отступление I
Я был удивлен, обнаружив, что игры, которые сегодня называют «рогаликами», являются лишь умеренными представителями старого жанра рогаликов. Все началось с культовой игры Rogue 1980 года, которая вдохновила многих студентов и программистов на создание собственных игр с похожими элементами. Думаю, большое влияние оказало и сообщество настольной игры D&D и ее вариаций.
Микрооптимизации
Пример 3:
Предупреждения этой группы указывают на места, которые потенциально можно оптимизировать, а не на ошибки.
V801 Снижение производительности. Лучше переопределить второй аргумент функции как ссылку. Рассмотрите возможность замены 'const… type' на 'const… &type'. map.cpp 4644
шаблон <typename Stack>
std::list<item> use_amount_stack( Stack stack, const itype_id type )
{
std::list<item> ret;
for( auto a = stack.begin(); a != stack.end() && quantity > 0; ) {
if( a->use_amount( type, ret ) ) {
a = stack.erase( a );
} еще {
++a;
}
}
return ret;
}
В этом коде itype_id на самом деле представляет собой замаскированную строку std::string . Поскольку аргумент в любом случае передается как константа, что означает его неизменяемость, простая передача ссылки на переменную поможет повысить производительность и сэкономить вычислительные ресурсы, избегая операции копирования. И хотя строка вряд ли будет длинной, копировать ее каждый раз без веской причины — плохая идея, тем более что эта функция вызывается различными вызывающими функциями, которые, в свою очередь, также получают тип извне и должны его копировать.
Аналогичные проблемы:
- V801 Снижение производительности. Лучше переопределить третий аргумент функции как ссылку. Рассмотрите возможность замены 'const… evt_filter' на 'const… &evt_filter'. input.cpp 691
- V801 Снижение производительности. Лучше переопределить пятый аргумент функции как ссылку. Рассмотрите возможность замены 'const… color' на 'const… &color'. output.h 207
- Анализатор выдал в общей сложности 32 предупреждения такого типа.
Пример 4:
V813 Снижена производительность. Аргумент 'str' следует, вероятно, отображать как константную ссылку. catacharset.cpp 256
std::string base64_encode( std::string str )
{
if( str.length() > 0 && str[0] == '#' ) {
вернуть строку;
}
int input_length = str.length();
std::string encoded_data( output_length, '\0' );
....
for( int i = 0, j = 0; i < input_length; ) {
....
}
for( int i = 0; i < mod_table[input_length % 3]; i++ ) {
encoded_data[output_length - 1 - i] = '=';
}
return "#" + encoded_data;
}
Хотя аргумент не является константой, он никак не изменяется в теле функции. Поэтому, в целях оптимизации, лучшим решением было бы передавать его по константной ссылке, а не заставлять компилятор создавать локальные копии.
Это предупреждение было не единственным; общее количество предупреждений такого типа составляет 26.
Аналогичные проблемы:
- V813 Снижена производительность. Аргумент 'message' следует, вероятно, отображать как константную ссылку. json.cpp 1452
- V813 Снижение производительности. Аргумент 's' следует, вероятно, отображать как константную ссылку. catacharset.cpp 218
- И так далее...
Отступление II
Некоторые классические roguelike-игры до сих пор находятся в активной разработке. Если вы посмотрите репозитории GitHub Cataclysm DDA или NetHack , вы увидите, что изменения вносятся ежедневно. NetHack — это, пожалуй, самая старая игра, которая до сих пор находится в разработке: она вышла в июле 1987 года, а последняя версия датируется 2018 годом.
Dwarf Fortress — одна из самых популярных, хотя и относительно новых, игр этого жанра. Разработка началась в 2002 году, а первая версия вышла в 2006 году. Её девиз «Проигрывать — это весело» отражает тот факт, что в этой игре невозможно победить. В 2007 году Dwarf Fortress была удостоена награды «Лучшая игра в жанре Roguelike года» по результатам ежегодного голосования на сайте ASCII GAMES.
Кстати, фанаты, возможно, обрадуются, узнав, что Dwarf Fortress выходит в Steam с улучшенной 32-битной графикой, добавленной двумя опытными моддерами. Премиум-версия также получит дополнительные музыкальные треки и поддержку Steam Workshop. Владельцы платных копий смогут при желании переключиться на старую ASCII-графику. Подробнее .
Переопределение оператора присваивания
Примеры 5, 6:
Вот пара интересных предупреждений.
V690 Класс 'JsonObject' реализует конструктор копирования, но не имеет оператора '='. Использование такого класса опасно. json.h 647
класс JsonObject
{
частный:
....
JsonIn *jsin;
....
публичный:
JsonObject( JsonIn &jsin );
JsonObject( const JsonObject &jsobj );
JsonObject() : positions(), start( 0 ), end( 0 ), jsin( NULL ) {}
~JsonObject() {
заканчивать();
}
void finish(); // перемещает поток в конец объекта
....
void JsonObject::finish()
{
....
}
....
}
Этот класс имеет конструктор копирования и деструктор, но не переопределяет оператор присваивания. Проблема в том, что автоматически сгенерированный оператор присваивания может присвоить указатель только на JsonIn . В результате оба объекта класса JsonObject будут указывать на один и тот же JsonIn . Я не могу с уверенностью сказать, может ли такая ситуация возникнуть в текущей версии, но кто-нибудь обязательно попадёт в эту ловушку однажды.
У следующего класса аналогичная проблема.
V690 Класс 'JsonArray' реализует конструктор копирования, но в нем отсутствует оператор '='. Использование такого класса опасно. json.h 820
класс JsonArray
{
частный:
....
JsonIn *jsin;
....
публичный:
JsonArray( JsonIn &jsin );
JsonArray( const JsonArray &jsarr );
JsonArray() : positions(), ...., jsin( NULL ) {};
~JsonArray() {
заканчивать();
}
void finish(); // переместить позицию потока в конец массива
void JsonArray::finish()
{
....
}
}
Опасность непереопределения оператора присваивания в сложном классе подробно объясняется в статье « Закон двух главных правил ».
Примеры 7, 8:
Эти две программы также посвящены переопределению операторов присваивания, но на этот раз рассматриваются конкретные варианты их реализации.
V794 Оператор присваивания должен быть защищен от случая 'this == &other'. mattack_common.h 49
class StringRef {
публичный:
....
частный:
friend struct StringRefTestAccess;
char const* m_start;
size_type m_size;
char* m_data = nullptr;
....
auto operator = ( StringRef const &other ) noexcept -> StringRef& {
delete[] m_data;
m_data = nullptr;
m_start = other.m_start;
m_size = other.m_size;
вернуть *это;
}
Данная реализация не обеспечивает защиты от потенциального самоприсваивания, что является небезопасной практикой. То есть, передача ссылки *this оператору this может привести к утечке памяти.
Вот похожий пример некорректно переопределенного оператора присваивания с необычным побочным эффектом:
V794 Оператор присваивания должен быть защищен от случая 'this == &rhs'. player_activity.cpp 38
player_activity &player_activity:
perator=( const player_activity &rhs ){
тип = rhs.type;
....
targets.clear();
targets.reserve( rhs.targets.size() );
std::transform( rhs.targets.begin(),
rhs.targets.end(),
std::back_inserter( targets ),
[]( const item_location & e ) {
return e.clone();
} );
вернуть *это;
}
В этом коде также отсутствует проверка на самоприсваивание, и, кроме того, он содержит вектор, который необходимо заполнить. При такой реализации оператора присваивания, присвоение объекта самому себе приведет к удвоению вектора в поле targets , при этом некоторые элементы будут повреждены. Однако перед оператором transform следует оператор clear , который очистит вектор объекта, что приведет к потере данных.
Отступление III
В 2008 году жанру roguelike даже дали формальное определение, известное под эпическим названием «Берлинская интерпретация». Согласно ей, все подобные игры имеют следующие общие элементы:
- Мир генерируется случайным образом, что повышает реиграбельность;
- Перманентная смерть: если ваш персонаж умирает, он умирает навсегда, и все его вещи теряются;
- Пошаговый геймплей: любые изменения происходят только вслед за действиями игрока; течение времени приостанавливается до тех пор, пока игрок не совершит какое-либо действие;
- Выживание: ресурсы скудны.
Наконец, важнейшей особенностью roguelike-игр является сосредоточенность главным образом на исследовании мира, поиске новых способов использования предметов и исследовании подземелий.
В Cataclysm DDA довольно часто случается , что ваш персонаж замерзает до костей, умирает от голода и жажды, а вдобавок ко всему, его две ноги заменяются шестью щупальцами.
Важные детали
Пример 9:
V1028 Возможно переполнение. Рекомендуется преобразовывать операнды оператора 'start + larger' в тип 'size_t', а не результат. worldfactory.cpp 638
void worldfactory::draw_mod_list( int &start, .... )
{
....
int larger = ....;
unsigned int iNum = ....;
....
для( .... )
{
if( iNum >= static_cast<size_t>( start )
&& iNum < static_cast<size_t>( start + larger ) )
{
....
}
....
}
....
}
Похоже, программист хотел принять меры предосторожности против переполнения. Однако преобразование типа суммы ничего не изменит, поскольку переполнение произойдет раньше, на этапе сложения значений, и преобразование будет выполнено для бессмысленного значения. Чтобы этого избежать, только один из аргументов следует привести к более широкому типу: (static_cast<size_t> (start) + larger) .
Пример 10:
V530 Возвращаемое значение функции 'size' необходимо использовать. worldfactory.cpp 1340
bool worldfactory::world_need_lua_build( std::string world_name )
{
#ifndef LUA
....
#endif
// Предотвращает ошибку "неиспользуемая переменная" при включенных режимах LUA и RELEASE.
world_name.size();
вернуть false;
}
Для подобных случаев есть один приём. Если у вас осталась неиспользуемая переменная, и вы хотите подавить предупреждение компилятора, просто напишите (void)world_name вместо вызова методов этой переменной.
Пример 11:
V812 Снижение производительности. Неэффективное использование функции 'count'. Возможно, её можно заменить вызовом функции 'find'. player.cpp 9600
bool player::read( int inventory_position, const bool continuous )
{
....
player_activity activity;
если( !непрерывный
|| !std::all_of( learners.begin(),
learners.end(),
[&]( std:
air<npc *, std::string> elem ){
return std::count( activity.values.begin(),
activity.values.end(),
elem.first->getID() ) != 0;
} )
{
....
}
....
}
Тот факт, что значение count сравнивается с нулем, говорит о том, что программист хотел выяснить, содержит ли activity хотя бы один необходимый элемент. Но count приходится проходить по всему контейнеру, поскольку он подсчитывает все вхождения элемента. Эту задачу можно было бы выполнить быстрее, используя find , который останавливается после обнаружения первого вхождения.
Пример 12:
Эту ошибку легко обнаружить, если знать одну сложную деталь о типе данных char .
V739 Значение EOF не следует сравнивать со значением типа 'char'. Значение 'ch' должно быть типа 'int'. json.cpp 762
void JsonIn::skip_separator()
{
подписанный char ch;
....
if (ch == ',') {
if( ate_separator ) {
....
}
....
} else if (ch == EOF) {
....
}
Это одна из ошибок, которую нелегко заметить, если не знать, что EOF определяется как -1. Поэтому при сравнении с переменной типа signed char условие почти всегда будет ложным . Единственное исключение — символ с кодом 0xFF (255). При использовании в сравнении он станет -1, что сделает условие истинным.
Пример 13:
Эта небольшая ошибка может когда-нибудь стать критической. В конце концов, есть веские причины, по которым она находится в списке рассылки CWE под номером CWE-834 . Обратите внимание, что проект уже пять раз выдавал это предупреждение.
V663 Возможен бесконечный цикл. Условие 'cin.eof()' недостаточно для выхода из цикла. Рассмотрите возможность добавления вызова функции 'cin.fail()' в условное выражение. action.cpp 46
void parse_keymap(std::istream &keymap_txt, ....)
{
while( !keymap_txt.eof() ) {
....
}
}
Как указано в предупреждении, недостаточно проверять наличие конца файла при чтении из файла — необходимо также проверять наличие ошибки ввода, вызывая метод cin.fail() . Давайте исправим код, чтобы сделать его более безопасным:
while( !keymap_txt.eof() )
{
if(keymap_txt.fail())
{
keymap_txt.clear();
keymap_txt.ignore(numeric_limits<streamsize>::max(),'\n');
перерыв;
}
....
}
Функция keymap_txt.clear() предназначена для очистки состояния ошибки (флага) потока после возникновения ошибки чтения, чтобы вы могли прочитать остальную часть текста. Вызов функции keymap_txt.ignore с параметрами numeric_limits<streamsize>::max() и символом новой строки позволяет пропустить оставшуюся часть строки.
Есть гораздо более простой способ остановить чтение:
while( !keymap_txt )
{
....
}
Объяснить с
В логическом контексте поток будет преобразовывать себя в значение, эквивалентное true, до тех пор, пока не будет достигнут конец файла (EOF) .
Отступление IV
Самые популярные в наше время игры в жанре roguelike сочетают в себе элементы оригинальных roguelike-игр и других жанров, таких как платформеры, стратегии и так далее. Такие игры стали известны как «roguelike-like» или «roguelite». К ним относятся такие известные игры, как Don't Starve , The Binding of Isaac , FTL: Faster Than Light , Darkest Dungeon и даже Diablo .
Однако различие между roguelike и roguelite порой настолько незначительно, что невозможно с уверенностью сказать, к какой категории относится игра. Некоторые утверждают, что Dwarf Fortress не является roguelike в строгом смысле этого слова, в то время как другие считают Diablo классической игрой в жанре roguelike.
Заключение
Несмотря на то, что проект в целом оказался высококачественным, с небольшим количеством серьезных дефектов, это не означает, что он может обойтись без статического анализа. Сила статического анализа заключается в его регулярном использовании, а не в разовых проверках, подобных тем, которые мы проводим для популяризации. При регулярном использовании статические анализаторы помогают обнаруживать ошибки на самых ранних этапах разработки и, следовательно, удешевляют их исправление. Примеры расчетов .
!
