# Проверяем Blender
Командный центр PVS\-Studio: "Как быстро летит время\.\.\. А ведь в этом году, второго января, Blender исполнилось 30 лет\! Как будто ещё вчера мы публиковали статью с разбором ошибок\.\.\. Как 8 лет назад? Надо срочно исправлять ситуацию\!"\.
## Несколько слов о проекте
Blender — проект, безусловно, культовый, и заслужил с течением лет всеобщий интерес и признание\. Случилось это благодаря тому факту, что с 2000 года, существуя как Open Source решение, он позволяет любому художнику, хотя бы малость знакомому с инструментом, воплотить в X\-Y\-Z\-axis\-реальность свои самые смелые фантазии\.
Также у него очень приятный и удобный интерфейс, с которым хочется работать\. В наличии такие рендеры как Eevee и Cycles, которые могут сделать красивое\. А если хочется ещё больше и разнообразнее, много чего можно загуглить и установить\. Вообще, для Blender существует категорически много разнообразных аддонов, которыми можно полностью закрыть все потребности\.
Однако\! Есть и ложка дёгтя в бочке мёда\. Все те, и я в том числе, кто пользуется Blender для прохождения курсов за 200 тысяч рублей, безусловно, в курсе, что инструмент может капризничать, особенно если не знаешь, что делаешь\.
Да, давным\-давно пару раз у меня Blender отваливался, благо присутствует автосохранение\. Именно тогда я понял: чтобы сюрпризов не случалось, надо как следует научиться пользоваться этим инструментом\. Благо YouTube содержит кучу бесплатных курсов и отдельных видео о том, как сделать ту или иную, э\.\.\. штуку\. Как уже было сказано, проект культовый\.
Так, о чём это я? Баги\! Не то чтобы всё постоянно отваливалось\. Нет\! Работает всё стабильно, и работать можно по 12\-14 часов в день, и всё хорошо\! Но время от времени ваша искорёженная во все стороны анимация как бы намекааает, что\-то не так\. Думаю, любой, кто когда\-либо пользовался или пользуется Blender, со мной согласится\. Особенно если не уследил за параметрами Transform, в частности Scale — это база, это знать надо\!
Коммит, на котором я клонировал проект: [76092ac](https://github.com/blender/blender/tree/91b4f9f1f609e1cd119dc41943cfb2c2276092ac)\.
Дисклеймер
Написание и публикация этой статьи не имеют цели обесценить труд программистов, занимающихся разработкой рассматриваемого продукта\. Мне он очень нравится\! Цель — популяризация статического анализатора, нахождение ошибок и передача отчёта разработчикам для улучшения качества кода проекта\.
## Результаты проверки
Перед разбором кода и предупреждений хочется отметить, что, разбирая тот или иной случай срабатывания диагностического правила в этом проекте, часто мне не всегда было понятно: баг это или фича\. Часто код очень запутан, разобраться в нём сложно и это требует времени\. Однако список ошибок составлен, разбор срабатываний произведён\. Можно с уверенностью сказать, что потраченного времени не жалко, потому что будет интересно\.
Так что далее, в присущей мне манере, представляю вам наиболее интересные участки кода Blender, в которых, по мнению статического анализатора PVS\-Studio, есть подозрительные места\.
Пристёгивайтесь, и да начнётся разбор\!
## Прозрачные баги
Вдохновляясь идеей прозрачного кода, я разобрал некоторые проблемные места кода Blender и открыл для себя концепцию прозрачных багов: "_Идея прозрачных багов проста\. Такого рода система прозрачна тогда, когда вы можете взглянуть на её код и понять, где баги, что они делают и как_"\.
**Фрагмент N1**
```cpp
void Cryptomatte::begin_sync()
{
const eViewLayerEEVEEPassType enabled_passes =
static_cast(
inst.film.enabled_passes_get() &
( EEVEE_RENDER_PASS_CRYPTOMATTE_OBJECT |
EEVEE_RENDER_PASS_CRYPTOMATTE_ASSET |
EEVEE_RENDER_PASS_CRYPTOMATTE_ASSET ) );
....
}
```
Предупреждение анализатора: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'EEVEE\_RENDER\_PASS\_CRYPTOMATTE\_ASSET' to the left and to the right of the '\|' operator\. eevee\_cryptomatte\.cc 18
Очевидный баг: два раза используется одна и та же константа _EEVEE\_RENDER\_PASS\_CRYPTOMATTE\_ASSET_ при получении побитовой маски\. Скорее всего, должна использоваться другая константа — _EEVEE\_RENDER\_PASS\_CRYPTOMATTE\_MATERIAL \= \(1 << 18\)_\.
**Фрагмент N2**
```cpp
void MeshFromGeometry::create_edges(Mesh *mesh)
{
....
for (int i = 0; i < tot_edges; ++i)
{
....
dst_edge[0] = mesh_geometry_.global_to_local_vertices_
.lookup_default(src_edge[0], 0);
dst_edge[1] = mesh_geometry_.global_to_local_vertices_
.lookup_default(src_edge[1], 0);
BLI_assert( dst_edge[0] < total_verts
&& dst_edge[0] < total_verts);
}
....
}
```
Предупреждение анализатора: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'dst\_edge\[0\] < total\_verts' to the left and to the right of the '&&' operator\. obj\_import\_mesh\.cc 266
Левое и правое подвыражения логического "И" в _BLI\_assert_ абсолютно одинаковы\. Скорее всего, во втором подвыражении должен был проверяться _dst\_edge\[1\]_ вместо _dst\_edge\[0\]_\.
**Фрагмент N3**
```cpp
static void get_nearest_fcurve_verts_list (bAnimContext *ac,
const int mval[2],
ListBase *matches)
{
....
filter = (ANIMFILTER_DATA_VISIBLE |
ANIMFILTER_CURVE_VISIBLE |
ANIMFILTER_FCURVESONLY |
ANIMFILTER_NODUPLIS |
ANIMFILTER_FCURVESONLY);
....
}
```
Предупреждение анализатора: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions 'ANIMFILTER\_FCURVESONLY' to the left and to the right of the '\|' operator\. graph\_select\.cc 178
Форматирование кода таблицей — как одна из форм искусства\. В этом примере всё очевидно\!
Концепция прозрачности багов работает\.
Ничего не надо объяснять\.
Это прекрасно\!
Кстати, подобных мест в коде проекта не много\. Могу лишь обратить внимание, что форматирование кода часто решает проблему, и прозрачные баги исправляются сразу же\.
## Проблемы при разыменовании
_"Интересным примером разыменования нулевых указателей в случае срабатывания диагностического правила [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) является тот факт, что разыменование — лишь последствие ошибки, а не причина возникновения"\._
Разыменование нулевых указателей — проблема довольно частая\. Особенно из\-за логических ошибок в условиях кода программы\. Как эти баги устроены? Как их найти? У нас с анализатором есть ответы на эти вопросы, а также парочка интересных примеров для нашего разбора\.
**Фрагмент N4**
```cpp
static bool gpencil_stroke_eraser_is_occluded (tGPsdata *p, bGPDlayer *gpl,
bGPDspoint *pt, const int x,
const int y)
{
Object *obact = (Object *)p->ownerPtr.data;
Brush *brush = p->brush;
Brush *eraser = p->eraser;
BrushGpencilSettings *gp_settings = nullptr;
if (brush->gpencil_tool == GPAINT_TOOL_ERASE)
{
gp_settings = brush->gpencil_settings;
}
else
if ((eraser != nullptr) & // <=
(eraser->gpencil_tool == GPAINT_TOOL_ERASE))
{
gp_settings = eraser->gpencil_settings;
}
if ((gp_settings != nullptr) &&
(gp_settings->flag & GP_BRUSH_OCCLUDE_ERASER) ) {
RegionView3D *rv3d = static_cast(p->region->regiondata);
....
return false;
}
```
Предупреждение анализатора: [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'eraser' might take place\. Check the bitwise operation\. gpencil\_paint\.cc 1429
Как мы можем заметить, с проверкой указателя и его разыменовыванием проблем нет\. Если, конечно, не обращать внимание на символ _&_, гордо разделяющий два подвыражения в скобках\. Конечно же, здесь должен быть использован двойной амперсант _&&_ как в условии ниже\. Однако код именно таков, каков он и есть: оператор побитового "И" исполнит также и правое подвыражение, которое разыменует потенциальный нулевой указатель\.
Кстати, в этом же файле присутствует ещё одно такое же срабатывание:
* V522 Dereferencing of the null pointer 'eraser' might take place\. Check the bitwise operation\. gpencil\_paint\.cc 1821
**Фрагмент N5**
```cpp
void ui_draw_popover_back (ARegion *region, uiStyle * /*style*/,
uiBlock *block, rcti *rect )
{
....
if (block)
{
float mval_origin[2] = {float(block->bounds_offset[0]),
float(block->bounds_offset[1])};
ui_window_to_block_fl (region, block, &mval_origin[0], &mval_origin[1]);
ui_draw_popover_back_impl (wt->wcol_theme, rect, block->direction,
U.widget_unit / block->aspect, mval_origin);
}
else
{
const float zoom = 1.0f / block->aspect; // <=
wt->state (wt, &STATE_INFO_NULL, UI_EMBOSS_UNDEFINED);
wt->draw_block (&wt->wcol, rect, 0, 0, zoom);
}
....
}
```
Предупреждение анализатора: [V522](https://pvs-studio.ru/ru/docs/warnings/v522/) Dereferencing of the null pointer 'block' might take place\. interface\_widgets\.cc 5294
В этом кусочке кода есть определённая проблема в логике\. Если _block \=\= nullptr_, то выполнится ветка _else_, в которой произойдёт гарантированное разыменование нулевого указателя _block\._
И такое срабатывание не одно:
* V522 Dereferencing of the null pointer 'em' might take place\. transform\.cc 2117
* V522 Dereferencing of the null pointer 'mesh' might take place\. MOD\_cloth\.cc 108
* V522 Dereferencing of the null pointer 'data\.mval\_fl' might take place\. editmesh\_select\.cc 801
## Условие\.\.\. результат никогда не меняется\.\.\.
_"Безумие — это точное повторение одного и того же действия\. Раз за разом, в надежде на изменение\. Это есть безумие"_, — Ваас, FarCry 3\.
Довольно символично: у нас есть баг, мы пытаемся вызвать функцию, а она не вызывается, код правильный, что же не так? Бегаем по одному и тому же коду уже полчаса и не понимаем, в чём дело\. Чудеса, да и только\! А потом, неожиданно для самих себя, находим проблему в том месте, где и не думали её встретить\. Ошибки в логике условий\. Потому что часто логику переусложняют, а в голову и мысли не приходит о том, что мы сделали что\-то не так\.
**Фрагмент N6**
```cpp
void BLI_threadpool_init(ListBase *threadbase,
void *(*do_thread)(void *),
int tot)
{
....
if (threadbase != nullptr && tot > 0)
{
....
if (tot > RE_MAX_THREAD)
{
tot = RE_MAX_THREAD;
}
else if (tot < 1) // <=
{
tot = 1;
}
....
}
....
}
```
Предупреждение анализатора: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'tot < 1' is always false\. threads\.cc 131
Тут всё просто: если заглянуть в первое условие, то мы найдём проверку _tot \> 0\. _При этом _tot_ имеет тип _int_, и если оно больше 0, то меньше 1 уже быть не может\. Соответственно, условие _tot < 1_ всегда будет _false_\. В итоге код можно упростить, полностью убрав эту проверку\.
**Фрагмент N7**
```cpp
enum
{
ALIGN_WORLD = 0,
ALIGN_VIEW,
ALIGN_CURSOR,
};
bool ED_object_add_generic_get_opts(bool *r_is_view_aligned, ....)
{
....
if (RNA_struct_property_is_set(op->ptr, "rotation"))
{
....
}
else
{
int alignment = ALIGN_WORLD;
PropertyRNA *prop = RNA_struct_find_property(op->ptr, "align");
if (RNA_property_is_set(op->ptr, prop))
{
*r_is_view_aligned = alignment == ALIGN_VIEW;
alignment = RNA_property_enum_get(op->ptr, prop);
}
}
....
}
```
Оставлю вам возможность самостоятельно догадаться, в чём здесь проблема\.
Предупреждение анализатора: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'alignment \=\= ALIGN\_VIEW' is always false\. object\_add\.cc 544
Так, о чём это я? Поводом, возможно, послужила опечатка, а убийцей был дворецкий\. Переменная _r\_is\_view\_aligned_ имеет тип _bool \*_, и проверка действительно всегда равна _false_\. Причём, скорее всего, выражение справа от оператора сравнения \=\= должно быть другим, или всё же так и задумано? Абсолютно непонятная ситуация, но главное, что проблему нашли, а там уже и до исправления недалеко\. Мелочь, а приятно\!
Дополнительные срабатывания:
* V547 Expression 'changed \=\= false' is always true\. editmesh\_tools\.cc 1456
* V547 Expression 'ob\-\>type \=\= OB\_GPENCIL\_LEGACY' is always true\. gpencil\_edit\.cc 130
* V547 Expression 'closure\_count \> i' is always true\. eevee\_raytrace\.cc 371
* V547 Expression 'retval \=\= OPERATOR\_FINISHED' is always false\. wm\_gizmo\_group\.cc 490
## Я хочу сыграть с вами в одну игру\.\.\. На внимательность
**Фрагмент N8**
```cpp
void BKE_gpencil_stroke_copy_settings(const bGPDstroke *gps_src,
bGPDstroke *gps_dst)
{
gps_dst->thickness = gps_src->thickness;
gps_dst->flag = gps_src->flag;
gps_dst->inittime = gps_src->inittime;
gps_dst->mat_nr = gps_src->mat_nr;
copy_v2_v2_short(gps_dst->caps, gps_src->caps);
gps_dst->hardness = gps_src->hardness;
copy_v2_v2(gps_dst->aspect_ratio, gps_src->aspect_ratio);
gps_dst->fill_opacity_fac = gps_dst->fill_opacity_fac; // <=
copy_v3_v3(gps_dst->boundbox_min, gps_src->boundbox_min);
copy_v3_v3(gps_dst->boundbox_max, gps_src->boundbox_max);
gps_dst->uv_rotation = gps_src->uv_rotation;
copy_v2_v2(gps_dst->uv_translation, gps_src->uv_translation);
gps_dst->uv_scale = gps_src->uv_scale;
gps_dst->select_index = gps_src->select_index;
copy_v4_v4(gps_dst->vert_color_fill, gps_src->vert_color_fill);
}
```
Предупреждение анализатора: [V570](https://pvs-studio.ru/ru/docs/warnings/v570/) The 'gps\_dst\-\>fill\_opacity\_fac' variable is assigned to itself\. gpencil\_legacy\.cc 1029
Переменная _gps\_dst\-\>fill\_opacity\_fac_ присваивается сама себе\. Может, так и было задумано? Нет\. Скорее всего, имеет место быть опечатка, и в правой части присвоения должно быть другое выражение\. Что\-то типа _gps\_src\-\>fill\_opacity\_fac_\. Проблема Copy/Paste?
**Фрагмент N9**
```cpp
static int gizmo_cage2d_modal(....)
{
....
if ((transform_flag & ED_GIZMO_CAGE_XFORM_FLAG_SCALE_UNIFORM) == 0)
{
const bool use_temp_uniform = (event->modifier & KM_SHIFT) != 0;
const bool changed = data->use_temp_uniform != use_temp_uniform;
data->use_temp_uniform = data->use_temp_uniform;
....
}
....
}
```
Предупреждение анализатора: [V570](https://pvs-studio.ru/ru/docs/warnings/v570/) The 'data\-\>use\_temp\_uniform' variable is assigned to itself\. cage2d\_gizmo\.cc 1094
И снова проблема Copy/Paste\. Скорее всего, в итоге всё должно выглядеть так:
```cpp
data->use_temp_uniform = use_temp_uniform;
```
**Фрагмент N10**
```cpp
static void space_text_update_drawcache(SpaceText *st,
const ARegion *region)
{
....
if (st->wordwrap)
{
....
if (drawcache->update)
{
drawcache->valid_tail = drawcache->valid_head = 0;
....
memmove(new_tail, old_tail, drawcache->valid_tail);
....
}
....
}
....
}
```
[V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The 'memmove' function processes '0' elements\. Inspect the third argument\. text\_draw\.cc 673
Странно, что мы пытаемся копировать из одной области памяти в другую область памяти 0 байт информации\.
**Фрагмент N11**
```cpp
void ui_but_value_set(uiBut *but, double value)
{
....
if (but->editval) {
value = *but->editval = value;
}
else
....
ui_but_update_select_flag(but, &value);
}
```
Предупреждение анализатора: [V570](https://pvs-studio.ru/ru/docs/warnings/v570/) The same value is assigned twice to the 'value' variable\. interface\.cc 2676
Как вы думаете, так было задумано, или это тоже опечатка? Оставлю вопрос открытым, знаете, как в плохих фильмах ужасов\.
## Больше неопределённого поведения богу неопределённого поведения\!
_"Ктулху фхтагн, дорогие мои\! И правда, кто ещё, как ни спящий, может подойти на роль бога неопределённого поведения? Однако будьте осторожнее в своих действиях, вы же не хотите его пробуждения раньше назначенного часа? В коде уже и так много хаоса\!"_
**Фрагмент N12**
```cpp
static void rigidbody_update_ob_array(RigidBodyWorld *rbw)
{
if (rbw->group == nullptr)
{
rbw->numbodies = 0;
rbw->objects = static_cast