Долгожданная проверка Unreal Engine 4

• Unreal Engine
• Способ проверки nmake-based проекта
• Результаты проверки
  • Опечатки
  • Нулевые указатели
  • Ещё о нулевых указателях
  • Лишнее объявление переменной
  • Ошибки при вызове функций
  • Переменная присваивается сама себе
  • Самая красивая из найденных ошибок
  • Забыт оператор break
  • Микрооптимизации
  • Подозрительная сумма
  • Путаница с enum
  • Логическая ошибка
  • Подозрительный сдвиг
  • Устаревшие проверки
  • Copy-Paste
  • Разное
• Выводы

19 марта 2014 года Unreal Engine 4 стал доступен для всех желающих. Цена подписки всего 19$ в месяц. Исходные коды также выложены на github репозиторий. С этого момента нам поступила масса сообщений на почту, в твиттер и так далее, с просьбой проверить этот игровой движок. Мы удовлетворяем просьбу наших читателей. Давайте посмотрим, что интересного можно найти в исходном коде с помощью статического анализатора кода PVS-Studio.

Unreal Engine

Unreal Engine — игровой движок, разрабатываемый и поддерживаемый компанией Epic Games. Написан на языке C++. Позволяет создавать игры для большинства операционных систем и платформ: Microsoft Windows, Linux, macOS и macOS X, консолей Xbox, Xbox 360, PlayStation 2, PlayStation Portable, PlayStation 3, Wii, Dreamcast и Nintendo GameCube.

Официальный сайт: https://www.unrealengine.com/

Описание на сайте Wikipedia: Unreal Engine.

Способ проверки nmake-based проекта

С проверкой проекта Unreal Engine не всё так просто. Чтобы проверить этот проект, нам пришлось воспользоваться новой возможностью, которую мы недавно реализовали в PVS-Studio Standalone. Из-за этого нам пришлось немного задержать публикацию статьи. Мы решили опубликовать её после релиза PVS-Studio, где уже будет эта новая возможность. Возможно, многим захочется ее попробовать. Она существенно облегчает проверку проектов, где используются сложные или нестандартные системы сборки.

Классический принцип работы PVS-Studio таков:

• Вы открываете проект в Visual Studio.
• Нажимаете кнопку "проверить".
• Плагин, интегрированный в Visual Studio, собирает всю необходимую информацию: какие файлы надо проверять, какие макросы использовать, где лежат заголовочные файлы и так далее.
• Плагин запускает собственно анализатор и отображает полученные результаты.

Нюанс в том, что Unreal Engine 4 – это nmake-based проект, поэтому проверить его с помощью плагина PVS-Studio нельзя.

Поясню. Есть проект для среды Visual Studio. Но сборка осуществляется с помощью nmake. Это значит, что плагин не знает, какие файлы и с какими ключами компилируются. Соответственно, анализ невозможен. Вернее, анализ возможен, но процесс трудоёмок (смотри раздел документации: "Прямая интеграция анализатора в системы автоматизации сборки").

И здесь на выручку приходит PVS-Studio Standalone! Он отслеживает запуски компилятора и "подсматривает" всю необходимую информацию.

Вот как происходила проверка Unreal Engine:

• Запустили PVS-Studio Standalone.
• Нажали кнопку "Compiler Monitoring".
• Нажали на кнопку "Start Monitoring". Увидели, что включился режим наблюдения за вызовами компилятора.
• Открыли проект Unreal Engine в Visual Studio. Запустили сборку проекта. При этом в окне мониторинга видно, что перехватываются вызовы компилятора.
• Когда сборка в Visual Studio закончилась, мы нажали кнопку Stop Monitoring. После этого начал работу анализатор PVS-Studio.

В результате, диагностические сообщения появляются в окне утилиты PVS-Studio Standalone.

Hint. Для удобства лучше использовать Visual Studio, а не встроенный в PVS-Studio Standalone редактор. Достаточно сохранить результаты анализа в файл, а затем открыть его из среды Visual Studio (Menu->PVS-Studio->Open/Save->Open Analysis Report).

Всё это и многое другое описано в статье "PVS-Studio теперь поддерживает любую сборочную систему на Windows и любой компилятор. Легко и "из коробки". Прошу обязательно прочитать эту статью, прежде чем начинать эксперименты с PVS-Studio Standalone!

Результаты проверки

Код проекта Unreal Engine показался мне очень качественным. Например, разработчики используют при разработке статический анализ кода. Об этом говорят такие фрагменты в коде, как:

// Suppress static code analysis warning about a
// potential comparison of two constants
CA_SUPPRESS(6326);
....
// Suppress static code analysis warnings about a
// potentially ill-defined loop. BlendCount > 0 is valid.
CA_SUPPRESS(6294)
....
#if USING_CODE_ANALYSIS

Судя по этим фрагментам кода, используется статический анализатор кода, встроенный в Visual Studio. Об этом анализаторе кода: Visual Studio 2013 Static Code Analysis in depth: What? When and How?

Возможно, используются и другие анализаторы, но я про это ничего не знаю.

Итак, код написан весьма качественно. При работе используются инструменты статического анализа кода. Поэтому, проверив проект с помощью PVS-Studio, мы нашли мало подозрительных фрагментов кода. Однако, как и в любом большом проекте, в нём есть ошибки. И некоторые из них способен найти PVS-Studio. Давайте же посмотрим, что удалось заметить подозрительного.

Опечатки

static bool PositionIsInside(....)
{
  return
    Position.X >= Control.Center.X - BoxSize.X * 0.5f &&
    Position.X <= Control.Center.X + BoxSize.X * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f &&
    Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f;
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'Position.Y >= Control.Center.Y - BoxSize.Y * 0.5f' to the left and to the right of the '&&' operator. svirtualjoystick.cpp 97

Обратите внимание, что переменная "Position.Y" два раза сравнивается с выражением "Control.Center.Y - BoxSize.Y * 0.5f". Это явно опечатка. В последней строчке следует заменить оператор '-' на оператор '+' и '>=' на '<='.

Ещё одна схожая опечатка в условии:

void FOculusRiftHMD::PreRenderView_RenderThread(
  FSceneView& View)
{
  ....
  if (View.StereoPass == eSSP_LEFT_EYE ||
      View.StereoPass == eSSP_LEFT_EYE)
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'View.StereoPass == eSSP_LEFT_EYE' to the left and to the right of the '||' operator. oculusrifthmd.cpp 1453

Видимо, работа с Oculus Rift пока не очень оттестирована.

Продолжим.

struct FMemoryAllocationStats_DEPRECATED
{
  ....
  SIZE_T  NotUsed5;
  SIZE_T  NotUsed6;
  SIZE_T  NotUsed7;
  SIZE_T  NotUsed8;
  ....
};

FMemoryAllocationStats_DEPRECATED()
{
  ....
  NotUsed5 = 0;
  NotUsed6 = 0;
  NotUsed6 = 0;  
  NotUsed8 = 0;  
  ....
}

Предупреждение PVS-Studio: V519 The 'NotUsed6' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 86, 88. memorybase.h 88

Инициализируются члены структуры. Из-за опечатки, член 'NotUsed6' инициализируется дважды. А член 'NotUsed7' остался неинициализированным. Впрочем, суффикс _DEPRECATED() в названии функции говорит, что это уже неинтересный код.

Ещё пара мест, где одной переменной дважды присваивается значение:

• V519 The 'HighlightText' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 204, 206. srichtextblock.cpp 206
• V519 The 'TrackError.MaxErrorInScaleDueToScale' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1715, 1716. animationutils.cpp 1716

Нулевые указатели

Очень часто я встречаю разыменование нулевого указателя в обработчиках ошибок. Это не удивительно. Такие фрагменты сложно и не интересно тестировать. Встретить разыменование нулевого указателя в обработчике ошибок можно и в проекте Unreal Engine:

bool UEngine::CommitMapChange( FWorldContext &Context )
{
  ....
  LevelStreamingObject = Context.World()->StreamingLevels[j];
  if (LevelStreamingObject != NULL)
  {
    ....
  }
  else
  {
    check(LevelStreamingObject);
    UE_LOG(LogStreaming, Log,
           TEXT("Unable to handle streaming object %s"),
           *LevelStreamingObject->GetName());
  }
  ....
}

Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'LevelStreamingObject' might take place. unrealengine.cpp 10768

Если произошла ошибка, то хочется распечатать имя объекта. Вот только не существует этого объекта.

Рассмотрим другой фрагмент, где будет разыменован нулевой указатель. Здесь всё интереснее. Возможно, ошибка присутствует из-за неправильного merge. В любом случае, по комментарию видно, что код не доделан:

void FStreamingPause::Init()
{
  ....
  if( GStreamingPauseBackground == NULL && GUseStreamingPause )
  {
    // @todo UE4 merge andrew
    // GStreamingPauseBackground = new FFrontBufferTexture(....);
    GStreamingPauseBackground->InitRHI();
  }
}

Предупреждение PVS-Studio: V522 Dereferencing of the null pointer 'GStreamingPauseBackground' might take place. streamingpauserendering.cpp 197

Ещё о нулевых указателях

Почти в любой программе я встречаю массу предупреждений с кодом V595 (примеры). Эти предупреждения говорят о следующей ситуации:

Вначале указатель разыменовывается. Ниже этот указатель проверяется на равенство нулю. Это далеко не всегда ошибка. Но это очень подозрительный код, и его надо проверить!

Диагностика V595 позволяет выявлять вот такие ляпы:

/**
 * Global engine pointer.
 * Can be 0 so don't use without checking.
 */
ENGINE_API UEngine* GEngine = NULL;

bool UEngine::LoadMap( FWorldContext& WorldContext,
  FURL URL, class UPendingNetGame* Pending, FString& Error )
{
  ....
  if (GEngine->GameViewport != NULL)
  {
    ClearDebugDisplayProperties();
  }

  if( GEngine )
  {
    GEngine->WorldDestroyed( WorldContext.World() );
  }
  ....
}

Предупреждение PVS-Studio: V595 The 'GEngine' pointer was utilized before it was verified against nullptr. Check lines: 9714, 9719. unrealengine.cpp 9714

Обратите внимание на комментарий. Глобальная переменная GEngine может быть равна нулю. Прежде чем её использовать, её нужно проверить.

В функции LoadMap() действительно есть такая проверка:

if( GEngine )

Только вот незадача. Проверка расположена уже после того, как указателем попользовались:

if (GEngine->GameViewport != NULL)

Предупреждений V595 довольно много (82 штуки). Думаю, многие из них окажутся ложными. Поэтому не буду захламлять статью сообщениями и приведу их отдельным списком: ue-v595.txt.

Лишнее объявление переменной

Рассмотрим красивую ошибку. Она связана с тем, что случайно создаётся новая переменная, а не используется старая.

void FStreamableManager::AsyncLoadCallback(....)
{
  ....
  FStreamable* Existing = StreamableItems.FindRef(TargetName);
  ....
  if (!Existing)
  {
    // hmm, maybe it was redirected by a consolidate
    TargetName = ResolveRedirects(TargetName);
    FStreamable* Existing = StreamableItems.FindRef(TargetName);
  }
  if (Existing && Existing->bAsyncLoadRequestOutstanding)
  ....
}

Предупреждение PVS-Studio: V561 It's probably better to assign value to 'Existing' variable than to declare it anew. Previous declaration: streamablemanager.cpp, line 325. streamablemanager.cpp 332

Как я понимаю, на самом деле, должно быть написано так:

// hmm, maybe it was redirected by a consolidate
TargetName = ResolveRedirects(TargetName);
Existing = StreamableItems.FindRef(TargetName);

Ошибки при вызове функций

bool FRecastQueryFilter::IsEqual(
  const INavigationQueryFilterInterface* Other) const
{
  // @NOTE: not type safe, should be changed when
  // another filter type is introduced
  return FMemory::Memcmp(this, Other, sizeof(this)) == 0;
}

Предупреждение PVS-Studio: V579 The Memcmp function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. pimplrecastnavmesh.cpp 172

Комментарий предупреждает, что использовать Memcmp() опасно. Но, на самом деле, всё ещё хуже, чем думает программист. Дело в том, что функция сравнивает только часть объекта.

Оператор sizeof(this) возвращает размер указателя. То есть в 32-битной программе функция сравнит первые 4 байта. В 64-битной программе будут сравниваться 8 байт.

Правильный код:

return FMemory::Memcmp(this, Other, sizeof(*this)) == 0;

На этом злоключения с функцией Memcmp() не заканчиваются. Рассмотрив следующий фрагмент кода:

D3D11_STATE_CACHE_INLINE void GetBlendState(
  ID3D11BlendState** BlendState, float BlendFactor[4],
  uint32* SampleMask)
{
  ....
  FMemory::Memcmp(BlendFactor, CurrentBlendFactor,
                  sizeof(CurrentBlendFactor));
  ....
}

Предупреждение PVS-Studio: V530 The return value of function 'Memcmp' is required to be utilized. d3d11statecacheprivate.h 547

Анализатор удивился, увидев, что результат работы функции Memcmp() никак не используется. И действительно, это ошибка. Как я понимаю, здесь хотели копировать, а не сравнивать данные. Тогда надо использовать функцию Memcpy():

FMemory::Memcpy(BlendFactor, CurrentBlendFactor,
                sizeof(CurrentBlendFactor));

Переменная присваивается сама себе

enum ECubeFace;
ECubeFace CubeFace;

friend FArchive& operator<<(
  FArchive& Ar,FResolveParams& ResolveParams)
{
  ....
  if(Ar.IsLoading())
  {
    ResolveParams.CubeFace = (ECubeFace)ResolveParams.CubeFace;
  }
  ....
}

Предупреждение PVS-Studio: V570 The 'ResolveParams.CubeFace' variable is assigned to itself. rhi.h 1279

Переменная 'ResolveParams.CubeFace' имеет тип ECubeFace. Используется явное приведение переменной к типу ECubeFace. То есть ничего не происходит. После чего переменная присваивается сама себе. С этим кодом что-то не так.

Самая красивая из найденных ошибок

Больше всего, мне понравилась вот эта ошибка:

bool VertInfluencedByActiveBone(
  FParticleEmitterInstance* Owner,
  USkeletalMeshComponent* InSkelMeshComponent,
  int32 InVertexIndex,
  int32* OutBoneIndex = NULL);

void UParticleModuleLocationSkelVertSurface::Spawn(....)
{
  ....
  int32 BoneIndex1, BoneIndex2, BoneIndex3;
  BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE;

  if(!VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[1], &BoneIndex2) && 
     !VertInfluencedByActiveBone(
        Owner, SourceComponent, VertIndex[2]) &BoneIndex3)
  {
  ....
}

Предупреждение PVS-Studio: V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator. particlemodules_location.cpp 2120

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

Давайте разберёмся. Обратите внимание, что последний аргумент функции VertInfluencedByActiveBone() является необязательным.

В коде функция VertInfluencedByActiveBone() вызывается 3 раза. Два раза ей передаётся 4 аргумента. В последнем случае аргументов всего 3. В этом и есть ошибка.

Благодаря везению, код компилируется, и ошибка незаметна. Вот что здесь получается:

• Вызывается функция с 3 аргументами: "VertInfluencedByActiveBone(Owner, SourceComponent, VertIndex[2])";
• К результату функции применяется оператор '!';
• Результат выражения "!VertInfluencedByActiveBone(...)" имеет тип bool;
• К результату применяется оператор '&' (bitwise AND);
• Всё успешно компилируется, так как слева от оператора '&' находится выражение типа bool. Справа от '&' находится целочисленная переменная BoneIndex3.

Анализатор заподозрил неладное, когда увидел, что один из аргументов оператора '&' является тип 'bool'. Об этом он сообщил. И не зря.

Чтобы исправить ошибку, нужно добавить запятую и разместить закрывающуюся скобку в другом месте:

if(!VertInfluencedByActiveBone(
      Owner, SourceComponent, VertIndex[0], &BoneIndex1) &&
   !VertInfluencedByActiveBone(
      Owner, SourceComponent, VertIndex[1], &BoneIndex2) && 
   !VertInfluencedByActiveBone(
      Owner, SourceComponent, VertIndex[2], &BoneIndex3))

Забыт оператор break

static void VerifyUniformLayout(....)
{
  ....
  switch(Member.GetBaseType())
  {
    case UBMT_STRUCT:  BaseTypeName = TEXT("struct"); 
    case UBMT_BOOL:    BaseTypeName = TEXT("bool"); break;
    case UBMT_INT32:   BaseTypeName = TEXT("int"); break;
    case UBMT_UINT32:  BaseTypeName = TEXT("uint"); break;
    case UBMT_FLOAT32: BaseTypeName = TEXT("float"); break;
    default:           
      UE_LOG(LogShaders, Fatal,
        TEXT("Unrecognized uniform ......"));
  };
  ....
}

Предупреждение PVS-Studio: V519 The 'BaseTypeName' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 862, 863. openglshaders.cpp 863

В самом начале забыт оператор "break;". Думаю, комментарии и пояснения излишни.

Микрооптимизации

В анализаторе PVS-Studio имеется небольшой набор диагностических правил, которые помогают провести небольшие оптимизации в коде. Впрочем, иногда они весьма полезны. Рассмотрим в качестве примера один из операторов присваивания:

FVariant& operator=( const TArray<uint8> InArray )
{
  Type = EVariantTypes::ByteArray;
  Value = InArray;
  return *this;
}

Предупреждение PVS-Studio: V801 Decreased performance. It is better to redefine the first function argument as a reference. Consider replacing 'const .. InArray' with 'const .. &InArray'. variant.h 198

Не очень хорошая идея передавать массив по значению. Массив 'InArray' можно и нужно передавать, используя константную ссылку.

Анализатор выдаёт достаточно много предупреждений, связанных с микрооптимизациями. Далеко не все из них окажутся полезны, но на всякий случай приведу их списком: ue-v801-V803.txt.

Подозрительная сумма

uint32 GetAllocatedSize() const
{
  return UniformVectorExpressions.GetAllocatedSize()
    + UniformScalarExpressions.GetAllocatedSize()
    + Uniform2DTextureExpressions.GetAllocatedSize()
    + UniformCubeTextureExpressions.GetAllocatedSize()
    + ParameterCollections.GetAllocatedSize()
    + UniformBufferStruct
        ?
        (sizeof(FUniformBufferStruct) +
         UniformBufferStruct->GetMembers().GetAllocatedSize())
        :
        0;
}

Предупреждение PVS-Studio: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. materialshared.h 224

Код весьма сложный. Чтобы пояснить, что не так, я составлю упрощенный искусственный пример:

return A() + B() + C() + uniform ? UniformSize() : 0;

Вычисляется некий размер. В зависимости от значения переменной 'uniform', следует прибавлять 'UniformSize()' или 0. На самом деле, этот код работает не так. Приоритет операторов сложения '+' выше, чем приоритет оператора '?:'.

Вот, что получается:

return (A() + B() + C() + uniform) ? UniformSize() : 0;

Аналогичную ситуацию мы наблюдаем и в коде Unreal Engine. Мне кажется, вычисляется не то, что хотел программист.

Путаница с enum

Я вначале не хотел описывать этот случай, так как нужно привести достаточно большой фрагмент кода. Потом я всё-таки переборол лень. Прошу потерпеть и читателей.

namespace EOnlineSharingReadCategory
{
  enum Type
  {
    None          = 0x00,
    Posts         = 0x01,
    Friends       = 0x02,
    Mailbox       = 0x04,
    OnlineStatus  = 0x08,
    ProfileInfo   = 0x10,  
    LocationInfo  = 0x20,
    Default       = ProfileInfo|LocationInfo,
  };
}

namespace EOnlineSharingPublishingCategory
{
  enum Type {
    None          = 0x00,
    Posts         = 0x01,
    Friends       = 0x02,
    AccountAdmin  = 0x04,
    Events        = 0x08,
    Default       = None,
  };

  inline const TCHAR* ToString
    (EOnlineSharingReadCategory::Type CategoryType)
  {
    switch (CategoryType)
    {
    case None:
    {
      return TEXT("Category undefined");
    }
    case Posts:
    {
      return TEXT("Posts");
    }
    case Friends:
    {
      return TEXT("Friends");
    }
    case AccountAdmin:
    {
      return TEXT("Account Admin");
    }
    ....
  }
}

Анализатор выдаёт здесь сразу несколько предупреждений V556. Дело в том, что аргументом оператора 'switch()' является переменная типа EOnlineSharingReadCategory::Type. При этом в операторах 'case' используются значения от другого типа EOnlineSharingPublishingCategory::Type.

Логическая ошибка

const TCHAR* UStructProperty::ImportText_Internal(....) const
{
  ....
  if (*Buffer == TCHAR('\"'))
  {
    while (*Buffer && *Buffer != TCHAR('\"') &&
           *Buffer != TCHAR('\n') && *Buffer != TCHAR('\r'))
    {
      Buffer++;
    }

    if (*Buffer != TCHAR('\"'))
  ....
}

Предупреждение PVS-Studio: V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 310, 312. propertystruct.cpp 310

Здесь хотели пропустить всё, что содержится в двойных кавычках. Алгоритм должен был быть таким:

• Если находим двойную кавычку, то запускаем цикл.
• В цикле попускаем символы до тех пор, пока не встретим следующую двойную кавычку.

Ошибка в том, что после нахождения первой двойной кавычки, мы не сдвигаем указатель на следующий символ. В результате, сразу находится вторая кавычка. Цикл не запускается.

Поясню это на упрощенном коде:

if (*p == '\"')
{
  while (*p && *p != '\"')
      p++;
}

Чтобы исправить ошибку, надо сделать следующие изменения:

if (*p == '\"')
{
  p++;
  while (*p && *p != '\"')
      p++;
}

Подозрительный сдвиг

class FMallocBinned : public FMalloc
{
  ....
  /* Used to mask off the bits that have been used to
     lookup the indirect table */
  uint64 PoolMask;
  ....
  FMallocBinned(uint32 InPageSize, uint64 AddressLimit)
  {
    ....
    PoolMask = ( ( 1 << ( HashKeyShift - PoolBitShift ) ) - 1 );
    ....
  }
}

Предупреждение PVS-Studio: V629 Consider inspecting the '1 << (HashKeyShift - PoolBitShift)' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. mallocbinned.h 800

Есть здесь ошибка или нет, зависит от того, нужно ли сдвигать 1 более чем на 31 разряд. Так как результат помещается в 64-битную переменную PoolMask, видимо такая необходимость есть.

Если я угадал, то в библиотеке имеется ошибка в подсистеме выделения памяти.

Число 1 имеет тип int. Это значит, что сдвинуть 1, скажем на 35 разрядов, нельзя. Формально, возникнет неопределённое поведение (подробности). На практике, произойдёт переполнение и будет вычислено неправильное значение.

Правильный код:

PoolMask = ( ( 1ull << ( HashKeyShift - PoolBitShift ) ) - 1 );

Устаревшие проверки

void FOculusRiftHMD::Startup()
{
  ....
  pSensorFusion = new SensorFusion();
  if (!pSensorFusion)
  {
    UE_LOG(LogHMD, Warning,
      TEXT("Error creating Oculus sensor fusion."));
    return;
  }
  ....
}

Предупреждение PVS-Studio: V668 There is no sense in testing the 'pSensorFusion' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. oculusrifthmd.cpp 1594

Уже давно, в случае ошибки выделения памяти, оператор 'new' генерирует исключение. Проверка "if (!pSensorFusion)" не нужна.

В больших проектах я, как правило, встречаю очень много таких мест. На удивление, в Unreal Engine таких мест мало. Список: ue-V668.txt.

Copy-Paste

Следующие фрагменты кода, скорее всего, получились из-за использования методики Copy-Paste. Независимо от условия, выполняется одно и то же действие:

FString FPaths::CreateTempFilename(....)
{
  ....  
  const int32 PathLen = FCString::Strlen( Path );
  if( PathLen > 0 && Path[ PathLen - 1 ] != TEXT('/') )
  {
    UniqueFilename =
      FString::Printf( TEXT("%s/%s%s%s"), Path, Prefix,
                       *FGuid::NewGuid().ToString(), Extension );
  }
  else
  {
    UniqueFilename =
      FString::Printf( TEXT("%s/%s%s%s"), Path, Prefix,
                       *FGuid::NewGuid().ToString(), Extension );
  }
  ....
}

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. paths.cpp 703

Ещё один такой фрагмент кода:

template< typename DefinitionType >            
FORCENOINLINE void Set(....)
{
  ....
  if ( DefinitionPtr == NULL )
  {
    WidgetStyleValues.Add( PropertyName,
      MakeShareable( new DefinitionType( InStyleDefintion ) ) );
  }
  else
  {
    WidgetStyleValues.Add( PropertyName,
      MakeShareable( new DefinitionType( InStyleDefintion ) ) );
  }
}

Предупреждение PVS-Studio: V523 The 'then' statement is equivalent to the 'else' statement. slatestyle.h 289

Разное

Осталась разная мелочь. Описывать её не интересно. Просто приведу фрагменты кода и диагностические сообщения.

void FNativeClassHeaderGenerator::ExportProperties(....)
{
  ....
  int32 NumByteProperties = 0;
  ....
  if (bIsByteProperty)
  {
    NumByteProperties;
  }
  ....
}

Предупреждение PVS-Studio: V607 Ownerless expression 'NumByteProperties'. codegenerator.cpp 633

static void GetModuleVersion( .... )
{
  ....
  char* VersionInfo = new char[InfoSize];
  ....
  delete VersionInfo;
  ....
}

Предупреждение PVS-Studio: 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 [] VersionInfo;'. windowsplatformexceptionhandling.cpp 107

const FSlateBrush* FSlateGameResources::GetBrush(
  const FName PropertyName, ....)
{
  ....
  ensureMsgf(BrushAsset, TEXT("Could not find resource '%s'"),
             PropertyName);
  ....
}

Предупреждение PVS-Studio: V510 The 'EnsureNotFalseFormatted' function is not expected to receive class-type variable as sixth actual argument. slategameresources.cpp 49

Выводы

Использовать статический анализатор, встроенный в Visual Studio, полезно, но не достаточно. Разумно использовать и специализированные инструменты, такие как наш анализатор PVS-Studio. Например, если сравнивать PVS-Studio и статический анализатора из VS2013, то PVS-Studio находит в 6 раз больше ошибок. Чтобы не быть голословным - "Методология сравнения".

Приглашаю всех ценителей качественного кода попробовать наш анализатор кода.