Static Code Analysis With PVS-Studio (Part 6)

Here we go again with another compendium of miscellaneous findings across the Unreal codebase, this time delving in to version 4.15 to see how things are shaping up…

The first thing we’ll look at:-

V768 The enumeration constant ‘RQT_AbsoluteTime’ is used as a variable of a Boolean-type.

Here’s the code we should be interested in, found in OpenGLQuery.cpp:-

check(QueryType == RQT_Occlusion || RQT_AbsoluteTime);

Yep, really obvious mistake here. This can of course be changed to:-

check(QueryType == RQT_Occlusion || QueryType == RQT_AbsoluteTime);

The same warning was appearing for some other code:-

V768 The enumeration constant ‘LAUNCHERSERVICES_SHAREABLEPROJECTPATHS’ is used as a variable of a Boolean-type.

Here’s where we should be looking, in LauncherProfile.h (~line 833 as of 4.15.0):-

if (LAUNCHERSERVICES_SHAREABLEPROJECTPATHS)
{
   FullProjectPath = FPaths::ConvertRelativePathToFull(FPaths::RootDir(), ShareableProjectPath);
}

FString DeployPlatformString = DefaultDeployPlatform.ToString();
if (Version >= LAUNCHERSERVICES_FIXCOMPRESSIONSERIALIZE)
{

Yep, let’s change that first line:-

if (Version >= LAUNCHERSERVICES_SHAREABLEPROJECTPATHS)

Moving right along:-

V767 Suspicious access to element of ‘SelectedObjects’ array by a constant index inside a loop.

This time we’ll find this in SkeletonNotifyDetails.cpp:-

void FSkeletonNotifyDetails::CustomizeDetails( IDetailLayoutBuilder& DetailBuilder )
{
...
   UEditorSkeletonNotifyObj* EdObj = NULL;
   for(int i = 0; i < SelectedObjects.Num(); ++i)
   {
      UObject* Obj = SelectedObjects[0].Get();
      EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);
      if(EdObj)
      {
         break;
      }
   }

And as you’ll see on line 7 above, we’re always pulling in the same object… so we can happily fix this with:-

UObject* Obj = SelectedObjects[i].Get();

Next up, something that could simply never have worked…

V547 Expression ‘* p && * p < L’0’ && * p > L’9” is always false.

The offending code (in OpenGLShaderCompiler.cpp):-

// Skip to a number, take that to be the line number. 
while (*p && *p < TEXT('0') && *p > TEXT('9')) { p++; }
while (*p && *p >= TEXT('0') && *p <= TEXT('9'))
{
   LineNumber = 10 * LineNumber + (*p++ - TEXT('0'));
}

As you’ll see, line 2 is totally redundant as-is… but thankfully the comment above hints at what the code is meant to do. So we can fix this by replacing line 2 with the following – leaving the rest of the code as it is:-

while (*p && (*p < TEXT('0') || *p > TEXT('9'))) { p++; }

Here’s one that I’ll just quickly mention:-

V570 The ‘PkgInfo->TimeStamp’ variable is assigned to itself.

The code, found in PackageDependencyInfo:-

// Start off with setting the dependent time to the package itself
   PkgInfo->TimeStamp = PkgInfo->TimeStamp;

To be totally honest, I just laughed at this one and moved on – I assume it should be doing something like FDateTime::Now() … but I’ll leave this for someone else to fix up as I can’t be totally sure that that’s the right thing to do (and didn’t have the time to investigate more)..

Finally for today, a miscalculation in resource sizes is evident… hinted at with:-

V568 It’s odd that ‘sizeof()’ operator evaluates the size of a pointer to a class, but not the size of the ‘AssetImportData’ class object.

Here’s the code we should be looking at (found in GeometryCache.cpp):-

void UGeometryCache::GetResourceSizeEx(FResourceSizeEx& CumulativeResourceSize)
{
   Super::GetResourceSizeEx(CumulativeResourceSize);
#if WITH_EDITORONLY_DATA
   CumulativeResourceSize.AddDedicatedSystemMemoryBytes(sizeof(AssetImportData));
#endif

AssetImportData is found here:-

class UAssetImportData* AssetImportData;

So, yeah, we can go ahead and change that sizeof() line to be:-

CumulativeResourceSize.AddDedicatedSystemMemoryBytes(sizeof(UAssetImportData));

As Leszek Godlewski suggests, though, a more future-proof solution would be:-

CumulativeResourceSize.AddDedicatedSystemMemoryBytes(sizeof(*AssetImportData));

And… that’s it! It’s great to see that PVS-Studio is still doing a great job finding problems in the source code. Many thanks to the brilliant guys at Viva64 for the constant iteration on the software with regular updates! Until next time…

Credit(s): Robert Troughton (Coconut Lizard),
Evgeniy Ryzhkov (Viva64)
Status: Currently unfixed in 4.15