# Checking the Unity C\# Source Code

Recently a long\-awaited event has happened \- Unity Technologies uploaded the C\# source code of the game engine, available for free download on GitHub\. The code of the engine and the editor is available\.  Of course, we couldn't pass up, especially since lately we've not written so many articles about checking projects on C\#\. Unity allows to use the provided sources only for information purposes\. We'll use them exactly in these ways\.  Let's try out the latest version PVS\-Studio 6\.23 on the Unity code\.

![0568_UnityCS/image1.png](https://import.viva64.com/docx/blog/0568_UnityCS/image1.png)

## Introduction

Previously we've written [an article](https://pvs-studio.com/en/blog/posts/csharp/0423/) about checking Unity\. At that time so much C\#\-code was not available for the analysis: some components, libraries and examples of usage\. However, the author of the article managed to find quite interesting bugs\.

How did Unity please us this time? I'm saying "please" and hope not to offend the authors of the project\. Especially since the amount of the source Unity C\#\-code, presented on [GitHub](https://github.com/Unity-Technologies/UnityCsReference), is about 400 thousand lines \(excluding empty\) in 2058 files with the extension "cs"\.  It's a lot, and the analyzer had a quite considerable scope\.

Now about the results\. Before the analysis, I've slightly simplified the work, having enabled the mode of the code display according to the CWE classification for the found bugs\. I've also activated the warnings suppression mechanism of the third level of certainty \(Low\)\. These settings are available in the drop\-down menu of PVS\-Studio in Visual Studio development environment, and in the parameters of the analyzer\. Getting rid of the warnings with low certainty, I made the analysis of the Unity source code\. As a result, I got 181 warnings of the first level of certainty \(High\) and 506 warnings of the second level of certainty \(Medium\)\. 

I have not studied absolutely all the warnings, because there were quite a lot of them\. Developers or enthusiasts can easily conduct an in\-depth analysis by testing Unity themselves\. To do this, PVS\-Studio provides free trial and [free](https://pvs-studio.com/en/blog/posts/0457/) modes of using\. Companies can also [buy our product](https://pvs-studio.com/en/order/) and get quick and detailed support along with the license\.

Judging by the fact that I immediately managed to find couple of real bugs practically in every group of warnings with one or two attempts, there are a lot of them in Unity\.  And yes, they are diverse\. Let's review the most interesting errors\.

## Results of the check

**Something's wrong with the flags**

**PVS\-Studio warning:** [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'MethodAttributes\.Public' to the left and to the right of the '\|' operator\. SyncListStructProcessor\.cs 240

```cpp
MethodReference GenerateSerialization()
{
  ....
  MethodDefinition serializeFunc = new
      MethodDefinition("SerializeItem", MethodAttributes.Public |
            MethodAttributes.Virtual |
            MethodAttributes.Public |  // <=
            MethodAttributes.HideBySig,
            Weaver.voidType);
  ....
}
```

When combining enumeration flags _MethodAttributes,_ an error was made: the _Public_ value was used twice\. Perhaps, the reason for this is the wrong code formatting\.

A similar bug is also made in code of the method _GenerateDeserialization_:

* V3001 There are identical sub\-expressions 'MethodAttributes\.Public' to the left and to the right of the '\|' operator\. SyncListStructProcessor\.cs 309

**Copy\-Paste**

**PVS\-Studio warning:** [V3001](https://pvs-studio.com/en/docs/warnings/v3001/) There are identical sub\-expressions 'format \=\= RenderTextureFormat\.ARGBFloat' to the left and to the right of the '\|\|' operator\. RenderTextureEditor\.cs 87

```cpp
public static bool IsHDRFormat(RenderTextureFormat format)
{
  Return (format == RenderTextureFormat.ARGBHalf ||
    format == RenderTextureFormat.RGB111110Float ||
    format == RenderTextureFormat.RGFloat ||
    format == RenderTextureFormat.ARGBFloat ||
    format == RenderTextureFormat.ARGBFloat ||
    format == RenderTextureFormat.RFloat ||
    format == RenderTextureFormat.RGHalf ||
    format == RenderTextureFormat.RHalf);
}
```

I gave a piece of code, preliminary having formatted it, so the error is easily detected visually: the comparison with _RenderTextureFormat\.ARGBFloat_ is performed twice\. In the original code, it looks differently:

![0568_UnityCS/image3.png](https://import.viva64.com/docx/blog/0568_UnityCS/image3.png)

Probably, another value of enumeration _RenderTextureFormat_ has to be used in one of two identical comparisons\. 

**Double work**

**PVS\-Studio warning:** [V3008](https://pvs-studio.com/en/docs/warnings/v3008/) CWE\-563 The 'fail' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 1633, 1632\. UNetWeaver\.cs 1633

```cpp
class Weaver
{
  ....
  public static bool fail;
  ....
  static public bool IsValidTypeToGenerate(....)
  {
    ....
    if (....)
    {
      ....
      Weaver.fail = true;
      fail = true;
      return false;
    }
    return true;
  }
....
}
```

The _true_ value is assigned twice to the value, as _Weaver\.fail_ and _fail_ is one and the same static field of the _Weaver_ class\. Perhaps, there is no crucial error, but the code definitely needs attention\.

**No options**

**PVS\-Studio warning:** [V3009](https://pvs-studio.com/en/docs/warnings/v3009/) CWE\-393 It's odd that this method always returns one and the same value of 'false'\. ProjectBrowser\.cs 1417

```cpp
// Returns true if we should early out of OnGUI
bool HandleCommandEventsForTreeView()
{
  ....
  if (....)
  {
    ....
    if (....)
      return false;
    ....
  }
  return false;
}
```

The method always returns _false_\. Pay attention to the comment in the beginning\.

**A developer forgot about the result**

**PVS\-Studio warning:** [V3010](https://pvs-studio.com/en/docs/warnings/v3010/) CWE\-252 The return value of function 'Concat' is required to be utilized\. AnimationRecording\.cs 455

```cpp
static public UndoPropertyModification[] Process(....)
{
  ....
  discardedModifications.Concat(discardedRotationModifications);
  return discardedModifications.ToArray();
}
```

When concatenating two arrays _discardedModifications_ and _discardedRotationModifications_ the author forgot to save the result\. Probably a programmer assumed that the result would be expressed immediately in the array _discardedModifications_\. But it is not so\. As a result, the original array _discardedModifications_ is returned from the method\. The code needs to be corrected as follows:

```cpp
static public UndoPropertyModification[] Process(....)
{
  ....
  return discardedModifications.Concat(discardedRotationModifications)
                               .ToArray();
}
```

**Wrong variable was checked**

**PVS\-Studio warning:** [V3019](https://pvs-studio.com/en/docs/warnings/v3019/) CWE\-697 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword\. Check variables 'obj', 'newResolution'\. GameViewSizesMenuItemProvider\.cs 104

```cpp
private static GameViewSize CastToGameViewSize(object obj)
{
  GameViewSize newResolution = obj as GameViewSize;
  if (obj == null)
  {
    Debug.LogError("Incorrect input");
    return null;
  }
  return newResolution;
}
```

In this method, the developers forgot to consider a situation where the variable _obj_ is not equal to _null_, but it will not be able to cast to the _GameViewSize _type\. Then the variable _newResolution_ will be set to _null_, and the debug output will not be made\. A correct variant of code will be like this:

```cpp
private static GameViewSize CastToGameViewSize(object obj)
{
  GameViewSize newResolution = obj as GameViewSize;
  if (newResolution == null)
  {
    Debug.LogError("Incorrect input");
  }
  return newResolution;
}
```

**Deficiency**

**PVS\-Studio warning:** [V3020](https://pvs-studio.com/en/docs/warnings/v3020/) CWE\-670 An unconditional 'return' within a loop\. PolygonCollider2DEditor\.cs 96

```cpp
private void HandleDragAndDrop(Rect targetRect)
{
  ....
  foreach (....)
  {
    ....
    if (....)
    {
      ....
    }
    return;
  }
  ....
}
```

The loop will execute only one iteration, after that the method terminates its work\. Various scenarios are probable\. For example, _return_ must be inside the unit _if_, or somewhere before _return,_ a directive _continue_ is missing\. It may well be that there is no error here, but then one should make the code more understandable\.

**Unreachable code**

**PVS\-Studio warning:** [V3021](https://pvs-studio.com/en/docs/warnings/v3021/) CWE\-561 There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains method return\. This means that the second 'if' statement is senseless CustomScriptAssembly\.cs 179

```cpp
public bool IsCompatibleWith(....)
{
  ....
  if (buildingForEditor)
    return IsCompatibleWithEditor();

  if (buildingForEditor)
    buildTarget = BuildTarget.NoTarget; // Editor
  ....
}
```

Two identical checks, following one after another\. It is clear that in case of _buildingForEditor _equality to the _true _value, the second check is meaningless, because the first method terminates its work\. If the value _buildingForEditor _is _false_, neither then\-brunch nor _if_ operator will be executed\.  There is an erroneous construction that requires correction\.

**Unconditional condition**

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-570 Expression 'index < 0 && index \>\= parameters\.Length' is always false\. AnimatorControllerPlayable\.bindings\.cs 287

```cpp
public AnimatorControllerParameter GetParameter(int index)
{
  AnimatorControllerParameter[] param = parameters;
  if (index < 0 && index >= parameters.Length)
    throw new IndexOutOfRangeException(
      "Index must be between 0 and " + parameters.Length);
  return param[index];
}
```

The condition of the index check is incorrect \- the result will always be false\. However, in case of passing the incorrect index to the_ GetParameter_ method, the exception _IndexOutOfRangeException_ will still be thrown when attempting to access an array element in the _return_ block\. Although, the error message will be slightly different\.  One has to use \|\| in a condition instead of the operator && so that the code worked the way a developer expected: 

```cpp
public AnimatorControllerParameter GetParameter(int index)
{
  AnimatorControllerParameter[] param = parameters;
  if (index < 0 || index >= parameters.Length)
    throw new IndexOutOfRangeException(
      "Index must be between 0 and " + parameters.Length);
  return param[index];
}
```

Perhaps, due to the use of the Copy\-Paste method, there is another the same error in the Unity code:

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-570 Expression 'index < 0 && index \>\= parameters\.Length' is always false\. Animator\.bindings\.cs 711

And another similar error associated with the incorrect condition of the check of the array index:

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-570 Expression 'handle\.valueIndex < 0 && handle\.valueIndex \>\= list\.Length' is always false\. StyleSheet\.cs 81

```cpp
static T CheckAccess<T>(T[] list, StyleValueType type,
  StyleValueHandle handle)
{
  T value = default(T);
  if (handle.valueType != type)
  {
    Debug.LogErrorFormat(....  );
  }
  else if (handle.valueIndex < 0 && handle.valueIndex >= list.Length)
  {
    Debug.LogError("Accessing invalid property");
  }
  else
  {
    value = list[handle.valueIndex];
  }
  return value;
}
```

And in this case, a release of the _IndexOutOfRangeException_ exception is possible\. As in the previous code fragments, one has to use the operator \|\| instead of && to fix an error\. 

**Simply strange code**

Two warnings are issued for the code fragment below\.  

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-571 Expression 'bRegisterAllDefinitions \|\| \(AudioSettings\.GetSpatializerPluginName\(\) \=\= "GVR Audio Spatializer"\)' is always true\. AudioExtensions\.cs 463

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-571 Expression 'bRegisterAllDefinitions \|\| \(AudioSettings\.GetAmbisonicDecoderPluginName\(\) \=\= "GVR Audio Spatializer"\)' is always true\. AudioExtensions\.cs 467

```cpp
// This is where we register our built-in spatializer extensions.
static private void RegisterBuiltinDefinitions()
{
  bool bRegisterAllDefinitions = true;
  
  if (!m_BuiltinDefinitionsRegistered)
  {
    if (bRegisterAllDefinitions ||
        (AudioSettings.GetSpatializerPluginName() ==
          "GVR Audio Spatializer"))
    {
    }
    
    if (bRegisterAllDefinitions ||
        (AudioSettings.GetAmbisonicDecoderPluginName() ==
          "GVR Audio Spatializer"))
    {
    }
    
    m_BuiltinDefinitionsRegistered = true;
  }
}
```

It looks like an incomplete method\. It is unclear why it has been left as such and why developers haven't commented the useless code blocks\. All, that the method does at the moment:

```cpp
if (!m_BuiltinDefinitionsRegistered)
{
  m_BuiltinDefinitionsRegistered = true;
}
```

**Useless method**

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-570 Expression 'PerceptionRemotingPlugin\.GetConnectionState\(\) \!\= HolographicStreamerConnectionState\.Disconnected' is always false\. HolographicEmulationWindow\.cs 171

```cpp
private void Disconnect()
{
  if (PerceptionRemotingPlugin.GetConnectionState() !=
      HolographicStreamerConnectionState.Disconnected)
    PerceptionRemotingPlugin.Disconnect();
}
```

To clarify the situation, it is necessary to look at the declaration of the method  _PerceptionRemotingPlugin\.GetConnectionState\(\)_:

```cpp
internal static HolographicStreamerConnectionState
GetConnectionState()
{
  return HolographicStreamerConnectionState.Disconnected;
}
```

Thus, calling the _Disconnect\(\)_ method leads to nothing\.

One more error relates to the same method _PerceptionRemotingPlugin\.GetConnectionState\(\)_:

**PVS\-Studio warning:** [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) CWE\-570 Expression 'PerceptionRemotingPlugin\.GetConnectionState\(\) \=\= HolographicStreamerConnectionState\.Connected' is always false\. HolographicEmulationWindow\.cs 177

```cpp
private bool IsConnectedToRemoteDevice()
{
  return PerceptionRemotingPlugin.GetConnectionState() ==
         HolographicStreamerConnectionState.Connected;
}
```

The result of the method is equivalent to the following:

```cpp
private bool IsConnectedToRemoteDevice()
{
  return false;
}
```

As we can see, among the warnings [V3022](https://pvs-studio.com/en/docs/warnings/v3022/) many interesting ones were found\. Probably, if one spends much time, he can increase the list\. But let's move on\.

**Not on the format**

**PVS\-Studio warning:** [V3025](https://pvs-studio.com/en/docs/warnings/v3025/) CWE\-685 Incorrect format\. A different number of format items is expected while calling 'Format' function\. Arguments not used: index\. Physics2D\.bindings\.cs 2823

```cpp
public void SetPath(....)
{
  if (index < 0)
    throw new ArgumentOutOfRangeException(
      String.Format("Negative path index is invalid.", index));
  ....
}
```

There is no error in code, but as the saying goes, the code "smells"\. Probably, an earlier message was more informative, like this: _"Negative path index \{0\} is invalid\."_\. Then it was simplified, but developers forgot to remove the parameter _index_ for the method _Format_\.  Of course, this is not the same as a forgotten parameter for the indicated output string specifier, i\.e\. the construction of the type _String\.Format\("Negative path index \{0\} is invalid\."\)_\. In such a case, an exception would be thrown\. But in our case we also need neatness when refactoring\. The code has to be fixed as follows:

```cpp
public void SetPath(....)
{
  if (index < 0)
    throw new ArgumentOutOfRangeException(
      "Negative path index is invalid.");
  ....
}
```

**Substring of the substring**

**PVS\-Studio warning:** [V3053](https://pvs-studio.com/en/docs/warnings/v3053/) An excessive expression\. Examine the substrings 'UnityEngine\.' and 'UnityEngine\.SetupCoroutine'\. StackTrace\.cs 43

```cpp
static bool IsSystemStacktraceType(object name)
{
  string casted = (string)name;
  return casted.StartsWith("UnityEditor.") ||
    casted.StartsWith("UnityEngine.") ||
    casted.StartsWith("System.") ||
    casted.StartsWith("UnityScript.Lang.") ||
    casted.StartsWith("Boo.Lang.") ||
    casted.StartsWith("UnityEngine.SetupCoroutine");
}
```

Search of the substring "UnityEngine\.SetupCoroutine" in the condition is meaningless, because before that the search for "UnityEngine\." is performed\. Therefore, the last check should be removed or one has to clarify the correctness of substrings\.

Another similar error:

**PVS\-Studio warning:** [V3053](https://pvs-studio.com/en/docs/warnings/v3053/) An excessive expression\. Examine the substrings 'Windows\.dll' and 'Windows\.'\. AssemblyHelper\.cs 84

```cpp
static private bool CouldBelongToDotNetOrWindowsRuntime(string
  assemblyPath)
{
  return assemblyPath.IndexOf("mscorlib.dll") != -1 ||
    assemblyPath.IndexOf("System.") != -1 ||
    assemblyPath.IndexOf("Windows.dll") != -1 ||  // <=
    assemblyPath.IndexOf("Microsoft.") != -1 ||
    assemblyPath.IndexOf("Windows.") != -1 ||  // <=
    assemblyPath.IndexOf("WinRTLegacy.dll") != -1 ||
    assemblyPath.IndexOf("platform.dll") != -1;
}
```

**Size does matter**

**PVS\-Studio warning:** [V3063](https://pvs-studio.com/en/docs/warnings/v3063/) CWE\-571 A part of conditional expression is always true if it is evaluated: pageSize <\= 1000\. UNETInterface\.cs 584

```cpp
public override bool IsValid()
{
  ....
  return base.IsValid()
    && (pageSize >= 1 || pageSize <= 1000)
    && totalFilters <= 10;
}
```

Condition for a check of a valid page size is erroneous\. Instead of the operator \|\|, one has to use &&\. The corrected code:

```cpp
public override bool IsValid()
{
  ....
  return base.IsValid()
    && (pageSize >= 1 && pageSize <= 1000)
    && totalFilters <= 10;
}
```

**Possible division by zero**

**PVS\-Studio warning:** [V3064](https://pvs-studio.com/en/docs/warnings/v3064/) CWE\-369 Potential division by zero\. Consider inspecting denominator '\(float\)\(width \- 1\)'\. ClothInspector\.cs 249

```cpp
Texture2D GenerateColorTexture(int width)
{
  ....
  for (int i = 0; i < width; i++)
    colors[i] = GetGradientColor(i / (float)(width - 1));
  ....
}
```

The problem may occur when passing the value _width_ _\=_ _1 _into the method\. In the method, it is not checked anyway\. The method _GenerateColorTexture_ is called in the code just once with the parameter 100:

```cpp
void OnEnable()
{
  if (s_ColorTexture == null)
    s_ColorTexture = GenerateColorTexture(100);
  ....
}
```

So, there is no error here so far\.  But, just in case, in the method _GenerateColorTexture_ the possibility of transferring incorrect width value should be provided\.

**Paradoxical check**

**PVS\-Studio warning:** [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) CWE\-476 Possible null dereference\. Consider inspecting 'm\_Parent'\. EditorWindow\.cs 449

```cpp
public void ShowPopup()
{
  if (m_Parent == null)
  {
    ....
    Rect r = m_Parent.borderSize.Add(....);
    ....
  }
}
```

Probably, due to a typo, the execution of such code guarantees the use of the null reference _m\_Parent_\. The corrected code:

```cpp
public void ShowPopup()
{
  if (m_Parent != null)
  {
    ....
    Rect r = m_Parent.borderSize.Add(....);
    ....
  }
}
```

The same error occurs later in the code:

**PVS\-Studio warning:** [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) CWE\-476 Possible null dereference\. Consider inspecting 'm\_Parent'\. EditorWindow\.cs 470

```cpp
internal void ShowWithMode(ShowMode mode)
{
  if (m_Parent == null)
  {
    ....
    Rect r = m_Parent.borderSize.Add(....);
    ....
}
```

And here's another interesting bug that can lead to access by a null reference due to incorrect check:

**PVS\-Studio warning:** [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) CWE\-476 Possible null dereference\. Consider inspecting 'objects'\. TypeSelectionList\.cs 48

```cpp
public TypeSelection(string typeName, Object[] objects)
{
  System.Diagnostics.Debug.Assert(objects != null ||
                                  objects.Length >= 1);
  ....
}
```

It seems to me that Unity developers quite often make errors related to misuse of operators \|\| and && in conditions\. In this case, if_ objects _has a null value, then this will lead to a check of second part of the condition _\(objects \!\= null \|\| objects\.Length \>\= 1\)_, which will entail the unexpected throw of an exception\. The error should be corrected as follows:

```cpp
public TypeSelection(string typeName, Object[] objects)
{
  System.Diagnostics.Debug.Assert(objects != null &&
                                  objects.Length >= 1);
  ....
}
```

**Early** **nullifying** 

**PVS\-Studio warning:** [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) CWE\-476 Possible null dereference\. Consider inspecting 'm\_RowRects'\. TreeViewControlGUI\.cs 272

```cpp
public override void GetFirstAndLastRowVisible(....)
{
  ....
  if (rowCount != m_RowRects.Count)
  {
    m_RowRects = null;
    throw new InvalidOperationException(string.Format("....",
              rowCount, m_RowRects.Count));
  }
  ....
}
```

In this case, the exception throw \(access by the null reference _m\_RowRects_\) will happen when generating the message string for another exception\. Code might be fixed, for example, as follows:

```cpp
public override void GetFirstAndLastRowVisible(....)
{
  ....
  if (rowCount != m_RowRects.Count)
  {
    var m_RowRectsCount = m_RowRects.Count;
    m_RowRects = null;
    throw new InvalidOperationException(string.Format("....",
              rowCount, m_RowRectsCount));
  }
  ....
}
```

**One** **more** **error** **when** **checking** 

**PVS\-Studio warning:** [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) CWE\-476 Possible null dereference\. Consider inspecting 'additionalOptions'\. MonoCrossCompile\.cs 279

```cpp
static void CrossCompileAOT(....)
{
  ....
  if (additionalOptions != null & additionalOptions.Trim().Length > 0)
    arguments += additionalOptions.Trim() + ",";  
  ....
}
```

Due to the fact that the & operator is used in a condition, the second part of the condition will always be checked, regardless of the result of the check of the first part\. In case if the variable _additionalOptions_ has the null value, the exception throw is inevitable\. The error has to be corrected, by using the operator && instead of &\.

As we can see, among the warnings with the number [V3080](https://pvs-studio.com/en/docs/warnings/v3080/) there are rather insidious errors\.

**Late check**

**PVS\-Studio warning:** [V3095](https://pvs-studio.com/en/docs/warnings/v3095/) CWE\-476 The 'element' object was used before it was verified against null\. Check lines: 101, 107\. StyleContext\.cs 101

```cpp
public override void OnBeginElementTest(VisualElement element, ....)
{
  if (element.IsDirty(ChangeType.Styles))
  {
    ....
  }

  if (element != null && element.styleSheets != null)
  {
    ....
  }
  ....
}
```

The variable _element is_ used without preliminary check for _null_\. While later in the code this check is performed\. The code probably needs to be corrected as follows:

```cpp
public override void OnBeginElementTest(VisualElement element, ....)
{
  if (element != null)
  {
    if (element.IsDirty(ChangeType.Styles))
    {
      ....
    }

    if (element.styleSheets != null)
    {
      ....
    }
  }
  ....
}
```

In code there are 18 more errors\. Let me give you a list of the first 10:

* V3095 CWE\-476 The 'property' object was used before it was verified against null\. Check lines: 5137, 5154\. EditorGUI\.cs 5137
* V3095 CWE\-476 The 'exposedPropertyTable' object was used before it was verified against null\. Check lines: 152, 154\. ExposedReferenceDrawer\.cs 152
* V3095 CWE\-476 The 'rectObjs' object was used before it was verified against null\. Check lines: 97, 99\. RectSelection\.cs 97
* V3095 CWE\-476 The 'm\_EditorCache' object was used before it was verified against null\. Check lines: 134, 140\. EditorCache\.cs 134
* V3095 CWE\-476 The 'setup' object was used before it was verified against null\. Check lines: 43, 47\. TreeViewExpandAnimator\.cs 43
* V3095 CWE\-476 The 'response\.job' object was used before it was verified against null\. Check lines: 88, 99\. AssetStoreClient\.cs 88
* V3095 CWE\-476 The 'compilationTask' object was used before it was verified against null\. Check lines: 1010, 1011\. EditorCompilation\.cs 1010
* V3095 CWE\-476 The 'm\_GenericPresetLibraryInspector' object was used before it was verified against null\. Check lines: 35, 36\. CurvePresetLibraryInspector\.cs 35
* V3095 CWE\-476 The 'Event\.current' object was used before it was verified against null\. Check lines: 574, 620\. AvatarMaskInspector\.cs 574
* V3095 CWE\-476 The 'm\_GenericPresetLibraryInspector' object was used before it was verified against null\. Check lines: 31, 32\. ColorPresetLibraryInspector\.cs 31

**Wrong Equals method**

**PVS\-Studio warning:** [V3115](https://pvs-studio.com/en/docs/warnings/v3115/) CWE\-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. CurveEditorSelection\.cs 74

```cpp
public override bool Equals(object _other)
{
  CurveSelection other = (CurveSelection)_other;
  return other.curveID == curveID && other.key == key &&
    other.type == type;
}
```

Overload of the _Equals_ method_ _was implemented carelessly\. One has to take into account the possibility of obtaining _null_ as a parameter, as this can lead to a throw of an exception, which hasn't been considered in the calling code\. In addition, the situation, when \__other_ can't be cast to the type_ CurveSelection,_ will lead to a throw of an exception\._ _ The code has to be fixed\. A good example of the implementation of _Object\.equals_ overload is given in the [documentation](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/statements-expressions-operators/how-to-define-value-equality-for-a-type)\.

In the code, there are other similar errors:

* V3115 CWE\-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. SpritePackerWindow\.cs 40
* V3115 CWE\-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. PlatformIconField\.cs 28
* V3115 CWE\-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. ShapeEditor\.cs 161
* V3115 CWE\-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. ActiveEditorTrackerBindings\.gen\.cs 33
* V3115 CWE\-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'\. ProfilerFrameDataView\.bindings\.cs 60

**Once again about the check for null inequality**

**PVS\-Studio warning:** [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) CWE\-476 The 'camera' object was used after it was verified against null\. Check lines: 184, 180\. ARBackgroundRenderer\.cs 184

```cpp
protected void DisableARBackgroundRendering()
{
  ....
  if (camera != null)
    camera.clearFlags = m_CameraClearFlags;

  // Command buffer
  camera.RemoveCommandBuffer(CameraEvent.BeforeForwardOpaque,
                             m_CommandBuffer);
  camera.RemoveCommandBuffer(CameraEvent.BeforeGBuffer,
                             m_CommandBuffer);
}
```

When the _camera_ variable is used the first time, it is checked for _null_ inequality\.  But further along the code the developers forget to do it\.  The correct variant could be like this: 

```cpp
protected void DisableARBackgroundRendering()
{
  ....
  if (camera != null)
  {
    camera.clearFlags = m_CameraClearFlags;

    // Command buffer
    camera.RemoveCommandBuffer(CameraEvent.BeforeForwardOpaque,
                               m_CommandBuffer);
    camera.RemoveCommandBuffer(CameraEvent.BeforeGBuffer,
                               m_CommandBuffer);
  }
}
```

Another similar error:

**PVS\-Studio warning:** [V3125](https://pvs-studio.com/en/docs/warnings/v3125/) CWE\-476 The 'item' object was used after it was verified against null\. Check lines: 88, 85\. TreeViewForAudioMixerGroups\.cs 88

```cpp
protected override Texture GetIconForItem(TreeViewItem item)
{
  if (item != null && item.icon != null)
    return item.icon;

  if (item.id == kNoneItemID) // <=
    return k_AudioListenerIcon;
  
  return k_AudioGroupIcon;
}
```

An error, that in some cases leads to an access by a null link\. The execution of the condition in the first block _if_ enables the exit from the method\. However, if this does not happen, then there is no guarantee that the reference _item_ is non\-zero\. Here is the corrected version of the code:

```cpp
protected override Texture GetIconForItem(TreeViewItem item)
{
  if (item != null)
  {
    if (item.icon != null)
      return item.icon;
    
    if (item.id == kNoneItemID)
      return k_AudioListenerIcon;
  }

  return k_AudioGroupIcon;
}
```

In the code there are 12 similar errors\. Let me give you a list of the first 10:

* V3125 CWE\-476 The 'element' object was used after it was verified against null\. Check lines: 132, 107\. StyleContext\.cs 132
* V3125 CWE\-476 The 'mi\.DeclaringType' object was used after it was verified against null\. Check lines: 68, 49\. AttributeHelper\.cs 68
* V3125 CWE\-476 The 'label' object was used after it was verified against null\. Check lines: 5016, 4999\. EditorGUI\.cs 5016
* V3125 CWE\-476 The 'Event\.current' object was used after it was verified against null\. Check lines: 277, 268\. HostView\.cs 277
* V3125 CWE\-476 The 'bpst' object was used after it was verified against null\. Check lines: 96, 92\. BuildPlayerSceneTreeView\.cs 96
* V3125 CWE\-476 The 'state' object was used after it was verified against null\. Check lines: 417, 404\. EditorGUIExt\.cs 417
* V3125 CWE\-476 The 'dock' object was used after it was verified against null\. Check lines: 370, 365\. WindowLayout\.cs 370
* V3125 CWE\-476 The 'info' object was used after it was verified against null\. Check lines: 234, 226\. AssetStoreAssetInspector\.cs 234
* V3125 CWE\-476 The 'platformProvider' object was used after it was verified against null\. Check lines: 262, 222\. CodeStrippingUtils\.cs 262
* V3125 CWE\-476 The 'm\_ControlPoints' object was used after it was verified against null\. Check lines: 373, 361\. EdgeControl\.cs 373

**The choice turned out to be small**

**PVS\-Studio warning:** [V3136](https://pvs-studio.com/en/docs/warnings/v3136/) CWE\-691 Constant expression in switch statement\. HolographicEmulationWindow\.cs 261

```cpp
void ConnectionStateGUI()
{
  ....
  HolographicStreamerConnectionState connectionState =
    PerceptionRemotingPlugin.GetConnectionState();
  switch (connectionState)
  {
    ....
  }
  ....
}
```

The method _PerceptionRemotingPlugin\.GetConnectionState\(\)_ is to blame here\.  We have already come across it when we were analyzing the warnings [V3022](https://pvs-studio.com/en/docs/warnings/v3022/):

```cpp
internal static HolographicStreamerConnectionState
  GetConnectionState()
{
  return HolographicStreamerConnectionState.Disconnected;
}
```

The method will return a constant\. This code is very strange\.  It needs to be paid attention\.

## Conclusions

![0568_UnityCS/image4.png](https://import.viva64.com/docx/blog/0568_UnityCS/image4.png)

I think we can stop at this point, otherwise the article will become boring and overextended\. Again, I listed the errors that I just couldn't miss\. Sure, the Unity code contains a big number of the erroneous and incorrect constructions, that need to be fixed\. The difficulty is that many of the issued warnings are very controversial and only the author of the code is able to make the exact "diagnosis" in each case\.

Generally speaking about the Unity project, we can say that it is rich for errors, but taking into account the size of its code base \(400 thousand lines\), it's not so bad\. Nevertheless, I hope that the authors will not neglect the code analysis tools to improve the quality of their product\.

Use [PVS\-Studio](https://pvs-studio.com/en/pvs-studio/download/) and I wish you bugless code\!