Стоило ли столько ждать, чтобы найти баг?

image1.png

Наверняка Вы задавались вопросом, чей код качественнее: у проекта с открытым кодом или закрытым? После прочтения нашего блога можно подумать, что все ошибки собрали проекты с открытым исходным кодом. Но это не совсем так. Ошибки есть во всех проектах, независимо от способа их хранения. А качество будет лучше там, где его повышают. Эта небольшая заметка о том, как в одном проекте исправляли баг 2 года, а могли бы сделать это за 5 минут.

Хронология событий


Minetest — это открытый кроссплатформенный игровой движок, содержащий около 200 тысяч строк кода на C, C++ и Lua. Он позволяет создавать разные игровые режимы в воксельном пространстве. Поддерживает мультиплеер, и множество модов от сообщества.

10 ноября 2018 года в багтрекере проекта отрыли Issue #7852item_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).
И скриншот:

image2.png

На скриншоте можно заметить незначительный выход картинок за границу внутренней области кнопок. Баг был замечен в далёком 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 10726Fix errors found by professional static code analyzer, где установили связь этого бага с Issue #7852. Это округление и искажало размеры кнопок.

Выводы


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

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

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


Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Did It Have to Take So Long to Find a Bug?.
@SvyatoslavMC
21.12.2020 10:08 UTC
Первоисточник

Комментарии

@Andrey_Epifantsev
21.12.2020 05:34 UTC
+6
В ваших статьях всё выглядит просто и понятно. В жизни, я подозреваю, всё не так просто. Статический анализатор — это расширенный набор предупреждений(warnings) сообщающий о потенциальных проблемах. Эдакие warnings пятого уровня. Обычно команды выбирают какой-то свой warning level(как правило 3 или 4) и правят предупреждения только на нём. Если на таком проекте поднять warning level на единичку, то сразу вывалиться огромное количество предупреждений, которые нужно будет исправить, или хотя бы посмотреть и понять насколько реальны проблемы, о которых они сообщают.
Аналогично, если подключить статический анализатор, то он тоже выдаст кучу предупреждений о потенциально опасном коде. Лишь некоторые из них являются ошибками.

Легко сказать «чего они ждали два года, это же так просто исправить этот warning». А в реальности этих ворнингов несколько сотен. И из них лишь пара являются ошибками. И нужна воля со стороны руководства проекта, чтобы потратить ресурсы на их исправление без видимой и измеримой отдачи.
@SvyatoslavMC
21.12.2020 05:45 UTC
+5
если подключить статический анализатор, то он тоже выдаст кучу предупреждений о потенциально опасном коде
Типичный проект без какого-либо контроля качества выглядит примерно так, как вы написали. Но это только первый запуск будет таким. На этот случай есть отключение выдачи предупреждений на существующий код. А на новом коде предупреждения уже появляются в приемлемом количестве. И есть возможность вернуться к техническому долгу.
@Andrey2008
21.12.2020 07:42 UTC
+1
Мой коллега уже дал ответ. Я же хочу добавить, что этот и релевантные моменты подробно изложены в статье "Как внедрить статический анализатор кода в legacy проект и не демотивировать команду".
@SvyatoslavMC
21.12.2020 05:45 UTC
0
DEL
@aamonster
21.12.2020 06:46 UTC
+2

Да, не зря Вирт в Паскале для целочисленного деления отдельный оператор сделал...

@NN1
21.12.2020 20:09 UTC
0

Python 3 аналогично: / и //.

@Phara0n
21.12.2020 17:30 UTC
+1
Немного оффтоп — когда ваш анализатор можно будет использовать с .NET 5 проектами?
@SvyatoslavMC
21.12.2020 17:32 UTC
0
Призываю foto_shooter для ответа)
@SergVasiliev
24.12.2020 06:14 UTC
+1
Добрый день!

Точных сроков пока нет. :(
Но Вы можете написать мне в ЛС свою почту, и как только мы поддержим .NET 5 проекты, я Вам напишу. :)
@alan008
21.12.2020 22:53 UTC
+1

А можете поделиться roadmap проекта PVS на следующий год? Как я понимаю, с точки зрения предупреждений для C/++ уже реализованы практически все мыслимые и немыслимые случаи, а Java и C# анализаторы не находят большой аудитории юзеров, т. к., во-первых, в этих языках сложнее допустить ошибку, а во-вторых, там есть другие, более привычные по сравнению с PVS анализаторы. Вопрос — куда планируете двигаться дальше?

@Andrey2008
23.12.2020 18:41 UTC
0
Что всё реализовано, это сииильное преувеличение :). Тем более, что язык развивается.
Мы напишем заметку на эту тему (roadmap).
24.12.2020 10:36 UTC
+1

Спасибо, очень интересно следить за развитием хорошего продукта.


язык развивается

Почему-то именно в контексте языка C/++ это звучит как "появилось еще больше способов выстрелить себе в ногу" :-) Шучу, кочечно.

24.12.2020 11:49 UTC
+1
@Andrey2008
08.02.2021 12:25 UTC
0
Затянул с оформлением материалов в виде статьи, но вот: Дорожная карта PVS-Studio на 2021 год.