Хабрахабр

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

Picture 5

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

Введение

Ранее мы уже писали статью о проверке Unity. На тот момент для анализа было доступно не так много C#-кода: некоторые компоненты, библиотеки и примеры использования. Тем не менее, автору статьи удалось обнаружить довольно интересные ошибки.

Я говорю «порадовал» и надеюсь, что не обижу этим авторов проекта. Чем же Unity порадовал на этот раз? Это немало, и анализатор имел весьма широкое поле для деятельности. Тем более, что объем исходного C#-кода Unity, представленного на GitHub, составляет около 400 тысяч строк (без учета пустых) в 2058 файлах с расширением «cs».

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

Разработчики или энтузиасты могут легко провести глубокий анализ, выполнив проверку Unity самостоятельно. Я не стал изучать абсолютно все полученные предупреждения, так как их достаточно много. Также компании могут купить наш продукт и получить помимо лицензии быструю и подробную поддержку. Для этого у PVS-Studio предусмотрены триальный и бесплатный режимы использования.

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

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

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

Public' to the left and to the right of the '|' operator. Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'MethodAttributes. SyncListStructProcessor.cs 240

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

ARGBFloat' to the left and to the right of the '||' operator. Предупреждение PVS-Studio: V3001 There are identical sub-expressions 'format == RenderTextureFormat. RenderTextureEditor.cs 87

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. В оригинальном коде это выглядит несколько иначе:

Picture 3

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

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

Perhaps this is a mistake. Предупреждение PVS-Studio: V3008 CWE-563 The 'fail' variable is assigned values twice successively. UNetWeaver.cs 1633
Check lines: 1633, 1632.

class Weaver
return true; }
....
}

Переменной дважды присваивается значение true, так как Weaver.fail и fail — это одно и то же статическое поле класса Weaver. Возможно, грубой ошибки тут и нет, но код определенно требует внимания.

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

ProjectBrowser.cs 1417
Предупреждение PVS-Studio: V3009 CWE-393 It's odd that this method always returns one and the same value of 'false'.

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

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

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

AnimationRecording.cs 455
Предупреждение PVS-Studio: V3010 CWE-252 The return value of function 'Concat' is required to be utilized.

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

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

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

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

Check variables 'obj', 'newResolution'. Предупреждение PVS-Studio: V3019 CWE-697 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. GameViewSizesMenuItemProvider.cs 104

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, а отладочный вывод не будет произведен. Исправленный вариант кода мог бы иметь вид:

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

Недоработка

PolygonCollider2DEditor.cs 96
Предупреждение PVS-Studio: V3020 CWE-670 An unconditional 'return' within a loop.

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

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

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

The first 'if' statement contains method return. Предупреждение PVS-Studio: V3021 CWE-561 There are two 'if' statements with identical conditional expressions. This means that the second 'if' statement is senseless CustomScriptAssembly.cs 179

public bool IsCompatibleWith(....)
{ .... if (buildingForEditor) return IsCompatibleWithEditor(); if (buildingForEditor) buildTarget = BuildTarget.NoTarget; // Editor ....
}

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

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

Length' is always false. Предупреждение PVS-Studio: V3022 CWE-570 Expression 'index < 0 && index >= parameters. AnimatorControllerPlayable.bindings.cs 287

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. Правда, сообщение об ошибке будет несколько иным. Для того, чтобы код вел себя так, как ожидает разработчик, необходимо вместо оператора && в условии использовать ||:

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 присутствует ещё одна точно такая же ошибка:

Length' is always false. Предупреждение PVS-Studio: V3022 CWE-570 Expression 'index < 0 && index >= parameters. Animator.bindings.cs 711

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

Length' is always false. Предупреждение PVS-Studio: V3022 CWE-570 Expression 'handle.valueIndex < 0 && handle.valueIndex >= list. StyleSheet.cs 81

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. Для исправления ошибки необходимо, как и в предыдущих фрагментах кода, использовать в условии оператор || вместо &&.

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

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

GetSpatializerPluginName() == «GVR Audio Spatializer»)' is always true. Предупреждение PVS-Studio: V3022 CWE-571 Expression 'bRegisterAllDefinitions || (AudioSettings. AudioExtensions.cs 463

GetAmbisonicDecoderPluginName() == «GVR Audio Spatializer»)' is always true. Предупреждение PVS-Studio: V3022 CWE-571 Expression 'bRegisterAllDefinitions || (AudioSettings. AudioExtensions.cs 467

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

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

if (!m_BuiltinDefinitionsRegistered)
{ m_BuiltinDefinitionsRegistered = true;
}

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

GetConnectionState() != HolographicStreamerConnectionState. Предупреждение PVS-Studio: V3022 CWE-570 Expression 'PerceptionRemotingPlugin. HolographicEmulationWindow.cs 171
Disconnected' is always false.

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

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

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

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

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

GetConnectionState() == HolographicStreamerConnectionState. Предупреждение PVS-Studio: V3022 CWE-570 Expression 'PerceptionRemotingPlugin. HolographicEmulationWindow.cs 177
Connected' is always false.

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

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

private bool IsConnectedToRemoteDevice()
{ return false;
}

Как видим, среди предупреждений V3022 нашлось немало интересных. Вероятно, если потратить ещё какое-то время, можно увеличить список. Но давайте двигаться дальше.

Не по формату

A different number of format items is expected while calling 'Format' function. Предупреждение PVS-Studio: V3025 CWE-685 Incorrect format. Physics2D.bindings.cs 2823
Arguments not used: index.

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.»). В таком случае было бы выброшено исключение. Но и в нашем случае необходима аккуратность при рефакторинге. Код нужно исправить так:

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

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

Examine the substrings 'UnityEngine.' and 'UnityEngine. Предупреждение PVS-Studio: V3053 An excessive expression. StackTrace.cs 43
SetupCoroutine'.

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.». Таким образом, следует удалить последнюю проверку, либо уточнить правильность подстрок.

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

Examine the substrings 'Windows.dll' and 'Windows.'. Предупреждение PVS-Studio: V3053 An excessive expression. AssemblyHelper.cs 84

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

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

UNETInterface.cs 584
Предупреждение PVS-Studio: V3063 CWE-571 A part of conditional expression is always true if it is evaluated: pageSize <= 1000.

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

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

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

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

Consider inspecting denominator '(float)(width — 1)'. Предупреждение PVS-Studio: V3064 CWE-369 Potential division by zero. ClothInspector.cs 249

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

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

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

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

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

Consider inspecting 'm_Parent'. Предупреждение PVS-Studio: V3080 CWE-476 Possible null dereference. EditorWindow.cs 449

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

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

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

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

Consider inspecting 'm_Parent'. Предупреждение PVS-Studio: V3080 CWE-476 Possible null dereference. EditorWindow.cs 470

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

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

Consider inspecting 'objects'. Предупреждение PVS-Studio: V3080 CWE-476 Possible null dereference. TypeSelectionList.cs 48

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

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

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

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

Consider inspecting 'm_RowRects'. Предупреждение PVS-Studio: V3080 CWE-476 Possible null dereference. TreeViewControlGUI.cs 272

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

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

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

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

Consider inspecting 'additionalOptions'. Предупреждение PVS-Studio: V3080 CWE-476 Possible null dereference. MonoCrossCompile.cs 279

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

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

Как видим, среди предупреждений с номером V3080 присутствуют довольно коварные ошибки.

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

Check lines: 101, 107. Предупреждение PVS-Studio: V3095 CWE-476 The 'element' object was used before it was verified against null. StyleContext.cs 101

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

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

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
  • Предупреждение PVS-Studio: 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

CurveEditorSelection.cs 74
Предупреждение PVS-Studio: V3115 CWE-684 Passing 'null' to 'Equals' method should not result in 'NullReferenceException'.

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 приведен в документации.

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

  • 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

Check lines: 184, 180. Предупреждение PVS-Studio: V3125 CWE-476 The 'camera' object was used after it was verified against null. ARBackgroundRenderer.cs 184

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. А вот далее по коду это сделать забывают. Исправленный вариант мог бы иметь вид:

protected void DisableARBackgroundRendering()
{ .... if (camera != null) { camera.clearFlags = m_CameraClearFlags; // Command buffer camera.RemoveCommandBuffer(CameraEvent.BeforeForwardOpaque, m_CommandBuffer); camera.RemoveCommandBuffer(CameraEvent.BeforeGBuffer, m_CommandBuffer); }
}

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

Check lines: 88, 85. Предупреждение PVS-Studio: V3125 CWE-476 The 'item' object was used after it was verified against null. TreeViewForAudioMixerGroups.cs 88

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 ненулевая. Исправленный вариант кода:

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

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

HolographicEmulationWindow.cs 261
Предупреждение PVS-Studio: V3136 CWE-691 Constant expression in switch statement.

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

И тут оказался виноват метод PerceptionRemotingPlugin.GetConnectionState(). Мы уже сталкивались с ним, когда изучали предупреждения V3022:

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

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

Выводы

Picture 6

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

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

Используйте PVS-Studio и безбажного всем кода!

Checking the Unity C# Source Code Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Khrenov.

Показать больше

Похожие публикации

Добавить комментарий

Ваш адрес email не будет опубликован. Обязательные поля помечены *

Кнопка «Наверх»