# Ищем аномалии в X\-Ray Engine X\-Ray Engine \- игровой движок, который используется в играх серии S\.T\.A\.L\.K\.E\.R\. 16 сентября 2014 года его исходный код был выложен в открытый доступ, и с тех пор его развитием занимаются фанаты\. Большой размер проекта, огромное количество багов в играх \- всё это располагает к отличной демонстрации возможностей статического анализатора кода PVS\-Studio\. ![0405_XRay_ru/image1.png](https://import.viva64.com/docx/blog/0405_XRay_ru/image1.png) ## Вступление X\-Ray был создан украинской компанией GSC GameWorld для игры S\.T\.A\.L\.K\.E\.R\.: Тень Чернобыля\. Движок включает рендер с поддержкой DirectX 8\.1/9\.0c/10/10\.1/11, физический и звуковой движки, мультиплеер и систему искусственного интеллекта A\-Life\. Впоследствии компания создавала движок версии 2\.0 для своей новой игры, но разработка была прекращена и исходные коды утекли в сеть\. Проект вместе со всеми его зависимостями легко собирается в Visual Studio 2015\. Для проверки использовался исходный код движка версии 1\.6 из [репозитория на GitHub](https://github.com/openxray/xray-16) и статический анализатор кода PVS\-Studio 6\.04, загрузить который можно по [ссылке](https://pvs-studio.ru/ru/pvs-studio/download/)\. ## Copy\-paste Для начала рассмотрим ошибки, связанные с копированием кода\. Сценарий их возникновения в разных случаях обычно похож: скопировали код, поменяли часть переменных, а несколько \- забыли\. Такие ошибки могут быстро распространяться по кодовой базе, и без статического анализатора их очень легко пропустить\. ![0405_XRay_ru/image2.png](https://import.viva64.com/docx/blog/0405_XRay_ru/image2.png) ```cpp MxMatrix& MxQuadric::homogeneous(MxMatrix& H) const { .... unsigned int i, j; for(i=0; iline_exist(ppi_section,"color_base")) sscanf(ini->r_string(ppi_section,"color_base"), "%f,%f,%f", &data.m_attack_effector.ppi.color_base.r, &data.m_attack_effector.ppi.color_base.g, &data.m_attack_effector.ppi.color_base.b); if (ini->line_exist(ppi_section,"color_base")) sscanf(ini->r_string(ppi_section,"color_gray"), "%f,%f,%f", &data.m_attack_effector.ppi.color_gray.r, &data.m_attack_effector.ppi.color_gray.g, &data.m_attack_effector.ppi.color_gray.b); if (ini->line_exist(ppi_section,"color_base")) sscanf(ini->r_string(ppi_section,"color_add"), "%f,%f,%f", &data.m_attack_effector.ppi.color_add.r, &data.m_attack_effector.ppi.color_add.g, &data.m_attack_effector.ppi.color_add.b); .... } ``` **Предупреждения PVS\-Studio**: * [V581](https://pvs-studio.ru/ru/docs/warnings/v581/) The conditional expressions of the 'if' operators situated alongside each other are identical\. Check lines: 445, 447\. base\_monster\_startup\.cpp 447 * [V581](https://pvs-studio.ru/ru/docs/warnings/v581/) The conditional expressions of the 'if' operators situated alongside each other are identical\. Check lines: 447, 449\. base\_monster\_startup\.cpp 449 В данном фрагменте используются подряд несколько одинаковых условных выражений\. Очевидно, что необходимо заменить _color\_base _на_ color\_gray _и_ color\_add _в соответствии с кодом в теле _if_ ветви_\._ ```cpp /* process a single statement */ static void ProcessStatement(char *buff, int len) { .... if (strncmp(buff,"\\pauthr\\",8) == 0) { ProcessPlayerAuth(buff, len); } else if (strncmp(buff,"\\getpidr\\",9) == 0) { ProcessGetPid(buff, len); } else if (strncmp(buff,"\\getpidr\\",9) == 0) { ProcessGetPid(buff, len); } else if (strncmp(buff,"\\getpdr\\",8) == 0) { ProcessGetData(buff, len); } else if (strncmp(buff,"\\setpdr\\",8) == 0) { ProcessSetData(buff, len); } } ``` **Предупреждение PVS\-Studio**: [V517](https://pvs-studio.ru/ru/docs/warnings/v517/) The use of 'if \(A\) \{\.\.\.\} else if \(A\) \{\.\.\.\}' pattern was detected\. There is a probability of logical error presence\. Check lines: 1502, 1505\. gstats\.c 1502 Как и в предыдущем примере, здесь используются два одинаковых условия \(_strncmp\(buff,"\\\\getpidr\\\\",9\) \=\= 0_\)\. Сложно сказать наверняка, является ли это ошибкой или просто недостижимым кодом, но на это точно стоит обратить внимание\. Возможно, что здесь должны быть блоки с _getpidr_/_setpidr_ по аналогии с _getpdr_/_setpdr_\. ```cpp class RGBAMipMappedCubeMap { .... size_t height() const { return cubeFaces[0].height(); } size_t width() const { return cubeFaces[0].height(); } .... }; ``` **Предупреждение PVS\-Studio**: [V524](https://pvs-studio.ru/ru/docs/warnings/v524/) It is odd that the body of 'width' function is fully equivalent to the body of 'height' function\. tpixel\.h 1090 Методы _height\(\)_ и _width\(\)_ имеют одинаковое тело\. Учитывая, что вычисляются размеры граней куба, возможно, ошибки здесь нет\. Но лучше переписать метод _width\(\)_ следующим образом: ```cpp size_t width() const { return cubeFaces[0].width(); } ``` ## Неправильное использование C\+\+ C\+\+ \- замечательный язык, который предоставляет программисту много возможностей\.\.\. отстрелить себе ногу особо жестоким образом\. Неопределённое поведение, утечки памяти и, конечно же, опечатки \- об ошибках такого рода пойдёт речь в текущем разделе\. ![0405_XRay_ru/image3.png](https://import.viva64.com/docx/blog/0405_XRay_ru/image3.png) ```cpp template struct _matrix33 { public: typedef _matrix33Self; typedef Self& SelfRef; .... IC SelfRef sMTxV(Tvector& R, float s1, const Tvector& V1) const { R.x = s1*(m[0][0] * V1.x + m[1][0] * V1.y + m[2][0] * V1.z); R.y = s1*(m[0][1] * V1.x + m[1][1] * V1.y + m[2][1] * V1.z); R.z = s1*(m[0][2] * V1.x + m[1][2] * V1.y + m[2][2] * V1.z); } .... } ``` **Предупреждение PVS\-Studio**: [V591](https://pvs-studio.ru/ru/docs/warnings/v591/) Non\-void function should return a value\. \_matrix33\.h 435 В конце метода пропущен _return \*this_\. По стандарту подобный код приведёт к неопределённому поведению\. Так как возвращаемое значение является ссылкой, это, скорее всего, приведёт к падению программы при попытке обратиться к возвращаемому значению\. ```cpp ETOOLS_API int __stdcall ogg_enc(....) { .... FILE *in, *out = NULL; .... input_format *format; .... in = fopen(in_fn, "rb"); if(in == NULL) return 0; format = open_audio_file(in, &enc_opts); if(!format){ fclose(in); return 0; }; out = fopen(out_fn, "wb"); if(out == NULL){ fclose(out); return 0; } .... } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The null pointer is passed into 'fclose' function\. Inspect the first argument\. ogg\_enc\.cpp 47 Довольно интересный пример\. Анализатор обнаружил, что аргумент в вызове _fclose _равен _nullptr_, что делает вызов функции бессмысленным\. Можно предположить, что должны были закрыть поток _in\._ ```cpp void NVI_Image::ABGR8_To_ARGB8() { // swaps RGB for all pixels assert(IsDataValid()); assert(GetBytesPerPixel() == 4); UINT hxw = GetNumPixels(); for (UINT i = 0; i < hxw; i++) { DWORD col; GetPixel_ARGB8(&col, i); DWORD a = (col >> 24) && 0x000000FF; DWORD b = (col >> 16) && 0x000000FF; DWORD g = (col >> 8) && 0x000000FF; DWORD r = (col >> 0) && 0x000000FF; col = (a << 24) | (r << 16) | (g << 8) | b; SetPixel_ARGB8(i, col); } } ``` **Предупреждения PVS\-Studio:** * [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: 0x000000FF\. nvi\_image\.cpp 170 * [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: 0x000000FF\. nvi\_image\.cpp 171 * [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: 0x000000FF\. nvi\_image\.cpp 172 * [V560](https://pvs-studio.ru/ru/docs/warnings/v560/) A part of conditional expression is always true: 0x000000FF\. nvi\_image\.cpp 173 В данном участке кода перепутаны логические и битовые операции\. Результат будет не таким, какого ожидал программист: _col_ будет всегда равен 0x01010101 независимо от входных данных\. Правильный вариант: ```cpp DWORD a = (col >> 24) & 0x000000FF; DWORD b = (col >> 16) & 0x000000FF; DWORD g = (col >> 8) & 0x000000FF; DWORD r = (col >> 0) & 0x000000FF; ``` Ещё один пример странного кода: ```cpp VertexCache::VertexCache() { VertexCache(16); } ``` **Предупреждение PVS\-Studio**: [V603](https://pvs-studio.ru/ru/docs/warnings/v603/) The object was created but it is not being used\. If you wish to call constructor, 'this\-\>VertexCache::VertexCache\(\.\.\.\.\)' should be used\. vertexcache\.cpp 6 Вместо вызова одного конструктора из другого для инициализации экземпляра будет создан и тут же уничтожен новый объект типа _VertexCache_\. В результате члены создаваемого объекта останутся непроинициализированными\. ```cpp BOOL CActor::net_Spawn(CSE_Abstract* DC) { .... m_States.empty(); .... } ``` **Предупреждение PVS\-Studio**: [V530](https://pvs-studio.ru/ru/docs/warnings/v530/) The return value of function 'empty' is required to be utilized\. actor\_network\.cpp 657 Анализатор предупреждает, что возвращаемое функцией значение не используется\. Похоже, что программист перепутал методы _empty\(\)_ и _clear\(\)_: _empty\(\)_ не очищает массив, а проверяет, является ли он пустым\. Такие ошибки [нередко встречаются](https://pvs-studio.ru/ru/blog/examples/v530/) в различных проектах\. Проблема в том, что имя _empty\(\)_ не очевидно: некоторые воспринимают его как действие \- удаление\. Для того, чтобы подобной неоднозначности не возникало лучше добавлять глаголы has, is к началу метода: действительно, _isEmpty\(\)_ с _clear\(\)_ сложно перепутать\. Похожее предупреждение: V530 The return value of function 'unique' is required to be utilized\. uidragdroplistex\.cpp 780 ```cpp size_t xrDebug::BuildStackTrace(EXCEPTION_POINTERS* exPtrs, char *buffer, size_t capacity, size_t lineCapacity) { memset(buffer, capacity*lineCapacity, 0); .... } ``` **Предупреждение PVS\-Studio**: [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) The 'memset' function processes '0' elements\. Inspect the third argument\. xrdebug\.cpp 104 При вызове _memset_ аргументы перепутали местами и в итоге буфер не обнуляется, как изначально задумывалось\. Подобная ошибка может жить в проекте очень долго, так как её очень трудно обнаружить\. В таких местах на выручку программисту приходит статический анализатор\. Корректное использование _memset_: ```cpp memset(buffer, 0, capacity*lineCapacity); ``` Следующая ошибка связана с неправильно составленным логическим выражением\. ```cpp void configs_dumper::dumper_thread(void* my_ptr) { .... DWORD wait_result = WaitForSingleObject( this_ptr->m_make_start_event, INFINITE); while ( wait_result != WAIT_ABANDONED) || (wait_result != WAIT_FAILED)) .... } ``` **Предупреждение PVS\-Studio**: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression is always true\. Probably the '&&' operator should be used here\. configs\_dumper\.cpp 262 Выражения вида _x \!\= a \|\| x \!\= b_ всегда являются истинным\. Вероятнее всего вместо оператора \|\| подразумевался оператор &&\. Подробнее об ошибках в логических выражениях можно прочитать в статье "[Логические выражения в C/C\+\+\. Как ошибаются профессионалы](https://pvs-studio.ru/ru/blog/posts/cpp/0390/)"\. ```cpp void SBoneProtections::reload(const shared_str& bone_sect, IKinematics* kinematics) { .... CInifile::Sect &protections = pSettings->r_section(bone_sect); for (CInifile::SectCIt i=protections.Data.begin(); protections.Data.end() != i; ++i) { string256 buffer; BoneProtection BP; .... BP.BonePassBullet = (BOOL) ( atoi( _GetItem(i->second.c_str(), 2, buffer) )>0.5f); .... } } ``` **Предупреждение PVS\-Studio**: [V674](https://pvs-studio.ru/ru/docs/warnings/v674/) The '0\.5f' literal of the 'float' type is compared to a value of the 'int' type\. boneprotections\.cpp 54 Анализатор обнаружил сравнение целочисленного значения с вещественной константой\. Возможно, что здесь по аналогии должна была использоваться функция _atof_, а не _atoi_, в другом случае стоит переписать это сравнение, чтобы оно не выглядело подозрительно\. Однако сказать наверняка, является ли этот пример ошибочным или нет, может только разработчик, писавший его\. ```cpp class IGameObject : public virtual IFactoryObject, public virtual ISpatial, public virtual ISheduled, public virtual IRenderable, public virtual ICollidable { public: .... virtual u16 ID() const = 0; .... } BOOL CBulletManager::test_callback( const collide::ray_defs& rd, IGameObject* object, LPVOID params) { bullet_test_callback_data* pData = (bullet_test_callback_data*)params; SBullet* bullet = pData->pBullet; if( (object->ID() == bullet->parent_id) && (bullet->fly_distflags.ricochet_was)) return FALSE; BOOL bRes = TRUE; if (object){ .... } return bRes; } ``` **Предупреждение PVS\-Studio**: [V595](https://pvs-studio.ru/ru/docs/warnings/v595/) The 'object' pointer was utilized before it was verified against nullptr\. Check lines: 42, 47\. level\_bullet\_manager\_firetrace\.cpp 42 Проверка указателя _object _на равенство _nullptr_ идёт после того, как разыменовали _object\-\>ID\(\)_\. В случае, когда _object_ равен nullptr, это приведёт к падению программы\. ```cpp #ifdef _EDITOR BOOL WINAPI DllEntryPoint(....) #else BOOL WINAPI DllMain(....) #endif { switch (ul_reason_for_call) { .... case DLL_THREAD_ATTACH: if (!strstr(GetCommandLine(), "-editor")) CoInitializeEx(NULL, COINIT_MULTITHREADED); timeBeginPeriod(1); break; .... } return TRUE; } ``` **Предупреждение PVS\-Studio**: [V718](https://pvs-studio.ru/ru/docs/warnings/v718/) The 'CoInitializeEx' function should not be called from 'DllMain' function\. xrcore\.cpp 205 В теле _DllMain_ нельзя использовать часть WinAPI функций, включая _CoInitializeEx\._ Убедиться в этом можно, прочитав [документацию на MSDN](https://msdn.microsoft.com/en-us/library/windows/desktop/ms695279%28v=vs.85%29.aspx)\. Нельзя дать какой\-то однозначный совет, как стоит переписать эту функцию, но стоит понимать, что такая ситуация опасна, так как она может привести к взаимной блокировке потоков или аварийному завершению\. ## Ошибки в приоритетах ```cpp int sgetI1( unsigned char **bp ) { int i; if ( flen == FLEN_ERROR ) return 0; i = **bp; if ( i > 127 ) i -= 256; flen += 1; *bp++; return i; } ``` **Предупреждение PVS\-Studio**: [V532](https://pvs-studio.ru/ru/docs/warnings/v532/) Consider inspecting the statement of '\*pointer\+\+' pattern\. Probably meant: '\(\*pointer\)\+\+'\. lwio\.c 316 Ошибка связана с использованием инкремента\. Для наглядности перепишем данное выражение, расставив скобки: ```cpp *(bp++); ``` То есть произойдёт сдвиг не содержимого по адресу _bp,_ а самого указателя, что в данном контексте бессмысленно\. Ниже по коду есть фрагменты вида _\*bp \+\= N_, из\-за чего я и сделал вывод, что это ошибка\. Избежать подобной ошибки помогла бы расстановка скобок, что сделало бы порядок вычислений более понятным\. Также неплохой методикой является использование _const_ для аргументов, которые не должны меняться\. Аналогичные предупреждения: * V532 Consider inspecting the statement of '\*pointer\+\+' pattern\. Probably meant: '\(\*pointer\)\+\+'\. lwio\.c 354 * V532 Consider inspecting the statement of '\*pointer\+\+' pattern\. Probably meant: '\(\*pointer\)\+\+'\. lwob\.c 80 ```cpp void CHitMemoryManager::load (IReader &packet) { .... if (!spawn_callback || !spawn_callback->m_object_callback) if(!g_dedicated_server) Level().client_spawn_manager().add( delayed_object.m_object_id,m_object->ID(),callback); #ifdef DEBUG else { if (spawn_callback && spawn_callback->m_object_callback) { VERIFY(spawn_callback->m_object_callback == callback); } } #endif // DEBUG } ``` **Предупреждение PVS\-Studio**: [V563](https://pvs-studio.ru/ru/docs/warnings/v563/) It is possible that this 'else' branch must apply to the previous 'if' statement\. hit\_memory\_manager\.cpp 368 В этом фрагменте ветвь _else_ относится ко второму _if_ из\-за своей право\-ассоциативности, что не совпадает с форматированием кода\. К счастью, данный случай не отражается на работе программы, тем не менее, он может усложнить процесс отладки и тестирования\. Рекомендация проста \- в более\-менее сложных ветвлениях расставляйте фигурные скобки\. ```cpp void HUD_SOUND_ITEM::PlaySound(HUD_SOUND_ITEM& hud_snd, const Fvector& position, const IGameObject* parent, bool b_hud_mode, bool looped, u8 index) { .... hud_snd.m_activeSnd->snd.set_volume( hud_snd.m_activeSnd->volume * b_hud_mode?psHUDSoundVolume:1.0f); } ``` **Предупреждение PVS\-Studio**: [V502](https://pvs-studio.ru/ru/docs/warnings/v502/) Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\*' operator\. hudsound\.cpp 108 У тернарного условного оператора приоритет ниже, чем у умножения, поэтому порядок операций будет следующим: ```cpp (hud_snd.m_activeSnd->volume * b_hud_mode)?psHUDSoundVolume:1.0f ``` Очевидно, что правильный код должен выглядеть так: ```cpp hud_snd.m_activeSnd->volume * (b_hud_mode?psHUDSoundVolume:1.0f) ``` Выражения, содержащие тернарный оператор, несколько _if\-else_ ветвей или операции И/ИЛИ, \- это те случаи, когда лучше поставить лишние скобки\. Аналогичные предупреждения: * V502 Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\+' operator\. uihudstateswnd\.cpp 487 * V502 Perhaps the '?:' operator works in a different way than it was expected\. The '?:' operator has a lower priority than the '\+' operator\. uicellcustomitems\.cpp 106 ## Лишние сравнения ```cpp void CDestroyablePhysicsObject::OnChangeVisual() { if (m_pPhysicsShell){ if(m_pPhysicsShell)m_pPhysicsShell->Deactivate(); .... } .... } ``` **Предупреждение PVS\-Studio**: [V571](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'if \(m\_pPhysicsShell\)' condition was already verified in line 32\. destroyablephysicsobject\.cpp 33 В данном примере дважды проверяется _m\_pPhysicsShell_\. Скорее всего, вторая проверка лишняя\. ```cpp void CSE_ALifeItemPDA::STATE_Read(NET_Packet &tNetPacket, u16 size) { .... if (m_wVersion > 89) if ( (m_wVersion > 89)&&(m_wVersion < 98) ) { .... }else{ .... } } ``` **Предупреждение PVS\-Studio**: [V571](https://pvs-studio.ru/ru/docs/warnings/v571/) Recurring check\. The 'm\_wVersion \> 89' condition was already verified in line 987\. xrserver\_objects\_alife\_items\.cpp 989 Очень странный код\. То ли здесь забыли выражение после _if \(m\_wVersion \> 89\)_, то ли целую серию _else\-if_\. Данный метод требует более подробного рассмотрения разработчиком проекта\. ```cpp void ELogCallback(void *context, LPCSTR txt) { .... bool bDlg = ('#'==txt[0])||((0!=txt[1])&&('#'==txt[1])); if (bDlg){ int mt = ('!'==txt[0])||((0!=txt[1])&&('!'==txt[1]))?1:0; .... } } ``` **Предупреждения PVS\-Studio**: * [V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting the '\(0 \!\= txt\[1\]\) && \('\#' \=\= txt\[1\]\)' expression\. The expression is excessive or contains a misprint\. elog\.cpp 29 * [V590](https://pvs-studio.ru/ru/docs/warnings/v590/) Consider inspecting the '\(0 \!\= txt\[1\]\) && \('\!' \=\= txt\[1\]\)' expression\. The expression is excessive or contains a misprint\. elog\.cpp 31 В выражениях инициализации переменных _bDlg_ и _mt_ проверка _\(0 \!\= txt\[1\]\)_ является избыточной\. Если её опустить, выражения станут читаться значительно легче: ```cpp bool bDlg = ('#'==txt[0])||('#'==txt[1]); int mt = ('!'==txt[0])||('!'==txt[1])?1:0; ``` ## Ошибки в типах данных ![0405_XRay_ru/image4.png](https://import.viva64.com/docx/blog/0405_XRay_ru/image4.png) ```cpp float CRenderTarget::im_noise_time; CRenderTarget::CRenderTarget() { .... param_blur = 0.f; param_gray = 0.f; param_noise = 0.f; param_duality_h = 0.f; param_duality_v = 0.f; param_noise_fps = 25.f; param_noise_scale = 1.f; im_noise_time = 1/100; im_noise_shift_w = 0; im_noise_shift_h = 0; .... } ``` **Предупреждение PVS\-Studio**: [V636](https://pvs-studio.ru/ru/docs/warnings/v636/) The '1 / 100' 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;\. gl\_rendertarget\.cpp 245 Значение выражения 1/100 равно 0, так как выполняется операция целочисленного деления\. Чтобы получить значение 0\.01f, нужно использовать вещественный литерал, переписав выражение: 1/100\.0f\. Хотя возможно, что данное поведение было предусмотрено автором, и ошибки здесь нет\. ```cpp CSpaceRestriction::merge(....) const { .... LPSTR S = xr_alloc(acc_length); for ( ; I != E; ++I) temp = strconcat(sizeof(S),S,*temp,",",*(*I)->name()); .... } ``` **Предупреждение PVS\-Studio**: [V579](https://pvs-studio.ru/ru/docs/warnings/v579/) The strconcat function receives the pointer and its size as arguments\. It is possibly a mistake\. Inspect the first argument\. space\_restriction\.cpp 201 Функция _strconcat_, в качестве первого параметра принимает длину буфера\. Буфер _S_ объявлен, как _LPSTR_, то есть как указатель на строку\. _sizeof\(S\)_ будет равен размеру указателя в байтах, то есть _sizeof\(char \*\)_, а не количеству символов в строке\. Для вычисления длины следует использовать _strlen\(S\)_\. ```cpp class XRCDB_API MODEL { .... u32 status; // 0=ready, 1=init, 2=building .... } void MODEL::build (Fvector* V, int Vcnt, TRI* T, int Tcnt, build_callback* bc, void* bcp) { .... BTHREAD_params P = { this, V, Vcnt, T, Tcnt, bc, bcp }; thread_spawn(build_thread,"CDB-construction",0,&P); while (S_INIT == status) Sleep(5); .... } ``` **Предупреждение PVS\-Studio**: [V712](https://pvs-studio.ru/ru/docs/warnings/v712/) Be advised that compiler may delete this cycle or make it infinity\. Use volatile variable\(s\) or synchronization primitives to avoid this\. xrcdb\.cpp 100 Компилятор может убрать проверку _S\_INIT \=\= status_ в качестве оптимизации, так как переменная _status_ не модифицируется в цикле\. Для того, чтобы избежать подобного поведения, нужно использовать _volatile_ переменные или типы синхронизации данных между потоками\. Аналогичные предупреждения: * V712 Be advised that compiler may delete this cycle or make it infinity\. Use volatile variable\(s\) or synchronization primitives to avoid this\. levelcompilerloggerwindow\.cpp 23 * V712 Be advised that compiler may delete this cycle or make it infinity\. Use volatile variable\(s\) or synchronization primitives to avoid this\. levelcompilerloggerwindow\.cpp 232 ```cpp void CAI_Rat::UpdateCL() { .... if (!Useful()) { inherited::UpdateCL (); Exec_Look (Device.fTimeDelta); CMonsterSquad *squad = monster_squad().get_squad(this); if (squad && ((squad->GetLeader() != this && !squad->GetLeader()->g_Alive()) || squad->get_index(this) == u32(-1))) squad->SetLeader(this); .... } .... } ``` **Предупреждение PVS\-Studio**: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'squad\-\>get\_index\(this\) \=\= u32\(\- 1\)' is always false\. The value range of unsigned char type: \[0, 255\]\. ai\_rat\.cpp 480 Для того, чтобы понять, почему это выражение всегда ложно, вычислим значения отдельных операндов\. u32\(\-1\) равен 0xFFFFFFFF или 4294967295\. Тип, возвращаемый методом _squad\-\>get\_index\(\.\.\.\.\)_, \- u8, следовательно его максимальное значение \- 0xFF или 255, что строго меньше, чем u32\(\-1\)\. Соответственно, значением такого сравнения всегда будет _false_\. Данный код легко исправить, поменяв тип данных на u8: ```cpp squad->get_index(this) == u8(-1) ``` Та же диагностика срабатывает и для избыточных сравнений беззнаковых переменных: ```cpp namespace ALife { typedef u64 _TIME_ID; } ALife::_TIME_ID CScriptActionCondition::m_tLifeTime; IC bool CScriptEntityAction::CheckIfTimeOver() { return((m_tActionCondition.m_tLifeTime >= 0) && ((m_tActionCondition.m_tStartTime + m_tActionCondition.m_tLifeTime) < Device.dwTimeGlobal)); } ``` **Предупреждение PVS\-Studio**: [V547](https://pvs-studio.ru/ru/docs/warnings/v547/) Expression 'm\_tActionCondition\.m\_tLifeTime \>\= 0' is always true\. Unsigned type value is always \>\= 0\. script\_entity\_action\_inline\.h 115 Переменная _m\_tLifeTime_ является беззнаковой, соответственно она всегда больше или равна нулю\. Является ли это лишней проверкой или же здесь скрыта ошибка в логике, судить разработчику\. Аналогичное предупреждение: V547 Expression 'm\_tActionCondition\.m\_tLifeTime < 0' is always false\. Unsigned type value is never < 0\. script\_entity\_action\_inline\.h 143 ```cpp ObjectFactory::ServerObjectBaseClass * CObjectItemScript::server_object (LPCSTR section) const { ObjectFactory::ServerObjectBaseClass *object = nullptr; try { object = m_server_creator(section); } catch(std::exception e) { Msg("Exception [%s] raised while creating server object from " "section [%s]", e.what(),section); return (0); } .... } ``` **Предупреждение PVS\-Studio**: [V746](https://pvs-studio.ru/ru/docs/warnings/v746/) Type slicing\. An exception should be caught by reference rather than by value\. object\_item\_script\.cpp 39 Функция _std::exception::what\(\) _является виртуальной и может быть переопределена в наследуемых классах\. В данном примере исключение ловится по значению, следовательно, экземпляр класса будет скопирован и вся информация о полиморфном типе будет потеряна\. Обращаться к _what\(\)_ в таком случае бессмысленно\. Исключение стоит перехватывать по ссылке: ```cpp catch(const std::exception& e) { ``` ## Разное ```cpp void compute_cover_value (....) { .... float value [8]; .... if (value[0] < .999f) { value[0] = value[0]; } .... } ``` **Предупреждение PVS\-Studio**: [V570](https://pvs-studio.ru/ru/docs/warnings/v570/) The 'value\[0\]' variable is assigned to itself\. compiler\_cover\.cpp 260 Переменная _value\[0\]_ присваивается сама себе\. Зачем это делать \- непонятно\. Возможно, ей должно было быть присвоено другое значение\. ```cpp void CActor::g_SetSprintAnimation(u32 mstate_rl, MotionID &head, MotionID &torso, MotionID &legs) { SActorSprintState& sprint = m_anims->m_sprint; bool jump = (mstate_rl&mcFall) || (mstate_rl&mcLanding) || (mstate_rl&mcLanding) || (mstate_rl&mcLanding2) || (mstate_rl&mcJump); .... } ``` **Предупреждение PVS\-Studio**: [V501](https://pvs-studio.ru/ru/docs/warnings/v501/) There are identical sub\-expressions '\(mstate\_rl & mcLanding\)' to the left and to the right of the '\|\|' operator\. actoranimation\.cpp 290 Вероятнее всего здесь просто лишняя проверка _mstate\_rl & mcLanding_, но часто подобные предупреждения сигнализируют об ошибке в логике и нерассмотренных значениях enum\. Аналогичные предупреждения: * V501 There are identical sub\-expressions 'HudItemData\(\)' to the left and to the right of the '&&' operator\. huditem\.cpp 338 * V501 There are identical sub\-expressions 'list\_idx \=\= e\_outfit' to the left and to the right of the '\|\|' operator\. uimptradewnd\_misc\.cpp 392 * V501 There are identical sub\-expressions '\(D3DFMT\_UNKNOWN \=\= fTarget\)' to the left and to the right of the '\|\|' operator\. hw\.cpp 312 ```cpp RELATION_REGISTRY::RELATION_MAP_SPOTS::RELATION_MAP_SPOTS() { .... spot_names[ALife::eRelationTypeWorstEnemy] = "enemy_location"; spot_names[ALife::eRelationTypeWorstEnemy] = "enemy_location"; .... } ``` **Предупреждение PVS\-Studio**: [V519](https://pvs-studio.ru/ru/docs/warnings/v519/) The variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 57, 58\. relation\_registry\.cpp 58 Анализатор обнаружил, что одной переменной присваиваются подряд два значения\. В данном случае похоже, что это просто мёртвый код и его стоит удалить\. ```cpp void safe_verify(....) { .... printf("FATAL ERROR (%s): failed to verify data\n"); .... } ``` **Предупреждение PVS\-Studio**: [V576](https://pvs-studio.ru/ru/docs/warnings/v576/) Incorrect format\. A different number of actual arguments is expected while calling 'printf' function\. Expected: 2\. Present: 1\. entry\_point\.cpp 41 В функцию _printf_ передаётся недостаточное количество аргументов: формат '%s' указывает на то, что должен быть передан указатель на строку\. Такая ситуация может привести к ошибке доступа к памяти и экстренному завершению программы\. ## Заключение ![0405_XRay_ru/image5.png](https://import.viva64.com/docx/blog/0405_XRay_ru/image5.png) Проверка исходного кода X\-Ray Engine выявила большое количество как лишнего или подозрительного кода, так и однозначно ошибочных и опасных моментов\. Стоит отметить, что статический анализатор отлично помогает выявлять ошибки на раннем этапе разработки, что значительно облегчает жизнь программисту и освобождает время на создание новых версий приложения\.