Як команда PVS-Studio поліпшила код Unreal Engine

Наша компанія створює, просуває і продає статичний аналізатор коду PVS-Studio для C/C + + програмістів. Однак, наша взаємодія з клієнтами не обмежується виключно продажем їм ліцензій на продукт PVS-Studio. Наприклад, ми займаємося деякими контрактними роботами. В силу NDA зазвичай розповісти про них ми не можемо, та й цікавого оповідання не вийде. Назви проектів, в яких ми беремо участь, теж нічого не скажуть більшості наших читачів. Але в цей раз, назва якраз говорить про багато що. Ми попрацювали разом з компанією Epic Games над проектом Unreal Engine. Про це і буде наша розповідь.

Для просування статичного аналізатора коду PVS-Studio ми придумали цікавий формат статей. Ми перевіряємо відкриті проекти і пишемо про знайдені недоліки в коді. Ось тут можна глянути на ці статті: оновлюваний список. Від цих статей не всі у виграші. Читачам цікаво дивитися на чужі помилки. Заодно вони дізнаються для себе нові способи, як уникнути цих помилок, використовуючи якісь прийоми або стиль написання коду. Для нас користь у тому, що більше людей дізнається про існування PVS-Studio. Для авторів проектів теж користь — вони можуть виправити деякі баги.

Однією з таких статей була "Довгоочікувана перевірка Unreal Engine 4". Вихідний код Unreal Endine відрізнявся високою якістю, але як відомо, всі проекти по розробці програмного забезпечення містять помилки і PVS-Studio відмінно впорався із знаходженням цих помилок. Ми перевірили проект і відправили результати перевірки в Epic Games. Автори проекту сказали нам спасибі і поправили знайдені нами дефекти. Але нам цього було мало, і ми вирішили постаратися продати компанії Epic Games ліцензію на PVS-Studio.

Компанія Epic Games була зацікавлена у використанні PVS-Studio, щоб поліпшити свій движок. Таким чином, ми перевіряємо і правимо исходники Unreal Engine з таким розрахунком, щоб у коді не залишилося багів і при цьому аналізатор більше не видавав жодного помилкового спрацьовування. Після цього вони почнуть використовувати PVS-Studio на своїй кодової базі, звівши до мінімуму складність інтеграції аналізатора в свій процес розробки. У цьому випадку Epic Games погодилася оплатити не тільки ліцензію, але і додатково винагородити нас за виконану роботу.

Ми погодилися. Робота виконана. І тепер пропонуємо читачеві познайомитися з цікавими моментами, з якими ми зустрілися в процесі роботи з вихідними кодами Unreal Engine.

З боку PVS-Studio в проекті брали участь Павло Єремєєв, Святослав Размыслов, Антон Токарєв. З боку Epic Games найбільше допомоги ми отримали від Andy Bayle і Dan o'connor – без них наша робота була б неможлива, спасибі!

Інтеграція аналізу PVS-Studio в збірку Unreal Engine
Для складання в Unreal Engine використовується власна складальна система – Unreal Build Tool. Є також набір скриптів для генерації проектних файлів для різних платформ та компіляторів. Так як PVS-Studio орієнтована в першу чергу на роботу з компілятором Microsoft Visual C++, ми скористалися скриптом для отримання проектних файлів (*.vcxproj) для середовища Microsoft Visual Studio.

PVS-Studio має плагін, встраивающийся в середовище розробки Visual Studio і дозволяє виконати аналіз «в один клік». Однак, проекти, згенеровані для Unreal Engine, не є «звичайними» проектами MSBuild, які використовують Visual Studio.

Під час компіляції Unreal Engine в Visual Studio, середовище викликає MSBuild при запуску збірки, однак, сам MSBuild використовується лише як «обгортка» для виклику згаданого раніше Unreal Build Tool.

Для аналізу вихідного коду в PVS-Studio потрібні результати роботи препроцесора. Аналізатору потрібно *.i файл в якому вставлені всі відмінності файли і розкриті всі макроси.

Примітка. Далі цей розділ буде цікавий тільки тим, хто використовує нестандартну складальну систему, наприклад, як у Unreal Engine. Якщо ви плануєте спробувати перевірити з допомогою PVS-Studio свій проект, який збирається якось хитро, то пропоную дочитати цей розділ до кінця. Можливо, він допоможе впоратися і з перевіркою вашого проекту. Якщо ж у вас звичайний проект Visual Studio, або хочеться швидше почитати про знайдені помилки, то можна пропустити цей розділ.

Для того, щоб коректно запустити препроцесор, необхідна інформація про параметри компіляції. У «звичайних» MSBuild проектах ця інформація є, і плагін PVS-Studio «бачить» її і може самостійно препроцессировать потрібні вихідні матеріали для подальшого запуску аналізатора. Інакше йдуть справи з проектами Unreal Engine.

Як вже було сказано вище, ці проекти – всього лише «обгортка», а реальний виклик компілятора здійснює Unreal Build Tool. Тому параметри компіляції і недоступні плагіну PVS-Studio для Visual Studio. Запустити аналіз «одним кліком» не виходить, хоча плагін можна використовувати для перегляду результатів аналізу.

Безпосередньо сам аналізатор (PVS-Studio.exe є command-line додатком, схожим за своїм сценарієм використання з C++ компілятором. Його, як і компілятор, необхідно запускати для кожного вихідного файлу, передаючи параметри компіляції цього файлу через командний рядок або response файл. Аналізатор ж сам запустить правильний препроцесор і згодом виконає аналіз.

Примітка. Є й інший варіант роботи. Можна запустити аналізатор і для вже готового препроцессированного файлу.

Таким чином, універсальним рішенням для інтеграції аналізатора PVS-Studio буде виклик його виконуваного файлу в тому ж місці, де відбувається виклик компілятора – тобто у складальній системі. В даному випадку в Unreal Build Tool. Зрозуміло, що це потребує модифікації використовуваної складальної системи, що, як у нашому випадку, може бути небажано. Тому, як раз для подібних ситуацій ми створили систему «перехоплення» викликів компілятора – Compiler Monitoring.

Система Compiler Monitoring здатна «відловлювати» запуск процесів компіляції (у випадку з Visual C++ це процеси cl.exe), збирати всі необхідні для препроцессирования параметри у таких процесів, і перезапускати препроцессирование компилируемых файлів, з подальшим запуском їх аналізу. Так і поступимо.



Малюнок 1. Схематичне зображення процесу перевірки проекту Unreal Engine

Інтеграція аналізу Unreal Engine зводиться до запуску безпосередньо перед складанням процесу моніторингу (CLMonitor.exe), який вже виконає по завершенні збирання всі необхідні дії по препроцессированию і безпосереднього викликом аналізатора. Для запуску моніторингу потрібно виконати просту команду:
CLMonitor.exe monitor

CLMonitor.exe запустить сам себе в режимі відстеження і завершиться. При цьому ще один процес CLMonitor.exe залишиться висіти в тлі, здійснюючи безпосереднє «відлов» компіляторів. По завершенню збирання потрібно виконати ще одну просту команду:
CLMonitor.exe analyze "UE.plog"

Тепер CLMonitor.exe запустить безпосередньо аналіз зібраних раніше вихідних файлів і збереже результати у файл UE.plog, з яким вже можна працювати в нашому IDE плагіні.

Ми налаштували на своєму Continuous Integration сервері регулярну нічну збірку цікавих для нас конфігурацій Unreal Engine з подальшим запуском аналізу. Таким чином ми, по-перше, перевіряли, що наші правки не зламали збірку, а по-друге, отримували щоранку новий звіт про перевірку Unreal Engine, з урахуванням всіх закладених у попередній день правок. І перед відправкою Pull Request'а про включення наших поправок в основний репозиторій проекту, ми могли легко перевірити, що поточна версія в нашому репозиторії стабільна, просто перезапустити збірку на сервері.

Нелінійна швидкість правок
Отже, ми впоралися з процесом складання проекту та його аналізом. Тепер поговоримо про правки в коді, на основі видаються попереджень.

На перший погляд може здатися, що кількість попереджень, які видаються аналізатором день від дня, повинна зменшуватися рівномірно. Приблизно однакову кількість повідомлень пригнічується за допомогою якихось механізмів PVS-Studio, приблизно однакову кількість правок робиться в коді.

Тобто теоретично можна чекати якийсь такий графік:



Малюнок 2. Ідеальний графік. Кількість помилок зменшується рівномірно з кожним робочим днем.

Насправді, повідомлення зникають на початку швидше, ніж потім. По-перше, на початковому етапі придушуються попередження, що перебувають у макросах, і це швидко скорочує їх кількість. По-друге, виходить так, що на початку виправляються очевидні речі, а незрозумілі місця відкладаються на потім. Ми можемо пояснити, чому на початку правилися прості речі. Хотілося продемонструвати розробникам з Epic Games, що ми приступили до роботи і процес пішов. Дивно було б почати зі складного і застрягти на цьому.

Всього безпосередньо на роботу з кодом Unreal Engine пішло 17 робочих днів. Нашою метою було знищити всі попередження першого і другого рівня загального призначення. Ось як посувалась робота:



Таблиця 1. Кількість попереджень аналізатора в різні дні.

Зверніть увагу на виділені червоним числа. В перші пару днів ми трохи освоїлися, а потім придушили попередження в деяких макросах, різко скоротивши кількість помилкових спрацьовувань.

Сімнадцять робочих днів це багато і треба дати пояснення читачам. По-перше, над проектом працювала не вся команда, а тільки два людини. При цьому вони займалися й іншими завданнями. По-друге, код Unreal Engine нам абсолютно не знайомий, тому внесення правок було досить складним завданням. Доводилося постійно розбиратися, чи варто правити код, і якщо так, то як.

Тепер ті ж самі дані представлені у вигляді однорідного графіка:



Малюнок 3. Згладжений графік кількості попереджень.

Практичний висновок. Треба запам'ятати самим і інших попередити. Погана ідея робити оцінку часу, який знадобиться для опрацювання усіх попереджень, тільки за першими днями роботи. У перші дні робота йде дуже швидко і прогноз виявиться занадто оптимістичний.

Але оцінити як-то все одно треба. Думаю, тут добре підійде якась магічна формула і з часом ми її складемо і покажемо світу. Поки що у нас недостатньо статистичних даних, щоб придумати щось правдиве.

Про знайдені помилки
Ми поправили досить багато фрагментів коду. Правки умовно можна розділити на 3 групи:
  1. Справжні помилки. Нижче ми наведемо кілька таких помилок в якості прикладу.
  2. Біди не було, але код збивав з пантелику аналізатор і міг заплутати людину, яка почне вивчати цей код. Іншими словами, цей код, що «пахне» і його теж корисно поправити. Що ми і зробили.
  3. Правки, які викликані виключно потребою «догодити» аналізатору коду, що видає помилкові спрацьовування. Ми намагалися по можливості винести придушення неправдивих повідомлень в окремий спеціальний файл або поліпшити роботу самого аналізатора. Але все одно в декількох місцях довелося провести рефакторинг, щоб допомогти аналізатору зрозуміти, що до чого.
Як і обіцяли, подивимося на деякі приклади помилки. Ми вибрали код простіше і цікавіше.

Перше цікаве повідомлення PVS-Studio: V506 Pointer to local variable 'NewBitmap' is stored outside the scope of this variable. Such a pointer will become invalid. fontcache.cpp 466
void GetRenderData (....)
{
....
FT_Bitmap* Bitmap = nullptr;
if( Slot->bitmap.pixel_mode == FT_PIXEL_MODE_MONO )
{
FT_Bitmap NewBitmap;
....
Bitmap = &NewBitmap;
}
....
OutRenderData.RawPixels.AddUninitialized(
Bitmap->rows * Bitmap->width );
....
}

Адреса об'єкта NewBitmap зберігається в покажчику Bitmap. Прикрість у тому, що відразу після цього час життя об'єкта NewBitmap закінчується, і вона руйнується. Виходить, що Bitmap вказує на вже зруйнований об'єкт.

При спробі використовувати покажчик для доступу до знищеному об'єкту виникне невизначений поведінка. Як воно проявить себе — невідомо. Програма навіть може довгі роки успішно працювати, якщо завдяки везінню дані мертвого об'єкта (розташовані на стеку) не перетираються чимось іншим.

Правильно буде винести оголошення NewBitmap за межі оператора 'if':
void GetRenderData (....)
{
....
FT_Bitmap* Bitmap = nullptr;

FT_Bitmap NewBitmap;
if( Slot->bitmap.pixel_mode == FT_PIXEL_MODE_MONO )
{
FT_Bitmap_New( &NewBitmap );
// Convert the font to mono 8bbp from 1bpp
FT_Bitmap_Convert( FTLibrary, &Slot->bitmap, &NewBitmap, 4 );

Bitmap = &NewBitmap;
}
else
{
Bitmap = &Slot->bitmap;
}
....
OutRenderData.RawPixels.AddUninitialized(
Bitmap->rows * Bitmap->width );
....
}

Наступне попередження PVS-Studio: V522 Dereferencing of the null pointer 'GEngine' might take place. Check the logical condition. gameplaystatics.cpp 988
void UGameplayStatics::DeactivateReverbEffect (....)
{
if (GEngine || !GEngine->UseSound())
{
return;
}
UWorld* ThisWorld = GEngine->GetWorldFromContextObject (....);
....
}

Якщо покажчик GEngine ненульовий, то відбувається вихід з функції. Все добре. А ось якщо вказівник GEngine нульовий, то він разыменовывается.

Ми виправили код наступним чином:
void UGameplayStatics::DeactivateReverbEffect (....)
{
if (GEngine == nullptr || !GEngine->UseSound())
{
return;
}

UWorld* ThisWorld = GEngine->GetWorldFromContextObject (....);
....
}

Цікава помилка чекає на читачів у наступному фрагменті коду. У ньому аналізатор виявив безглуздий виклик функції: V530 The return value of function 'Memcmp' is required to be utilized. pathfollowingcomponent.cpp 715
int32 UPathFollowingComponent::OptimizeSegmentVisibility(
int32 StartIndex)
{
....
if (Path.IsValid())
{
Path->ShortcutNodeRefs.Reserve (....);
Path->ShortcutNodeRefs.SetNumUninitialized (....);
}
FPlatformMemory::Memcmp(Path->ShortcutNodeRefs.GetData(),
RaycastResult.CorridorPolys,
RaycastResult.CorridorPolysCount *
sizeof(NavNodeRef));
....
}

Результат функції Memcmp не використовується. Це і насторожило аналізатор.

Насправді, програміст планував скопіювати ділянка пам'яті за допомогою функції Memcpy(), але допустив помилку. Виправлений варіант коду:
int32 UPathFollowingComponent::OptimizeSegmentVisibility(
int32 StartIndex)
{
....
if (Path.IsValid())
{
Path->ShortcutNodeRefs.Reserve (....);
Path->ShortcutNodeRefs.SetNumUninitialized (....);

FPlatformMemory::Memcpy(Path->ShortcutNodeRefs.GetData(),
RaycastResult.CorridorPolys,
RaycastResult.CorridorPolysCount *
sizeof(NavNodeRef));
}
....
}

Поговоримо про діагностичному повідомленні, яке можна зустріти при перевірці практично будь-якого проекту. Дуже поширений тип помилки вона виявляє. Мова йде про діагностику V595. В нашій базі помилок, вона займає перше місце за кількістю знайдених недоліків (див. приклади). На перший погляд, список не такий вже великий, порівняно, скажімо, з V501. Але справа в тому, що діагностики V595 кілька нудні, щоб виписувати багато прикладів з якогось проекту. Часто виписаний один приклад і зроблена приписка виду: And 161 additional diagnostic messages. У половині випадків — це самі справжні помилки. Ось як це виглядає:



Малюнок 4. Жахи діагностики V595.

Діагностика V595 знаходить ділянки коду, де виконується розіменування вказівника до його перевірки на нуль. Зазвичай в перевіряються проекти завжди знаходиться деяка кількість таких попереджень. Перевірка та розіменування вказівника можуть знаходиться досить далеко в коді функції: у десятках чи навіть сотнях рядків один від одного, що ускладнює виправлення помилки. Але трапляються і маленькі, дуже наочні, приклади, як, наприклад, ця функція:
float SGammaUIPanel::OnGetGamma() const
{
float DisplayGamma = GEngine->DisplayGamma;
return GEngine ? DisplayGamma : 2.2 f;
}
Попередження: V595 The 'GEngine' pointer was utilized before it was verified against nullptr. Check lines: 47, 48. gammauipanel.cpp 47

Ми виправили цю функцію наступним чином:
float SGammaUIPanel::OnGetGamma() const
{
return GEngine ? GEngine->DisplayGamma : 2.2 f;
}

Перейдемо до наступної частини:

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 289, 299. automationreport.cpp 289
void FAutomationReport::ClustersUpdated(const int32 NumClusters)
{
...
//Fixup Results array
if( NumClusters > Results.Num() ) //<==
{
for( int32 ClusterIndex = Results.Num();
ClusterIndex < NumClusters; ++ClusterIndex )
{
....
Results.Add( AutomationTestResult );
}
}
else if( NumClusters > Results.Num() ) //<==
{
Results.RemoveAt(NumClusters, Results.Num() - NumClusters);
}
....
}

В поточному вигляді друге умова ніколи не виконується. Логічно припустити, що помилка в знаку другої умови, щоб з масиву 'Result' віддалялися зайві елементи:
void FAutomationReport::ClustersUpdated(const int32 NumClusters)
{
....
//Fixup Results array
if( NumClusters > Results.Num() )
{
for( int32 ClusterIndex = Results.Num();
ClusterIndex < NumClusters; ++ClusterIndex )
{
....
Results.Add( AutomationTestResult );
}
}
else if( NumClusters < Results.Num() )
{
Results.RemoveAt(NumClusters, Results.Num() - NumClusters);
}
....
}

Приклад коду на уважне читання. Повідомлення: V616 The 'DT_POLYTYPE_GROUND' named constant with the value of 0 is used in the bitwise operation. pimplrecastnavmesh.cpp 2006
/// Flags representing the type of a navigation mesh polygon.
enum dtPolyTypes
{
DT_POLYTYPE_GROUND = 0,
DT_POLYTYPE_OFFMESH_POINT = 1,
DT_POLYTYPE_OFFMESH_SEGMENT = 2,
};

uint8 GetValidEnds (...., const dtPoly& Poly)
{
....
if ((Poly.getType() & DT_POLYTYPE_GROUND) != 0)
{
return false;
}
....
}

На перший погляд все добре. Здається, що по масці виділяється якийсь біт і перевіряється його значення. Але, насправді, у перерахуванні 'dtPolyTypes' визначаються просто іменовані константи, не призначені для виділення певних біт.

В даному умови константа DT_POLYTYPE_GROUND дорівнює 0, а значить умова ніколи не виконається.

Виправлений варіант:
uint8 GetValidEnds (...., const dtPoly& Poly)
{
....
if (Poly.getType() == DT_POLYTYPE_GROUND)
{
return false;
}
....
}

Виявлена помилка: V501 There are identical sub-expressions to the left and to the right of the '||' operator: !bc.lclusters ||!bc.lclusters detourtilecache.cpp 687
dtStatus dtTileCache::buildNavMeshTile (....)
{
....
bc.lcset = dtAllocTileCacheContourSet(m_talloc);
bc.lclusters = dtAllocTileCacheClusterSet(m_talloc);
if (!bc.lclusters || !bc.lclusters) //<==
return status;
status = dtBuildTileCacheContours (....);
....
}

При копіюванні змінної, її забули перейменувати з 'bc.lclusters' в 'bc.lcset'.

Результати регулярної перевірки
Вище були перераховані далеко не всі знайдені помилки, а тільки невелика їх частина. Ми привели їх, щоб продемонструвати, які помилки може знайти PVS-Studio навіть в робочому і оттестированном коді.

Однак, ми не втомлюємося повторювати, що разова перевірка коду — це неправильний спосіб використовувати статичний аналізатор. Аналіз потрібно проводити регулярно і тоді величезна кількість помилок і друкарських помилок можна знайти ще на етапі написання коду, а не на етапі тестування або супроводу.

Зараз ми відмінно підкріпимо свої слова за допомогою проекту Unreal Engine.

На початку ми правили код, не звертаючи уваги, свіжий це код або старий. Було не до того. Зате ми дуже добре помітили, як аналізатор PVS-Studio почав виявляти помилки в новому або виправленому коді, коли ми звели кількість попереджень до 0.

Насправді, ми возилися з кодом трохи більше, ніж 17 робочих днів. Коли ми закінчили вносити правки, і аналізатор почав видавати 0 попереджень, ми протягом ще 2 днів чекали, коли розробники Unreal Engine приймуть наш останній Pull Request. І протягом цього часу продовжували оновлюватися з основного репозиторію і виконувати перевірку коду.

І за ці два дні ми встигли побачити, як аналізатор виявляв помилки у новому коді. Ці помилки ми теж поправили. Але найголовніше в тому, що це відмінно демонструє користь від регулярних перевірок.

Фактично, кінець графіка «повідомлень» придбав вигляд:



Малюнок 5. Схематичний графік зростання кількості попереджень, після того як їх стало 0.

Давайте подивимося, що ми встигли виявити в останні два дні, перевіряючи свіжі зміни в коді проекту.

День перший
Перше повідомлення аналізатора: V560 A part of conditional expression is always true: FBasicToken::TOKEN_Guid. k2node_mathexpression.cpp 235
virtual FString ToString() const override
{
if (Token.TokenType == FBasicToken::TOKEN_Identifier ||
FBasicToken::TOKEN_Guid) //<==
{
....
}
else if (Token.TokenType == FBasicToken::TOKEN_Const)
{
....
}

Забули дописати «Token.TokenType ==». В результаті, оскільки іменована константа 'FBasicToken::TOKEN_Guid' не дорівнює 0, умова завжди істинно.

Друге повідомлення аналізатора: V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] CompressedDataRaw;'. crashupload.cpp 222
void FCrashUpload::CompressAndSendData()
{
....
uint8* CompressedDataRaw = new uint8[BufferSize]; //<==

int32 CompressedSize = BufferSize;
int32 UncompressedSize = UncompressedData.Num();
....
// Copy compressed data into the array.
TArray<uint8> CompressedData;
CompressedData.Append( CompressedDataRaw, CompressedSize );
delete CompressedDataRaw; //<==
CompressedDataRaw = nullptr;
....
}

На практиці ця помилка не завжди проявляє себе, так як виділяється масив елементів типу char. Але все одно це помилка, яка призводить до невизначеної поведінки і її слід обов'язково виправити.

День другий
Перше попередження аналізатора: V521 Such expressions using the ',' operator are dangerous. Make sure the expression is correct. unrealaudiodevicewasapi.cpp 128
static void GetArrayOfSpeakers (....)
{
Speakers.Reset();
uint32 ChanCount = 0;
// Build a flag field of the speaker outputs of this device
for (uint32 SpeakerTypeIndex = 0;
SpeakerTypeIndex < ESpeaker::SPEAKER_TYPE_COUNT, //<==
ChanCount < NumChannels; ++SpeakerTypeIndex)
{
....
}

check(ChanCount == NumChannels);
}

Гарна така, жирна помилка.

Оператор кома ',' використовується для виконання завдань, які стоять по обидві сторони від нього виразів в порядку зліва направо і отримання значення правого вираження.

В результаті, умовою припинення циклу є тільки ця перевірка: ChanCount < NumChannels.

Виправлене умова:
static void GetArrayOfSpeakers (....)
{
Speakers.Reset();
uint32 ChanCount = 0;
// Build a flag field of the speaker outputs of this device
for (uint32 SpeakerTypeIndex = 0;
SpeakerTypeIndex < ESpeaker::SPEAKER_TYPE_COUNT &&
ChanCount < NumChannels; ++SpeakerTypeIndex)
{
....
}
check(ChanCount == NumChannels);
}

Друге попередження. V543 It is odd that value '-1' is assigned to the variable 'Result' of HRESULT type. unrealaudiodevicewasapi.cpp 568
#define S_OK ((HRESULT)0L)
#define S_FALSE ((HRESULT)1L)

bool
FUnrealAudioWasapi::OpenDevice(uint32 DeviceIndex,
EStreamType::Type StreamType)
{
check(WasapiInfo.DeviceEnumerator);

IMMDevice* Device = nullptr;
IMMDeviceCollection* DeviceList = nullptr;
WAVEFORMATEX* DeviceFormat = nullptr;
FDeviceInfo DeviceInfo;
HRESULT Result = S_OK; //<==
....
if (!GetDeviceInfo(DataFlow, DeviceIndex, DeviceInfo))
{
Result = -1; //<==
goto Cleanup;
}
....
}

HRESULT — це 32-розрядне значення, поділене на три різних поля: код серйозності помилки, код пристрою і код помилки. Для роботи зі значенням HRESULT служать спеціальні константи, такі як S_OK, E_FAIL, E_ABORT і так далі. А для перевірки значень типу HRESULT призначені такі макроси як SUCCEEDED, FAILED.

Попередження V543 видається в тому випадку, якщо в змінну типу HRESULT намагаються записати значення -1, true або false.

Запис значення "-1" некоректна. Якщо хочеться повідомити про якійсь незрозумілій помилку, то слід використовувати значення 0x80004005L (Unspecified failure). Ця та аналогічні константи описані в «WinError.h».

Ех, яке складне впровадження...
Деякі програмісти і менеджери можуть засмутитися, дізнавшись, що для інтеграції статичного аналізу в їх проект буде потрібно N днів. Проте, не обов'язково йти цим шляхом. Треба розуміти, що розробники Epic Games вибрали ІДЕАЛЬНИЙ ШЛЯХ, але не самий простий і швидкий.

Так, в ідеалі краще відразу виправити всі помилки і потім швидко реагувати на нові повідомлення аналізатора, що видаються на новий код. Але можна почати отримувати користь від статичного аналізу, не витрачаючи час на правку старого коду.

Для цього PVS-Studio пропонує спеціальний механізм для розмітки повідомлень. У двох словах опишемо загальну ідею.

Всі повідомлення, які видає аналізатор, позначаються в спеціальній базі як неактивні. І тоді програмісти бачать тільки повідомлення аналізатора, що відносяться до нового чи зміненим кодом. Тобто можна відразу почати отримувати користь від аналізу. А вже потім, якщо буде бажання і час, можна потихеньку переглядати повідомлення, що відносяться до старого коду і правити недоліки.

Детально про все це можна прочитати тут: документація, як швидко впровадити аналіз в проект.

«А ви відписали розробникам?»
Після кожної статті про перевірку якого-небудь проекту нас запитують: «А ви повідомили розробникам?» І хоча ми завжди повідомляємо, такі питання неабияк стомлюють. Однак, в цей раз ми не тільки «відписали розробникам», але і виправили всі ці помилки. У чому кожен бажаючий може переконатися в репозиторії на GitHub.

Висновок
Сподіваємося, виконана робота по виправленню і поліпшення коду Unreal Engine сподобалася розробникам Epic Games і принесла користь проекту. І чекаємо нових проектів на движку Unreal Engine.

Деякі висновки, які можна зробити за результатами нашої роботи:
  1. Код проекту Unreal Engine досить якісний. Нехай читачів не бентежить велика кількість попереджень на початковому етапі. Це нормальна ситуація. Більшість цих попереджень було прибрано з допомогою різних методів і налаштувань. Кількість цих помилок, виявлених у коді, для проекту такого розміру, дуже невелика.
  2. Правити чужий незнайомий код часто дуже складно. Втім, це і так зрозуміло будь-якому розробнику на інтуїтивному рівні. Тим не менш вирішили відзначити це.
  3. Швидкість «переробки» попереджень не лінійна. Вона буде сповільнюватися і це треба враховувати при розрахунку, скільки ще залишилося до кінця правок.
  4. Максимальна користь від статичного аналізу може бути отримана тільки при його регулярному використанні.
Спасибі всім, хто прочитав цю статтю і бажаємо всім безбажного коду. З повагою, розробники аналізатора PVS-Studio. І зараз саме час його завантажити і спробувати на своєму проекті.

Джерело: Хабрахабр

0 коментарів

Тільки зареєстровані та авторизовані користувачі можуть залишати коментарі.