Стоило ли столько ждать, чтобы найти баг?
Наверняка Вы задавались вопросом, чей код качественнее: у проекта с открытым кодом или закрытым? После прочтения нашего блога можно подумать, что все ошибки собрали проекты с открытым исходным кодом. Но это не совсем так. Ошибки есть во всех проектах, независимо от способа их хранения. А качество будет лучше там, где его повышают. Эта небольшая заметка о том, как в одном проекте исправляли баг 2 года, а могли бы сделать это за 5 минут.
Хронология событий
Minetest - это открытый кроссплатформенный игровой движок, содержащий около 200 тысяч строк кода на C, C++ и Lua. Он позволяет создавать разные игровые режимы в воксельном пространстве. Поддерживает мультиплеер, и множество модов от сообщества.
10 ноября 2018 года в багтрекере проекта отрыли Issue #7852 - item_image_button[]: button too small.
Описание следующее:
The button is too small resulting in the image exceeding its borders. Button should be the same size as inventory slots. See example below (using width and height of 1).
И скриншот:
На скриншоте можно заметить незначительный выход картинок за границу внутренней области кнопок. Баг был замечен в далёком 2018 году, а причину нашли только сейчас – в 2020.
Следующим событием в этой замечательной истории стала публикация технической статьи "PVS-Studio: Анализ pull request-ов в Azure DevOps при помощи self-hosted агентов" в июле 2020 года. Чтобы привести пример интеграции анализатора в Azure DevOps, был выбрана та самая игра – minetest. В статье приведено несколько найденных ошибок, но нам интересна одна конкретная из них:
V636 The 'rect.getHeight() / 16' expression was implicitly cast from 'int' type to 'float' type. Consider utilizing an explicit type cast to avoid the loss of a fractional part. An example: double A = (double)(X) / Y;. hud.cpp 771
void drawItemStack(....)
{
float barheight = rect.getHeight() / 16;
float barpad_x = rect.getWidth() / 16;
float barpad_y = rect.getHeight() / 16;
core::rect<s32> progressrect(
rect.UpperLeftCorner.X + barpad_x,
rect.LowerRightCorner.Y - barpad_y - barheight,
rect.LowerRightCorner.X - barpad_x,
rect.LowerRightCorner.Y - barpad_y);
}При делении значений ширины и высоты на 16 дробная часть результата отбрасывается, т.к. деление целочисленное.
И вот, спустя полгода результаты анализа были замечены разработчиками игры, и создана Issue 10726 - Fix errors found by professional static code analyzer, где установили связь этого бага с Issue #7852. Это округление и искажало размеры кнопок.
Выводы
Использование статических анализаторов кода позволяет экономить очень много времени на выявлении ошибок в коде. Можно сколь угодно утверждать, что описанный баг незначительный, но наш опыт показывает, что это типичный жизненный цикл ошибки любой критичности.
Предположим, здесь был бы серьёзный баг. На его исправление бросили бы все силы и за час отладки нашли бы и исправили его. Но анализатор всё равно бы нашёл его за пару минут.
Таким образом, можно сделать вывод, что автоматические способы поиска ошибок приносят неоспоримую пользу разрабатываемому проекту. Такие инструменты, как PVS-Studio, необходимо рассматривать как дополнение к codereview с другими программистами, а не замену этого процесса.