﻿# Проверяем исходный C\#\-код Unity

Недавно произошло долгожданное для многих событие \- компания Unity Technologies разместила исходный C\#\-код игрового движка Unity для свободного скачивания на GitHub\. Представлен код движка и редактора\. Конечно, мы не могли пройти мимо, тем более, что в последнее время мы пишем не так много статей о проверке проектов на C\#\. Unity разрешает использовать предоставленные исходники только в справочных целях\. Именно так и поступим\. Испытаем последнюю на данный момент версию PVS\-Studio 6\.23 на коде Unity\.

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

## Введение

Ранее мы уже писали [статью](https://pvs-studio.ru/ru/blog/posts/csharp/0423/) о проверке Unity\. На тот момент для анализа было доступно не так много C\#\-кода: некоторые компоненты, библиотеки и примеры использования\. Тем не менее, автору статьи удалось обнаружить довольно интересные ошибки\.

Чем же Unity порадовал на этот раз? Я говорю "порадовал" и надеюсь, что не обижу этим авторов проекта\. Тем более, что объем исходного C\#\-кода Unity, представленного на [GitHub](https://github.com/Unity-Technologies/UnityCsReference), составляет около 400 тысяч строк \(без учета пустых\) в 2058 файлах с расширением "cs"\. Это немало, и анализатор имел весьма широкое поле для деятельности\.

Теперь о результатах\. Перед началом анализа я немного упростил себе работу, включив режим отображения кодов по классификации CWE для найденных ошибок, а также активировав режим подавления предупреждений третьего уровня достоверности \(Low\)\. Эти настройки доступны в выпадающем меню PVS\-Studio среды разработки Visual Studio, а также в параметрах анализатора\. Избавившись таким образом от предупреждений с низкой достоверностью, я провел анализ исходного кода Unity, в результате которого было получено 181 предупреждение первого уровня достоверности \(High\) и 506 предупреждений второго уровня достоверности \(Medium\)\.

Я не стал изучать абсолютно все полученные предупреждения, так как их достаточно много\. Разработчики или энтузиасты могут легко провести глубокий анализ, выполнив проверку Unity самостоятельно\. Для этого у PVS\-Studio предусмотрены триальный и [бесплатный](https://pvs-studio.ru/ru/blog/posts/0457/) режимы использования\. Также компании могут [купить наш продукт](https://pvs-studio.ru/ru/order/) и получить помимо лицензии быструю и подробную поддержку\.

Судя по тому, что практически в каждой группе предупреждений мне с одной\-двух попыток сразу удавалось найти реальную ошибку \- их в Unity немало\. И да, они разнообразные\. Давайте изучим наиболее интересные ошибки\.

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

**Что\-то не так с флагами**

**Предупреждение PVS\-Studio:** [V3001](https://pvs-studio.ru/ru/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);
  ....
}
```

При комбинировании флагов перечисления _MethodAttributes_ была допущена ошибка: значение _Public_ использовано дважды\. Возможно, виной тому неудачное форматирование кода\.

Аналогичная ошибка допущена и в коде метода _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:** [V3001](https://pvs-studio.ru/ru/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);
}
```

Я привел фрагмент кода, предварительно отформатировав его, поэтому ошибка легко обнаруживается визуально: дважды производится сравнение с _RenderTextureFormat\.ARGBFloat_\. В оригинальном коде это выглядит несколько иначе:

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

Вероятно, в одном из двух одинаковых сравнений необходимо использовать другое значение перечисления _RenderTextureFormat_\.

**Двойная работа**

**Предупреждение PVS\-Studio:** [V3008](https://pvs-studio.ru/ru/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;
  }
....
}
```

Переменной дважды присваивается значение _true_, так как _Weaver\.fail_ и _fail_ \- это одно и то же статическое поле класса _Weaver_\. Возможно, грубой ошибки тут и нет, но код определенно требует внимания\.

**Без вариантов**

**Предупреждение PVS\-Studio:** [V3009](https://pvs-studio.ru/ru/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;
}
```

Метод всегда возвращает _false_\. Обратите внимание на комментарий в начале\.

**Забыли про результат**

**Предупреждение PVS\-Studio:** [V3010](https://pvs-studio.ru/ru/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();
}
```

При конкатенации двух массивов _discardedModifications_ и _discardedRotationModifications_ забыли сохранить результат\. Вероятно, программист предположил, что результат будет отражен сразу в массиве _discardedModifications_\. Но это не так\. В результате из метода возвращают исходный вариант массива _discardedModifications_\. Код необходимо исправить следующим образом:

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

**Не то проверили**

**Предупреждение PVS\-Studio:** [V3019](https://pvs-studio.ru/ru/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;
}
```

В данном методе забыли предусмотреть ситуацию, когда переменная _obj_ не будет равна _null_, но её не удастся привести к типу _GameViewSize_\. Тогда переменная _newResolution_ получит значение _null_, а отладочный вывод не будет произведен\. Исправленный вариант кода мог бы иметь вид:

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

**Недоработка**

**Предупреждение PVS\-Studio:** [V3020](https://pvs-studio.ru/ru/docs/warnings/v3020/) CWE\-670 An unconditional 'return' within a loop\. PolygonCollider2DEditor\.cs 96

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

Цикл выполнит только одну итерацию, после чего метод завершит работу\. Вероятны различные сценарии\. Например, _return_ должен находиться внутри блока _if_, либо где\-то перед _return_ пропущена директива _continue_\. Вполне может быть, что ошибки тут и нет, но тогда следует сделать код более понятным\.

**Недостижимый код**

**Предупреждение PVS\-Studio:** [V3021](https://pvs-studio.ru/ru/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
  ....
}
```

Две одинаковые проверки, следующие одна за другой\. Очевидно, что в случае равенства _buildingForEditor_ значению _true_, вторая проверка лишена смысла, так как в результате первой метод завершает работу\. Если же значение _buildingForEditor_ \- _false_, то не будет выполнена ни then\-ветвь первого оператора _if_, ни второго\. Налицо ошибочная конструкция, требующая исправления\.

**Безусловное условие**

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/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];
}
```

Условие проверки индекса некорректно \- результатом всегда будет _false_\. Тем не менее, в случае передачи в метод_ GetParameter _ошибочного индекса, исключение _IndexOutOfRangeException_ все же будет выброшено, но уже при попытке доступа к элементу массива в блоке _return_\. Правда, сообщение об ошибке будет несколько иным\. Для того, чтобы код вел себя так, как ожидает разработчик, необходимо вместо оператора && в условии использовать \|\|:

```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];
}
```

Вероятно, вследствие использования методики Copy\-Paste, в коде Unity присутствует ещё одна точно такая же ошибка:

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) CWE\-570 Expression 'index < 0 && index \>\= parameters\.Length' is always false\. Animator\.bindings\.cs 711

И ещё одна похожая ошибка, также связанная с некорректным условием проверки индекса массива:

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/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;
}
```

И в этом случае возможен выброс исключения _IndexOutOfRangeException\._ Для исправления ошибки необходимо, как и в предыдущих фрагментах кода, использовать в условии оператор \|\| вместо &&\.

**Просто странный код**

На приведенный далее фрагмент кода указывают сразу два предупреждения: 

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) CWE\-571 Expression 'bRegisterAllDefinitions \|\| \(AudioSettings\.GetSpatializerPluginName\(\) \=\= "GVR Audio Spatializer"\)' is always true\. AudioExtensions\.cs 463

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/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;
  }
}
```

Выглядит, как незавершённый метод\. Непонятно, почему его оставили в таком виде и не закомментировали бесполезные блоки кода\. Всё, что метод делает в настоящее время:

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

**Бесполезный метод**

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/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();
}
```

Для прояснения ситуации необходимо взглянуть на объявление метода _PerceptionRemotingPlugin\.GetConnectionState\(\)_:

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

Таким образом, вызов метода _Disconnect\(\)_ ни к чему не приводит\.

С тем же методом _PerceptionRemotingPlugin\.GetConnectionState\(\)_ связана ещё одна ошибка:

**Предупреждение PVS\-Studio:** [V3022](https://pvs-studio.ru/ru/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;
}
```

Результат работы метода эквивалентен следующему:

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

Как видим, среди предупреждений [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/) нашлось немало интересных\. Вероятно, если потратить ещё какое\-то время, можно увеличить список\. Но давайте двигаться дальше\.

**Не по формату**

**Предупреждение PVS\-Studio:** [V3025](https://pvs-studio.ru/ru/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));
  ....
}
```

Ошибки нет, но код, как говорится, "с запахом"\. Вероятно, ранее сообщение было более информативным, наподобие такого: _"Negative path index \{0\} is invalid\."_\. Затем его упростили, но параметр _index_ для метода _Format_ убрать забыли\. Конечно, это не то же самое, как забытый параметр для указанного спецификатора вывода в строку, то есть конструкция вида _String\.Format\("Negative path index \{0\} is invalid\."\)_\. В таком случае было бы выброшено исключение\. Но и в нашем случае необходима аккуратность при рефакторинге\. Код нужно исправить так:

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

**Подстрока подстроки**

**Предупреждение PVS\-Studio:** [V3053](https://pvs-studio.ru/ru/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");
}
```

Поиск подстроки "UnityEngine\.SetupCoroutine" в условии лишен всякого смысла, так как перед этим производится поиск "UnityEngine\."\. Таким образом, следует удалить последнюю проверку, либо уточнить правильность подстрок\.

Ещё одна подобная ошибка:

**Предупреждение PVS\-Studio:** [V3053](https://pvs-studio.ru/ru/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;
}
```

**Размер имеет значение**

**Предупреждение PVS\-Studio:** [V3063](https://pvs-studio.ru/ru/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;
}
```

Условие для проверки допустимого размера страницы ошибочно\. Вместо оператора \|\| необходимо использовать &&\. Исправленный код:

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

**Возможно деление на ноль**

**Предупреждение PVS\-Studio:** [V3064](https://pvs-studio.ru/ru/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));
  ....
}
```

Проблема может возникнуть при передаче в метод значения _width \= 1_\. В самом методе это никак не проверяется\. Метод _GenerateColorTexture_ вызывается в коде всего один раз с параметром 100:

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

Так что ошибки пока нет\. Но, на всякий случай, в методе _GenerateColorTexture_ следует предусмотреть возможность передачи некорректного значения ширины\.

**Парадоксальная проверка**

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/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(....);
    ....
  }
}
```

Вероятно, вследствие опечатки, выполнение данного кода гарантирует использование нулевой ссылки _m\_Parent_\. Исправленный код:

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

Точно такая же ошибка встречается и далее в коде:

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/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(....);
    ....
}
```

А вот ещё одна интересная ошибка, которая может привести к доступу по нулевой ссылке вследствие некорректной проверки:

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/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);
  ....
}
```

Мне кажется, что разработчики Unity довольно часто допускают ошибки, связанные с неправильным использованием операторов \|\| и && в условиях\. В данном случае, если _objects_ будет иметь значение _null_, то это приведет к проверке второй части условия _\(objects \!\= null \|\| objects\.Length \>\= 1\)_, что повлечет за собой непредвиденный выброс исключения\. Ошибку необходимо исправить следующим образом:

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

**Рано обнулили**

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/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));
  }
  ....
}
```

В данном случае выброс исключения \(доступ по нулевой ссылке _m\_RowRects_\) произойдет при формировании строки сообщения для другого исключения\. Код можно исправить, например, так:

```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));
  }
  ....
}
```

**Снова ошибка при проверке**

**Предупреждение PVS\-Studio:** [V3080](https://pvs-studio.ru/ru/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() + ",";  
  ....
}
```

Из\-за того, что в условии использован оператор &, вторая часть условия будет проверена всегда, вне зависимости от результата проверки первой части\. В случае, если переменная _additionalOptions_ будет иметь значение _null_, неизбежен выброс исключения\. Ошибку необходимо исправить, использовав оператор && вместо &\.

Как видим, среди предупреждений с номером [V3080](https://pvs-studio.ru/ru/docs/warnings/v3080/) присутствуют довольно коварные ошибки\.

**Запоздалая проверка**

**Предупреждение PVS\-Studio:** [V3095](https://pvs-studio.ru/ru/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)
  {
    ....
  }
  ....
}
```

Переменную _element_ используют без предварительной проверки на неравенство _null_\. При этом далее в коде такая проверка выполняется\. Код, вероятно, необходимо исправить таким образом:

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

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

В коде есть ещё 18 подобных ошибок\. Приведу списком первые 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

**Некорректный метод Equals**

**Предупреждение PVS\-Studio:** [V3115](https://pvs-studio.ru/ru/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;
}
```

Перегрузка метода _Equals_ выполнена небрежно\. Необходимо учесть возможность получения _null_ в качестве параметра, так как это может привести к выбросу исключения, которое не было предусмотрено в вызывающем коде\. Также к выбросу исключения приведет ситуация, когда не удастся привести _\_other_ к типу _CurveSelection_\. Код требует исправления\. Хороший пример реализации перегрузки _Equals_ приведен в [документации](https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/statements-expressions-operators/how-to-define-value-equality-for-a-type)\.

В коде есть и другие подобные ошибки:

* 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

**И снова о проверке на неравенство null**

**Предупреждение PVS\-Studio:** [V3125](https://pvs-studio.ru/ru/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);
}
```

При первом использовании переменной _camera_, её проверяют на неравенство _null_\. А вот далее по коду это сделать забывают\. Исправленный вариант мог бы иметь вид:

```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);
  }
}
```

Ещё одна подобная ошибка:

**Предупреждение PVS\-Studio:** [V3125](https://pvs-studio.ru/ru/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;
}
```

Допущена ошибка, приводящая в некоторых случаях к доступу по нулевой ссылке\. Выполнение условия в первом блоке _if_ обеспечивает выход из метода\. Однако если этого не происходит, то нет гарантий, что ссылка _item_ ненулевая\. Исправленный вариант кода:

```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;
}
```

В коде есть ещё 12 аналогичных ошибок\. Приведу списком первые 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

**Выбор оказался невелик**

**Предупреждение PVS\-Studio:** [V3136](https://pvs-studio.ru/ru/docs/warnings/v3136/) CWE\-691 Constant expression in switch statement\. HolographicEmulationWindow\.cs 261

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

И тут оказался виноват метод _PerceptionRemotingPlugin\.GetConnectionState\(\)_\. Мы уже сталкивались с ним, когда изучали предупреждения [V3022](https://pvs-studio.ru/ru/docs/warnings/v3022/):

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

Метод вернет константу\. Очень странный код\. Необходимо обратить на него пристальное внимание\.

## Выводы

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

Думаю, на этом можно остановиться, иначе статья станет скучной и затянутой\. Повторюсь, я привел ошибки, которые мне сразу бросились в глаза\. Код Unity, несомненно, содержит большее число ошибочных или некорректных конструкций, требующих исправления\. Трудность состоит в том, что многие из выданных предупреждений носят весьма спорный характер и точный "диагноз" в каждом конкретном случае способен поставить только автор кода\.

В целом о проекте Unity можно сказать, что он богат на ошибки, но с учётом объема его кодовой базы \(400 тысяч строк\), всё не так плохо\. Тем не менее, надеюсь, что авторы не будут пренебрегать инструментами анализа кода для улучшения качества своего продукта\.

Используйте [PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/download/) и безбажного всем кода\!