Хабрахабр

Проверка исходного кода библиотек .NET Core статическим анализатором PVS-Studio

Picture 19

Библиотеки .NET Core — один из самых популярных C# проектов на GitHub. Неудивительно, с учётом его широкой известности и используемости. Тем интереснее попробовать выяснить, какие тёмные уголки можно найти в исходном коде этих библиотек, что мы и попробуем сделать с помощью статического анализатора PVS-Studio. Как думаете, удалось ли в итоге обнаружить что-нибудь интересное?
К этой статье я шёл более полутора лет. В какой-то момент в моей голове поселилась мысль, что библиотеки .NET Core — лакомый кусочек, и их проверка будет интересной. Несколько раз я проверял проект, анализатор находил всё новые и новые интересные места, но дальше беглого пролистывания списка предупреждений дело не шло. И вот оно — свершилось! Проект проверен, статья — перед вами.

Подробнее про проект и анализ

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

Проверяемый проект

Наверное, можно было бы и не рассказывать, что такое CoreFX (библиотеки .NET Core), но, если вдруг вы не слышали, описание ниже. Я не стал его перефразировать и взял со страницы проекта на GitHub, где также можно и загрузить исходники.

NET Core. Описание: This repo contains the library implementation (called «CoreFX») for . Collections, System. It includes System. Xml, and many other components. IO, System. NET Core Runtime repo (called «CoreCLR») contains the runtime implementation for . The corresponding . It includes RyuJIT, the . NET Core. Runtime-specific library code (System. NET GC, and many other components. CoreLib) lives in the CoreCLR repo. Private. The rest of CoreFX is agnostic of runtime-implementation and can be run on any compatible . It needs to be built and versioned in tandem with the runtime. CoreRT). NET runtime (e.g.

Используемый анализатор и способ анализа

Проверял исходный код я с помощью статического анализатора PVS-Studio. Вообще PVS-Studio умеет анализировать не только C# код, но и C, C++, Java. Анализ C# кода пока работает только под Windows, в то время как код на C, C++, Java вы можете анализировать под Windows, Linux, macOS.

С CoreFX, однако, вышло чуть сложнее. Обычно для проверки C# проектов я использую плагин PVS-Studio для Visual Studio (поддерживаются версии 2010-2019), так как это, наверное, наиболее простой и удобный способ анализа: открыть solution, запустить анализ, работать со списком предупреждений.

Наверное, оно и хорошо — не очень представляю, как Visual Studio справилась бы с solution такого размера. Дело в том, что у проекта нет единого .sln файла, следовательно, открыть его в Visual Studio и провести полный анализ, используя плагин PVS-Studio, не получится.

Всё, что потребовалось от меня — написать небольшой скрипт, который бы запускал «PVS-Studio_Cmd.exe» на каждый .sln в директории CoreFX и складывал результаты анализа в отдельную директорию (указывается флагом запуска анализатора). Впрочем, никаких проблем с анализом не возникло, так как в состав дистрибутива PVS-Studio входит command line версия анализатора для MSBuild проектов (и, собственно, .sln).

— на выходе имею набор логов, в которых много интересного. Вуаля! Но мне было удобнее работать с отдельными логами, поэтому объединять их я не стал. При желании логи можно было бы объединить с помощью утилиты PlogConverter, идущей в составе дистрибутива.

Допускаю, что код, описанный в документации / находящийся в пакетах, может несколько отличаться от проанализированного. При описании некоторых ошибок я ссылаюсь на документацию с docs.microsoft.com и на NuGet пакеты, доступные для загрузки с nuget.org. Воспроизведение ошибок в пакетах из NuGet на тех же входных данных, что использовались для отладочных библиотек, показывает то, что проблема — не новая, и, что более важно, что её можно 'пощупать', не собирая проект из исходников. Тем не менее, будет очень странно, если, например, в документации нет описания генерируемых исключений при ряде входных данных, а в новой версии пакета они появятся — согласитесь, это будет сомнительный сюрприз.

Таким образом, допуская вероятность некоторой теоретической рассинхронизации кода, я нахожу допустимым обращаться к описанию соответствующих методов на docs.microsoft.com и к воспроизведению проблем на пакетах из nuget.org.

Также замечу, что описание по приводимым ссылкам, а также информация (комментарии) в пакетах (в других версиях) могли измениться в ходе написания статьи.

Другие проверенные проекты

Кстати, это ведь не уникальная в своём роде статья — мы пишем и другие статьи о проверке проектов, список которых можно найти здесь. Более того, на сайте у нас собраны не только статьи об анализе проектов, но и различные технические статьи по C, C++, C#, Java, а также просто интересные заметки. Найти всё это можно в блоге.

NET Core в 2015 году. Мой коллега ранее уже проверял библиотеки . NET Core Libraries (CoreFX)". Результаты предыдущего анализа можно найти в соответствующей статье: "Новогодняя проверка .

Обнаруженные ошибки, подозрительные и интересные места

Как и всегда, для большего интереса предлагаю сначала самостоятельно искать ошибки в приведённых фрагментах, а лишь затем читать предупреждение анализатора и описание проблемы.

Да и ссылаться на конкретные фрагменты так тоже проще. Для удобства я явно оделил рассматриваемые фрагменты друг от друга с использованием меток вида Issue N — так легче понять, где заканчивается описание одной ошибки и начинается разбор другой.

Issue 1

abstract public class Principal : IDisposable if (context == null) { Debug.Assert(this.unpersisted == true); throw new InvalidOperationException(SR.NullArguments); } .... } ....
}

Предупреждение PVS-Studio: V3095 The 'context' object was used before it was verified against null. Check lines: 340, 346. Principal.cs 340

Однако чуть выше, в предыдущем условии, происходит безусловное разыменование ссылки contextcontext. Разработчики явно обозначают, что значение null для параметра context является недопустимым, и хотят подчеркнуть это при помощи исключения типа InvalidOperationException. В итоге, если значение contextnull, вместо ожидаемого InvalidOperationExcetion будет сгенерировано исключение типа NullReferenceException. ContextType.

Подключим соответствующую библиотеку (System. Попробуем воспроизвести проблему. AccountManagement) в проект и исполняем следующий код: DirectoryServices.

GroupPrincipal groupPrincipal = new GroupPrincipal(new PrincipalContext(ContextType.Machine));
groupPrincipal.Save(null);

GroupPrincipal — наследник абстрактного класса Principal, который и содержит реализацию нужного нам метода Save. Запускаем код на исполнение и видим то, что и требовалось доказать.

Picture 1

Я поставил пакет версии 4. Ради интереса можно попробовать загрузить соответствующий пакет из NuGet и попробовать повторить проблему таким же образом. 0 и получил ожидаемый результат. 5.

Picture 2

Issue 2

private SearchResultCollection FindAll(bool findMoreThanOne)
{ searchResult = null; DirectoryEntry clonedRoot = null; if (_assertDefaultNamingContext == null) { clonedRoot = SearchRoot.CloneBrowsable(); } else { clonedRoot = SearchRoot.CloneBrowsable(); } ....
}

Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. DirectorySearcher.cs 629

Либо в какой-то ветви должно быть другое действие, либо можно опустить оператор if, чтобы не смущать программистов и анализатор. Вне зависимости от истинности условия _assertDefaultNamingContext == null будут выполнены одни и те же действия, так как then и else ветви оператора if имеют одинаковые тела.

Issue 3

public class DirectoryEntry : Component
{ .... public void RefreshCache(string[] propertyNames) { .... object[] names = new object[propertyNames.Length]; for (int i = 0; i < propertyNames.Length; i++) names[i] = propertyNames[i]; .... if (_propertyCollection != null && propertyNames != null) .... .... } ....
}

Предупреждение PVS-Studio: V3095 The 'propertyNames' object was used before it was verified against null. Check lines: 990, 1004. DirectoryEntry.cs 990

В методе есть проверка propertyNames != null, т.е. Опять видим странный порядок действий. Вот только выше можно наблюдать несколько операций обращения по этой потенциально нулевой ссылке — propertyNames. разработчики страхуют себя от того, что в метод придёт значение null. Результат вполне предсказуем — возникновение исключения типа NullReferenceExcepption в случае, если в метод передаётся нулевая ссылка. Length и propertyNames[i].

Попробуем повторить проблему? Какое совпадение, что RefreshCache — публичный метод в публичном классе. DirectoryServices — и напишем код следующего вида:
Для этого подключим к проекту нужную библиотеку — System.

DirectoryEntry de = new DirectoryEntry();
de.RefreshCache(null);

Запускаем код на исполнение и видим ожидаемую картину.

Picture 3

Подключаем к проекту NuGet пакет System. Ради интереса можно попробовать воспроизвести проблему на релизной версии NuGet пакета. 5. DirectoryServices (я использовал версию 4. Результат — ниже. 0) и запускаем уже знакомый код на исполнение.

Picture 4

Issue 4

Обратимся к структуре System. Сейчас мы пойдём от обратного — сначала попробуем написать код, который использует экземпляр класса, а затем заглянем внутрь. CharacterRange из библиотеки System. Drawing. Common и одноимённого NuGet пакета. Drawing.

Используемый код будет следующим:

CharacterRange range = new CharacterRange();
bool eq = range.Equals(null);
Console.WriteLine(eq);

На всякий случай, чтобы освежить память, обратимся к docs.microsoft.com, чтобы вспомнить, какое возвращаемое значение ожидается от выражения obj.Equals(null):

In the list, x, y, and z represent object references that are not null. The following statements must be true for all implementations of the Equals(Object) method.

....

Equals(null) returns false. x.

Конечно, нет, это было бы слишком просто. Как вы думаете, будет ли в консоль выведен текст «False»? 🙂 Исполняем код и смотрим на результат.

Picture 5

Drawing. Это был вывод при исполнении указанного выше кода с использованием NuGet пакета System. 5. Common версии 4. Запускаем тот же код с отладочной версией библиотеки и видим следующее: 1.

Picture 6

Теперь посмотрим на исходный код — реализацию метода Equals в структуре CharacterRange и предупреждение анализатора:

public override bool Equals(object obj)
{ if (obj.GetType() != typeof(CharacterRange)) return false; CharacterRange cr = (CharacterRange)obj; return ((_first == cr.First) && (_length == cr.Length));
}

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

Видим то, что и требовалось доказать — неаккуратно обрабатывается параметр obj, из-за чего в условном выражении при вызове экземплярного метода GetType происходит возникновение исключения типа NullReferenceException.

Issue 5

Save. Пока исследуем эту библиотеку, рассмотрим ещё одно интересное место — метод Icon. Перед исследованием посмотрим описание метода.

Описания к методу нет:

Picture 7

Save(Stream) Method". Обратимся к docs.microsoft.com — "Icon. Там, впрочем, тоже никаких ограничений на входные значения и информации о генерируемых исключениях нет.

Теперь переходим к исследованию кода.

public sealed partial class Icon : MarshalByRefObject, ICloneable, IDisposable, ISerializable
{ .... public void Save(Stream outputStream) { if (_iconData != null) { outputStream.Write(_iconData, 0, _iconData.Length); } else { .... if (outputStream == null) throw new ArgumentNullException("dataStream"); .... } } ....
}

Предупреждение PVS-Studio: V3095 The 'outputStream' object was used before it was verified against null. Check lines: 654, 672. Icon.Windows.cs 654

Вновь удачное стечение обстоятельств — и класс, и метод — публичные, значит, можно попробовать воспроизвести проблему. Опять уже известная нам история — возможное разыменование нулевой ссылки, так как параметр метода разыменовывается без проверки на null.

Write(_iconData, 0, _iconData. Задача проста — довести исполнение кода до выражения outputStream. Для этого достаточно, чтобы выполнилось условие _iconData != null. Length);, сохранив при этом значение переменной outputStreamnull.

Посмотрим на самый простой публичный конструктор:

public Icon(string fileName) : this(fileName, 0, 0)
{ }

Он просто делегирует работу другому конструктору. Хорошо, смотрим дальше — используемый здесь конструктор.

public Icon(string fileName, int width, int height) : this()
{ using (FileStream f = new FileStream(fileName, FileMode.Open, FileAccess.Read, FileShare.Read)) { Debug.Assert(f != null, "File.OpenRead returned null instead of throwing an exception"); _iconData = new byte[(int)f.Length]; f.Read(_iconData, 0, _iconData.Length); } Initialize(width, height);
}

Вот оно, то, что нужно. После вызова этого конструктора, если мы успешно считываем данные из файла и, если никаких падений в методе Initialize не происходит, поле _iconData будет содержать ссылку на какой-то объект, что нам и нужно.

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

Icon icon = new Icon(@"D:\document.ico");
icon.Save(null);

Результат исполнения ожидаемый.

Picture 8

Issue 6

Management. Продолжаем обзор и переходим к библиотеке System. UInt32 и остальными case.
Попробуйте найти 3 отличия между действиями, выполняемыми в case CimType.

private static string ConvertToNumericValueAndAddToArray(....)
{ string retFunctionName = string.Empty; enumType = string.Empty; switch(cimType) { case CimType.UInt8: case CimType.SInt8: case CimType.SInt16: case CimType.UInt16: case CimType.SInt32: arrayToAdd.Add(System.Convert.ToInt32( numericValue, (IFormatProvider)CultureInfo.InvariantCulture .GetFormat(typeof(int)))); retFunctionName = "ToInt32"; enumType = "System.Int32"; break; case CimType.UInt32: arrayToAdd.Add(System.Convert.ToInt32( numericValue, (IFormatProvider)CultureInfo.InvariantCulture .GetFormat(typeof(int)))); retFunctionName = "ToInt32"; enumType = "System.Int32"; break; } return retFunctionName;
}

Конечно, никаких отличий нет, о чём и предупреждает анализатор.

WMIGenerator.cs 5220 Предупреждение PVS-Studio: V3139 Two or more case-branches perform the same actions.

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

Issue 7

CSharp.
Библиотека Microsoft.

private static IList<KeyValuePair<string, object>>
QueryDynamicObject(object obj)
{ .... List<string> names = new List<string>(mo.GetDynamicMemberNames()); names.Sort(); if (names != null) { .... } ....
}

Предупреждение PVS-Studio: V3022 Expression 'names != null' is always true. DynamicDebuggerProxy.cs 426

Было много (очень много) странных проверок, но эта чем-то запала мне в душу. Я бы, наверное, мог проигнорировать это предупреждение наравне со многими аналогичными, которые были выданы диагностиками V3022 и V3063. Это не ошибка, конечно, но место интересное, как по мне. Возможно, тем, что перед сравнением локальной переменной names с null в эту переменную мало того, что записывается ссылка на новый созданный объект, так ещё и происходит вызов экземплярного метода Sort.

Issue 8

Вот ещё интересный фрагмент кода.

private static void InsertChildNoGrow(Symbol child)
{ .... while (sym?.nextSameName != null) { sym = sym.nextSameName; } Debug.Assert(sym != null && sym.nextSameName == null); sym.nextSameName = child; ....
}

Предупреждение PVS-Studio: V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'sym' object SymbolStore.cs 56

Цикл завершается при выполнении одного из двух условий: Смотрите, в чём здесь штука.

  • sym == null;
  • sym.nextSameName == null.

Со вторым условием проблем нет, чего нельзя сказать про первое, так как ниже идёт безусловное обращение к экземплярному полю nextSameName и, если symnull, при обращении возникнет исключение типа NullReferenceException.

Есть же вызов Debug. «Ты что, ослеп? Но в этом и вся соль! Assert, где проверяется, что sym != null» — может возразить кто-то. Assert ничем не поможет, и при описанном выше состоянии всё, что мы получим — NullReferenceException. При работе в Release версии Debug. Assert. Более того, я уже видел подобную ошибку в другом проекте от Microsoft — Roslyn, где была ну уж очень похожая ситуация с Debug. Немного отвлекусь на Roslyn с вашего позволения.

CodeAnalysis, либо прямо в Visual Studio при использовании Syntax Visualizer. Проблему можно было воспроизвести либо при использовании библиотек Microsoft. 1. На версии Visual Studio 16. 0 эта проблема ещё воспроизводится. 6 + Syntax Visualizer 1.

Для воспроизведения достаточно такого кода:

class C1<T1, T2>
{ void foo() { T1 val = default; if (val is null) { } }
}

Далее в Syntax Visualizer нужно найти узел синтаксического дерева типа ConstantPatternSyntax, соответствующий null в коде и запросить для него TypeSymbol.

Picture 9

Если зайдём в Event Viewer, найдём информацию о проблемах в библиотеках:
После этого Visual Studio перезагрузится.

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.Resources.MissingManifestResourceException at System.Resources.ManifestBasedResourceGroveler .HandleResourceStreamMissing(System.String) at System.Resources.ManifestBasedResourceGroveler.GrovelForResourceSet( System.Globalization.CultureInfo, System.Collections.Generic.Dictionary'2 <System.String,System.Resources.ResourceSet>, Boolean, Boolean, System.Threading.StackCrawlMark ByRef) at System.Resources.ResourceManager.InternalGetResourceSet( System.Globalization.CultureInfo, Boolean, Boolean, System.Threading.StackCrawlMark ByRef) at System.Resources.ResourceManager.InternalGetResourceSet( System.Globalization.CultureInfo, Boolean, Boolean) at System.Resources.ResourceManager.GetString(System.String, System.Globalization.CultureInfo) at Roslyn.SyntaxVisualizer.DgmlHelper.My. Resources.Resources.get_SyntaxNodeLabel()
....

И о проблеме с devenv.exe:

Faulting application name:
devenv.exe, version: 16.1.29102.190, time stamp: 0x5d1c133b
Faulting module name:
KERNELBASE.dll, version: 10.0.18362.145, time stamp: 0xf5733ace
Exception code: 0xe0434352
Fault offset: 0x001133d2
....

Имея отладочные версии библиотек Roslyn можно найти место, где возникло исключение:

private Conversion ClassifyImplicitBuiltInConversionSlow( TypeSymbol source, TypeSymbol destination, ref HashSet<DiagnosticInfo> useSiteDiagnostics)
{ Debug.Assert((object)source != null); Debug.Assert((object)destination != null); if ( source.SpecialType == SpecialType.System_Void || destination.SpecialType == SpecialType.System_Void) { return Conversion.NoConversion; } ....
}

Здесь, как и в рассматриваемом выше коде из библиотек .NET Core тоже есть проверка через Debug.Assert, которая, однако, никак не помогла при использовании релизных версий библиотек.

Issue 9

NET Core. Отвлеклись немного — и хватит, возвращаемся к библиотекам . IO. Пакет System. IsolatedStorage содержит следующий интересный код.

private bool ContainsUnknownFiles(string directory)
{ .... return (files.Length > 2 || ( (!IsIdFile(files[0]) && !IsInfoFile(files[0]))) || (files.Length == 2 && !IsIdFile(files[1]) && !IsInfoFile(files[1])) );
}

Предупреждение PVS-Studio: V3088 The expression was enclosed by parentheses twice: ((expression)). One pair of parentheses is unnecessary or misprint is present. IsolatedStorageFile.cs 839

Мельком пробегаясь взглядом по этому коду, я бы сказал, что левый операнд первого встреченного оператора || — files. Сказать, что форматирование кода сбивает с толку — не сказать ничего. По крайней мере, код отформатирован так. Length > 2, правый — то, что в скобках. На самом деле правый операнд — ((! Посмотрев чуть внимательнее, можно понять, что это не так. IsInfoFile(files[0]))). IsIdFile(files[0]) && ! По-моему, этот код порядочно сбивает с толку.

Issue 10

03 было добавлено диагностическое правило V3138, которое ищет ошибки в интерполированных строках. В релизе PVS-Studio 7. В библиотеках System. Точнее, в строках, которые, скорее всего, должны быть интерполированными, но из-за пропущенного символа $ таковыми не являются. Net нашлось несколько интересных срабатываний этого диагностического правила.

internal static void CacheCredential(SafeFreeCredentials newHandle)
{ try { .... } catch (Exception e) { if (!ExceptionCheck.IsFatal(e)) { NetEventSource.Fail(null, "Attempted to throw: {e}"); } }
}

Предупреждение PVS-Studio: V3138 String literal contains potential interpolated expression. Consider inspecting: e. SSPIHandleCache.cs 42

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

Issue 11

Встретился ещё один похожий случай.

public static async Task<string> GetDigestTokenForCredential(....)
{ .... if (NetEventSource.IsEnabled) NetEventSource.Error(digestResponse, "Algorithm not supported: {algorithm}"); ....
}

Предупреждение PVS-Studio: V3138 String literal contains potential interpolated expression. Consider inspecting: algorithm. AuthenticationHelper.Digest.cs 58

Ситуация аналогична описанной выше, опять пропущен символ $ — в метод Error идёт неверная строка.

Issue 12

Net. Пакет System. Метод небольшой, приведу его целиком, чтобы ошибку было искать чуть интереснее.
Mail
.

internal void SetContent(Stream stream)
{ if (stream == null) { throw new ArgumentNullException(nameof(stream)); } if (_streamSet) { _stream.Close(); _stream = null; _streamSet = false; } _stream = stream; _streamSet = true; _streamUsedOnce = false; TransferEncoding = TransferEncoding.Base64;
}

Предупреждение PVS-Studio: V3008 The '_streamSet' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 123, 119. MimePart.cs 123

Та же история и с обнулением переменной _stream. Странно выглядит двойное присвоение значения переменной _streamSet (сначала — под условием; потом — вне). В итоге _stream всё равно будет иметь значение stream, а _streamSettrue.

Issue 13

Linq. Интересное место из библиотеки System. В данном случае это скорее «фича», чем баг, но тем не менее, метод весьма интересный…
Expressions
, на которое анализатор выдал сразу 2 предупреждения.

// throws NRE when o is null
protected static void NullCheck(object o)
{ if (o == null) { o.GetType(); }
}

Предупреждения PVS-Studio:

  • V3010 The return value of function 'GetType' is required to be utilized. Instruction.cs 36
  • V3080 Possible null dereference. Consider inspecting 'o'. Instruction.cs 36

Тут, наверное, даже и комментировать нечего.

Picture 20

Issue 14

Сначала напишем код, выявим проблемы, а затем заглянем внутрь. Давайте рассмотрим ещё один случай, с которым мы будем работать «извне». Configuration. Для изучения возьмём библиотеку System. Я использовал пакет версии 4. ConfigurationManager и одноимённый NuGet пакет. 0. 5. Configuration. Будем работать с классом System. CommaDelimitedStringCollection.

Например, создадим объект, извлечём его строковое представление, получим длину этой строки и распечатаем её. Сделаем что-нибудь не очень хитрое. Соответствующий код:

CommaDelimitedStringCollection collection = new CommaDelimitedStringCollection();
Console.WriteLine(collection.ToString().Length);

На всякий случай посмотрим на описание метода ToString:

Picture 11

На всякий случай также загляну на docs.microsoft.com — "CommaDelimitedStringCollection. Ничего необычного — просто возвращается строковое представление объекта. Вроде бы тоже ничего особенного. ToString Method".

Хорошо, запускаем код на исполнение, иии…

Picture 12

Что ж, давайте попробуем добавить элемент в коллекцию, и затем получить её строковое представление. Хм, неожиданно. Код изменится и будет выглядеть так:
«Совершенно случайно» добавлять будем пустую строку :).

CommaDelimitedStringCollection collection = new CommaDelimitedStringCollection();
collection.Add(String.Empty);
Console.WriteLine(collection.ToString().Length);

Запускаем на исполнение и видим…

Picture 13

Что ж, давайте уже наконец взглянем на реализацию метода ToString класса CommaDelimitedStringCollection. Что, опять?! Код представлен ниже:

public override string ToString()
{ if (Count <= 0) return null; StringBuilder sb = new StringBuilder(); foreach (string str in this) { ThrowIfContainsDelimiter(str); // .... sb.Append(str.Trim()); sb.Append(','); } if (sb.Length > 0) sb.Length = sb.Length - 1; return sb.Length == 0 ? null : sb.ToString();
}

Предупреждения PVS-Studio:

  • V3108 It is not recommended to return 'null' from 'ToSting()' method. StringAttributeCollection.cs 57
  • V3108 It is not recommended to return 'null' from 'ToSting()' method. StringAttributeCollection.cs 71

Здесь мы видим 2 места, где текущая реализация ToString может вернуть значение null. Вспомним, что советует делать Microsoft при реализации метода ToString, для чего вновь обратимся к docs.microsoft.com — "Object.ToString Method":

Overrides of the ToString() method should follow these guidelines: Notes to Inheritors....

  • ....
  • Your ToString() override should not return Empty or a null string.
  • ....

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

Count — свойство базового класса StringCollection. Первый случай. Так как никаких элементов добавлено не было, Count == 0, выполняется условие Count <= 0, возвращается значение null.

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

public new void Add(string value)
{ ThrowIfReadOnly(); ThrowIfContainsDelimiter(value); _modified = true; base.Add(value.Trim());
}

Проверки в методе ThrowIf... успешно проходят и элемент добавляется в базовую коллекцию. Соответственно, значение Count становится равным 1. Теперь возвращаемся к методу ToString. Значение выражения Count <= 0false, следовательно, выхода из метода не происходит и код продолжает своё исполнение. Начинается обход внутренней коллекции, и в StringBuilder добавляются 2 элемента — пустая строка и запятая. В итоге получается, что в sb содержится только запятая, значение свойства Length, соответственно равно единице. Значение выражения sb.Length > 0true, выполняется вычитание и запись в sb.Length, теперь значение sb.Length — 0. Это ведёт к тому, что из метода опять возвращается значение null.

Issue 15

Configuration. Совершенно неожиданно мне захотелось поиспользовать класс System. Возьмём конструктор с наибольшим количеством параметров:
ConfigurationProperty
.

public ConfigurationProperty( string name, Type type, object defaultValue, TypeConverter typeConverter, ConfigurationValidatorBase validator, ConfigurationPropertyOptions options, string description);

Посмотрим описание последнего параметра:

// description:
// The description of the configuration entity.

В описании конструктора на docs.microsoft.com написано то же самое. Что ж, давайте взглянем, как этот параметр используется в теле конструктора:

public ConfigurationProperty(...., string description)
{ ConstructorInit(name, type, options, validator, typeConverter); SetDefaultValue(defaultValue);
}

А параметр-то и не используется.

ConfigurationProperty.cs 62 Предупреждение PVS-Studio: V3117 Constructor parameter 'description' is not used.

Вероятно, не используют его специально, но описание соответствующего параметра сбивает с толку.

Issue 16

Попробуйте найти ошибку самостоятельно, код конструктора привожу ниже.
Встретилось ещё одно похожее место.

internal SectionXmlInfo( string configKey, string definitionConfigPath, string targetConfigPath, string subPath, string filename, int lineNumber, object streamVersion, string rawXml, string configSource, string configSourceStreamName, object configSourceStreamVersion, string protectionProviderName, OverrideModeSetting overrideMode, bool skipInChildApps)
{ ConfigKey = configKey; DefinitionConfigPath = definitionConfigPath; TargetConfigPath = targetConfigPath; SubPath = subPath; Filename = filename; LineNumber = lineNumber; StreamVersion = streamVersion; RawXml = rawXml; ConfigSource = configSource; ConfigSourceStreamName = configSourceStreamName; ProtectionProviderName = protectionProviderName; OverrideModeSetting = overrideMode; SkipInChildApps = skipInChildApps;
}

Предупреждение PVS-Studio: V3117 Constructor parameter 'configSourceStreamVersion' is not used. SectionXmlInfo.cs 16

Есть соответствующее свойство, правда выглядит оно немного странно:

internal object ConfigSourceStreamVersion
{ set { }
}

В общем, код выглядит подозрительно. Возможно, параметр / свойство оставили для совместимости, но это лишь мои догадки.

Issue 17

Runtime. Посмотрим, что интересного нашлось в коде библиотеки System. UI. WindowsRuntime. Xaml и одноимённого NuGet пакета.

public struct RepeatBehavior : IFormattable
{ .... public override string ToString() { return InternalToString(null, null); } ....
}

Предупреждение PVS-Studio: V3108 It is not recommended to return 'null' from 'ToSting()' method. RepeatBehavior.cs 113

Из-за этого автор вызывающего кода, предполагающий, что RepeatBehavior. Знакомая история, которую уже проходили — метод ToString может вернуть значение null. Да и опять же, это отклонение от гайдлайнов Microsoft. ToString всегда возвращает ненулевую ссылку, в какой-то момент может быть неприятно удивлён.

Конечно, только из этого метода непонятно, что ToString может вернуть null — нужно копнуть глубже и заглянуть в метод InternalToString.

internal string InternalToString(string format, IFormatProvider formatProvider)
{ switch (_Type) { case RepeatBehaviorType.Forever: return "Forever"; case RepeatBehaviorType.Count: StringBuilder sb = new StringBuilder(); sb.AppendFormat( formatProvider, "{0:" + format + "}x", _Count); return sb.ToString(); case RepeatBehaviorType.Duration: return _Duration.ToString(); default: return null; }
}

Анализатор обнаружил, что в случае, если в switch выполнится default ветвь, InternalToString вернёт значение null, следовательно, null вернёт и ToString.

Для этого нужно создать экземпляр RepeatBehavior, вызвать у него метод ToString, но при этом не забывать, что _Type не должен быть равным RepeatBehaviorType. RepeatBehavior — публичная структура, а ToString — публичный метод, так что можно попробовать воспроизвести проблему на практике. Count или RepeatBehaviorType. Forever, RepeatBehaviorType. Duration.

_Type — приватное поле, которое можно назначить через публичное свойство:

public struct RepeatBehavior : IFormattable
{ .... private RepeatBehaviorType _Type; .... public RepeatBehaviorType Type { get { return _Type; } set { _Type = value; } } ....
}

Пока всё выглядит неплохо. Идём дальше, и смотрим, что из себя представляет тип RepeatBehaviorType.

public enum RepeatBehaviorType
{ Count, Duration, Forever
}

Как видно, RepeatBehaviorType — перечисление, содержащее все три элемента. Причём все эти три элемента покрываются в необходимом нам выражении switch. Это, однако, не значит, что default ветвь недостижима.

Runtime. Для воспроизведения проблемы подключим в проект пакет System. UI. WindowsRuntime. 3. Xaml (я использовал версию 4. 0) и выполним следующий код.

RepeatBehavior behavior = new RepeatBehavior()
{ Type = (RepeatBehaviorType)666
};
Console.WriteLine(behavior.ToString() is null);

В консоль вполне ожидаемо выводится True, а это означает ToString вернул null, т.к. _Type не был равен ни одному из значений в case ветвях и управление перешло в ветвь default. Чего мы, собственно, и добивались.

Хочу также отметить, что ни в комментариях к методу, ни на docs.microsoft.com, не указано, что метод может возвращать значение null.

Issue 18

Private. Далее разберём несколько предупреждений из System. DataContractSerialization.

private static class CharType
{ public const byte None = 0x00; public const byte FirstName = 0x01; public const byte Name = 0x02; public const byte Whitespace = 0x04; public const byte Text = 0x08; public const byte AttributeText = 0x10; public const byte SpecialWhitespace = 0x20; public const byte Comment = 0x40;
}
private static byte[] s_charType = new byte[256]
{ .... CharType.None, /* 9 (.) */ CharType.None| CharType.Comment| CharType.Comment| CharType.Whitespace| CharType.Text| CharType.SpecialWhitespace, /* A (.) */ CharType.None| CharType.Comment| CharType.Comment| CharType.Whitespace| CharType.Text| CharType.SpecialWhitespace, /* B (.) */ CharType.None, /* C (.) */ CharType.None, /* D (.) */ CharType.None| CharType.Comment| CharType.Comment| CharType.Whitespace, /* E (.) */ CharType.None, ....
};

Предупреждения PVS-Studio:

  • V3001 There are identical sub-expressions 'CharType.Comment' to the left and to the right of the '|' operator. XmlUTF8TextReader.cs 56
  • V3001 There are identical sub-expressions 'CharType.Comment' to the left and to the right of the '|' operator. XmlUTF8TextReader.cs 58
  • V3001 There are identical sub-expressions 'CharType.Comment' to the left and to the right of the '|' operator. XmlUTF8TextReader.cs 64

Анализатор счёл подозрительным использование выражения CharType.Comment| CharType.Comment. Выглядит немного странно, так как (CharType.Comment | CharType.Comment) == CharType.Comment. При инициализации других элементов массива, в которых используется CharType.Comment, подобного дублирования нет.

Issue 19

Посмотрим информацию о возвращаемом значении метода XmlBinaryWriterSession. Продолжаем. TryAdd(XmlDictionaryString, Int32) Method": Returns: true if the string could be added; otherwise, false. TryAdd в описании метода и на docs.microsoft.com — "XmlBinaryWriterSession.

Теперь заглянем в тело метода:

public virtual bool TryAdd(XmlDictionaryString value, out int key)
{ IntArray keys; if (value == null) throw System.Runtime .Serialization .DiagnosticUtility .ExceptionUtility .ThrowHelperArgumentNull(nameof(value)); if (_maps.TryGetValue(value.Dictionary, out keys)) { key = (keys[value.Key] - 1); if (key != -1) { // If the key is already set, then something is wrong throw System.Runtime .Serialization .DiagnosticUtility .ExceptionUtility .ThrowHelperError( new InvalidOperationException( SR.XmlKeyAlreadyExists)); } key = Add(value.Value); keys[value.Key] = (key + 1); return true; } key = Add(value.Value); keys = AddKeys(value.Dictionary, value.Key + 1); keys[value.Key] = (key + 1); return true;
}

Предупреждение PVS-Studio: V3009 It's odd that this method always returns one and the same value of 'true'. XmlBinaryWriterSession.cs 29

Выглядит странным, что метод либо возвращает true, либо кидает исключение, но значение false не возвращает никогда.

Issue 20

Встретился код с подобной проблемой, только теперь наоборот — всегда возвращается значение false:

internal virtual bool OnHandleReference(....)
{ if (xmlWriter.depth < depthToCheckCyclicReference) return false; if (canContainCyclicReference) { if (_byValObjectsInScope.Contains(obj)) throw ....; _byValObjectsInScope.Push(obj); } return false;
}

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

Поэтому, прежде чем продолжить, предлагаю сделать небольшой перерыв — размять мышцы, немного походить, дать отдохнуть глазам, посмотреть в окно… Итак, мы с вами уже проделали значительный путь!

Picture 21

🙂 Надеюсь, на этой точке вы вновь полны сил, так что продолжим.

Issue 21

Security. Посмотрим на интересные места проекта System. Algorithms.
Cryptography.

public override byte[] GenerateMask(byte[] rgbSeed, int cbReturn)
{ using (HashAlgorithm hasher = (HashAlgorithm)CryptoConfig.CreateFromName(_hashNameValue)) { byte[] rgbCounter = new byte[4]; byte[] rgbT = new byte[cbReturn]; uint counter = 0; for (int ib = 0; ib < rgbT.Length;) { // Increment counter -- up to 2^32 * sizeof(Hash) Helpers.ConvertIntToByteArray(counter++, rgbCounter); hasher.TransformBlock(rgbSeed, 0, rgbSeed.Length, rgbSeed, 0); hasher.TransformFinalBlock(rgbCounter, 0, 4); byte[] hash = hasher.Hash; hasher.Initialize(); Buffer.BlockCopy(hash, 0, rgbT, ib, Math.Min(rgbT.Length - ib, hash.Length)); ib += hasher.Hash.Length; } return rgbT; }
}

Предупреждение PVS-Studio: V3080 Possible null dereference. Consider inspecting 'hasher'. PKCS1MaskGenerationMethod.cs 37

TransformBlock значение переменной hasher может быть null, в случае чего произойдёт генерация исключения типа NullReferenceException. Анализатор предупреждает о том, что при вычислении выражения hasher. Появление этого предупреждения стало возможным благодаря межпроцедурному анализу.

Итак, чтобы понять, может ли hasher в данном случае принимать значение null, необходимо опуститься в метод CreateFromName.

public static object CreateFromName(string name)
{ return CreateFromName(name, null);
}

Пока ничего — опускаемся глубже. Тело перегруженной версии CreateFromName с двумя параметрами достаточно большое, поэтому привожу сокращённую версию.

public static object CreateFromName(string name, params object[] args)
{ .... if (retvalType == null) { return null; } .... if (cons == null) { return null; } .... if (candidates.Count == 0) { return null; } .... if (rci == null || typeof(Delegate).IsAssignableFrom(rci.DeclaringType)) { return null; } .... return retval;
}

Как видно, в методе есть несколько точек выхода, где явно возвращается значение null. Следовательно, хотя бы теоретически, в упоминаемом ранее методе, на который было выдано предупреждение, возможно возникновение исключения типа NullReferenceException.

Для этого ещё раз взглянем на исходный метод и отметим ключевые точки. Теория — это хорошо, но давайте попробуем воспроизвести проблему на практике. Неактуальный код из метода сократим.

public class PKCS1MaskGenerationMethod : .... // <= 1
{ .... public PKCS1MaskGenerationMethod() // <= 2 { _hashNameValue = DefaultHash; } .... public override byte[] GenerateMask(byte[] rgbSeed, int cbReturn) // <= 3 { using (HashAlgorithm hasher = (HashAlgorithm)CryptoConfig.CreateFromName(_hashNameValue)) // <= 4 { byte[] rgbCounter = new byte[4]; byte[] rgbT = new byte[cbReturn]; // <= 5 uint counter = 0; for (int ib = 0; ib < rgbT.Length;) // <= 6 { .... Helpers.ConvertIntToByteArray(counter++, rgbCounter); // <= 7 hasher.TransformBlock(rgbSeed, 0, rgbSeed.Length, rgbSeed, 0); .... } .... } }
}

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

Класс и метод имеют модификаторы доступа public. 1, 3. Следовательно, этот интерфейс доступен при подключении библиотеки — можно попробовать повторить проблему.

Класс — экземплярный неабстрактный, имеет публичный конструктор — должно быть легко создать экземпляр, с которым и будем работать. 2. В некоторых рассмотренных мной случаях классы были абстрактными, так что для повторения проблемы ещё приходилось искать наследников и способы их получения.

CreateFromName не должен сгенерировать никаких исключений и должен вернуть null — наиболее важная точка, к ней вернёмся позже. 4.

Значение cbReturn должно быть > 0 (и, конечно, в адекватных пределах для успешного создания массива). 5, 6. Length и захода в тело цикла. Выполнение условия cbReturn > 0 необходимо для выполнения дальнейшего условия ib < rgbT.

Helpres. 7. ConvertIntToByteArray должен отработать без исключений.

Для выполнения условий, зависящих от параметров метода, достаточно просто передать адекватные аргументы, например:

  • rgbCeed — new byte[] { 0, 1, 2, 3 };
  • cbReturn — 42.

Для того, чтобы «скомпрометировать» метод CryptoConfig.CreateFromName, необходима возможность изменять значение поля _hashNameValue. К счастью для нас, она есть, так как в классе определено свойство-обёртка над этим полем:

public string HashName
{ get { return _hashNameValue; } set { _hashNameValue = value ?? DefaultHash; }
}

Установив 'синтетическое' значение для HashName (точнее — _hashNameValue), можно получить значение null из метода CreateFromName на первой из отмеченных нами точек возврата. Вдаваться в подробности разбора этого метода не буду (надеюсь, вы мне простите это), так как он достаточно длинный.

В итоге код, который приведёт к возникновению исключения типа NullReferenceException, может выглядеть следующим образом:

PKCS1MaskGenerationMethod tempObj = new PKCS1MaskGenerationMethod();
tempObj.HashName = "Dummy";
tempObj.GenerateMask(new byte[] { 1, 2, 3 }, 42);

Подключаем к проекту отладочную библиотеки, запускаем и получаем ожидаемый результат:

Picture 10

3. Ради интереса попробовал исполнить этот же код на NuGet пакете версии 4. 1.

Picture 14

GenerateMask(Byte[], Int32) Method". Информации о генерируемых исключениях, ограничений на выходные параметры в описании метода, на docs.microsoft.com это тоже не описано — "PKCS1MaskGenerationMethod.

Кстати, уже в ходе написания статьи и описания последовательности воспроизведения проблемы нашёл ещё 2 способа «сломать» этот метод:

  • передать в качестве аргумента cbReturn слишком большое значение;
  • передать в качестве rgbSeed значение null.

В первом случае получаем исключение типа OutOfMemoryException.

Picture 15

Length. Во втором случае получаем исключение типа NullReferenceException при исполнении выражения rgbSeed. Length. В этом случае важно, чтобы hasher имел ненулевое значение, иначе поток исполнения не дойдёт до rgbSeed.

Issue 22

Встретилась ещё пара похожих мест.

public class SignatureDescription
{ .... public string FormatterAlgorithm { get; set; } public string DeformatterAlgorithm { get; set; } public SignatureDescription() { } .... public virtual AsymmetricSignatureDeformatter CreateDeformatter( AsymmetricAlgorithm key) { AsymmetricSignatureDeformatter item = (AsymmetricSignatureDeformatter) CryptoConfig.CreateFromName(DeformatterAlgorithm); item.SetKey(key); // <= return item; } public virtual AsymmetricSignatureFormatter CreateFormatter( AsymmetricAlgorithm key) { AsymmetricSignatureFormatter item = (AsymmetricSignatureFormatter) CryptoConfig.CreateFromName(FormatterAlgorithm); item.SetKey(key); // <= return item; } ....
}

Предупреждения PVS-Studio:

  • V3080 Possible null dereference. Consider inspecting 'item'. SignatureDescription.cs 31
  • V3080 Possible null dereference. Consider inspecting 'item'. SignatureDescription.cs 38

Опять же, мы можем записать в свойства FormatterAlgorithm и DeformatterAlgorithm такие значения, для которых метод CryptoConfig.CreateFromName вернёт значение null в методах CreateDeformatter и CreateFormatter. Далее, при вызове экземплярного метода SetKey будет сгенерировано исключение NullReferenceException. Проблема, опять же, легко воспроизводится на практике:

SignatureDescription signature = new SignatureDescription()
{ DeformatterAlgorithm = "Dummy", FormatterAlgorithm = "Dummy"
}; signature.CreateDeformatter(null); // NRE
signature.CreateFormatter(null); // NRE

В данном случае и при вызове CreateDeformatter, и при вызове CreateFormatter генерируется исключение типа NullReferenceException.

Issue 23

Private. Посмотрим на интересные места из проекта System. Xml.

public override void WriteBase64(byte[] buffer, int index, int count)
{ if (!_inAttr && (_inCDataSection || StartCDataSection())) _wrapped.WriteBase64(buffer, index, count); else _wrapped.WriteBase64(buffer, index, count);
}

Предупреждение PVS-Studio: V3004 The 'then' statement is equivalent to the 'else' statement. QueryOutputWriterV1.cs 242

Либо здесь ошибка, и в одной из ветвей предполагалось иное действие, либо оператор if можно опустить. Выглядит странно, что then и else ветви оператора if содержат один и тот же код.

Issue 24

internal void Depends(XmlSchemaObject item, ArrayList refs)
{ .... if (content is XmlSchemaSimpleTypeRestriction) { baseType = ((XmlSchemaSimpleTypeRestriction)content).BaseType; baseName = ((XmlSchemaSimpleTypeRestriction)content).BaseTypeName; } else if (content is XmlSchemaSimpleTypeList) { .... } else if (content is XmlSchemaSimpleTypeRestriction) { baseName = ((XmlSchemaSimpleTypeRestriction)content).BaseTypeName; } else if (t == typeof(XmlSchemaSimpleTypeUnion)) { .... } ....
}

Предупреждение PVS-Studio: V3003 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 381, 396. ImportContext.cs 381

Что ещё интереснее — тела then-ветвей соответствующих операторов содержат разный набор выражений. В последовательности if-else-if есть два одинаковых условных выражения — content is XmlSchemaSimpleTypeRestriction. Так или иначе, будет выполнено либо тело первой соответствующей then-ветви (если условное выражение истинно), либо ни то, ни другое, если соответствующее выражение ложно.

Issue 25

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

public bool MatchesXmlType(IList<XPathItem> seq, int indexType)
{ XmlQueryType typBase = GetXmlType(indexType); XmlQueryCardinality card; switch (seq.Count) { case 0: card = XmlQueryCardinality.Zero; break; case 1: card = XmlQueryCardinality.One; break; default: card = XmlQueryCardinality.More; break; } if (!(card <= typBase.Cardinality)) return false; typBase = typBase.Prime; for (int i = 0; i < seq.Count; i++) { if (!CreateXmlType(seq[0]).IsSubtypeOf(typBase)) return false; } return true;
}

Если справились — примите поздравления!
Если нет — предупреждение PVS-Studio поможет: V3102 Suspicious access to element of 'seq' object by a constant index inside a loop. XmlQueryRuntime.cs 738

Count. Выполняется цикл for, в качестве условия выхода которого используется выражение i < seq. Вот только в цикле выполняют доступ к элементам последовательности не с использованием счётчика (seq[i]), а с использованием числового литерала — нуля (seq[0]). Наводит на мысли о том, что хотят обойти последовательность seq.

Issue 26

Следующая ошибка умещается в маленьком фрагменте кода, но от того она не менее интересна.

public override void WriteValue(string value)
{ WriteValue(value);
}

Предупреждение PVS-Studio: V3110 Possible infinite recursion inside 'WriteValue' method. XmlAttributeCache.cs 166

Метод вызывает сам себя, образуя таким образом рекурсию без условия выхода из неё.

Issue 27

public IList<XPathNavigator> DocOrderDistinct(IList<XPathNavigator> seq)
{ if (seq.Count <= 1) return seq; XmlQueryNodeSequence nodeSeq = (XmlQueryNodeSequence)seq; if (nodeSeq == null) nodeSeq = new XmlQueryNodeSequence(seq); return nodeSeq.DocOrderDistinct(_docOrderCmp);
}

Предупреждение PVS-Studio: V3095 The 'seq' object was used before it was verified against null. Check lines: 880, 884. XmlQueryRuntime.cs 880

Ниже выполняется проверка переменной nodeSeq, полученной в результате явного приведения seq, но зачем она там — не очень понятно. В качестве аргумента методу может прийти значение null, из-за чего при обращении к свойству Count будет сгенерировано исключение типа NullReferenceException. Если значение seq — не null, то: Если значение seqnull, поток исполнения не дойдёт до этой проверки из-за исключения.

  • будет сгенерировано исключение типа InvalidCastException, если не удастся выполнить приведение;
  • если приведение пройдёт успешно, nodeSeq точно имеет значение, отличное от null.

Issue 28

Возможно, они оставлены для совместимости, но никаких комментариев касаемо этих неиспользуемых параметров я не нашёл. Встретилось 4 конструктора, в которых были неиспользуемые параметры.

Предупреждения PVS-Studio:

  • V3117 Constructor parameter 'securityUrl' is not used. XmlSecureResolver.cs 15
  • V3117 Constructor parameter 'strdata' is not used. XmlEntity.cs 18
  • V3117 Constructor parameter 'location' is not used. Compilation.cs 58
  • V3117 Constructor parameter 'access' is not used. XmlSerializationILGen.cs 38

Наиболее меня заинтересовал первый (по крайней мере, именно его я выписал в список предупреждений для статьи). Чем? Не уверен. Возможно, названием.

public XmlSecureResolver(XmlResolver resolver, string securityUrl)
{ _resolver = resolver;
}

Интереса ради зашёл посмотреть, что пишут на docs.microsoft.com — "XmlSecureResolver Constructors" про параметр securityUrl:

The XmlSecureResolver calls PermitOnly() on the created PermissionSet before calling GetEntity(Uri, String, Type) on the underlying XmlResolver. The URL used to create the PermissionSet that will be applied to the underlying XmlResolver.

Issue 29

Private. В пакете System. Вспомним один из советов со страницы "Object. Uri я нашёл метод, который вступал в небольшое противоречие с гайдлайнами Microsoft по переопределению метода ToString. ToString Method": Your ToString() override should not throw an exception.

Сам переопределённый метод выглядит следующим образом:

public override string ToString()
{ if (_username.Length == 0 && _password.Length > 0) { throw new UriFormatException(SR.net_uri_BadUserPassword); } ....
}

Предупреждение PVS-Studio: V3108 It is not recommended to throw exceptions from 'ToSting()' method. UriBuilder.cs 406

Пример такого кода:
Код, задавший через публичные свойства UserName и Password пустую строку для поля _username и непустую для поля _password соответственно, а затем вызвавший метод ToString, получит исключение.

UriBuilder uriBuilder = new UriBuilder()
{ UserName = String.Empty, Password = "Dummy"
}; String stringRepresentation = uriBuilder.ToString();
Console.WriteLine(stringRepresentation);

Но в данном случае разработчики честно предупреждают о том, что при вызове может быть сгенерировано исключение — это описано как в комментариях к методу, так и на docs.microsoft.com — "UriBuilder.ToString Method".

Issue 30

Data. Взглянем на предупреждения, выданные на код проекта System. Common.

private ArrayList _tables;
private DataTable GetTable(string tableName, string ns)
{ .... if (_tables.Count == 0) return (DataTable)_tables[0]; ....
}

Предупреждение PVS-Studio: V3106 Possibly index is out of bound. The '0' index is pointing beyond '_tables' bound. XMLDiffLoader.cs 277

Как вы думаете, что это? Этот фрагмент кода выглядит необычно? Такому подходу я бы, пожалуй, не удивился. Необычный способ сгенерировать исключение типа ArgumentOutOfRangeException? В общем, код странный и подозрительный.

Issue 31

internal XmlNodeOrder ComparePosition(XPathNodePointer other)
{ RealFoliate(); other.RealFoliate(); Debug.Assert(other != null); ....
}

Предупреждение PVS-Studio: V3095 The 'other' object was used before it was verified against null. Check lines: 1095, 1096. XPathNodePointer.cs 1095

Assert недвусмысленно намекает, что в качестве аргумента методу ComparePosition может приходить значение null. Выражение other != null как аргумент метода Debug. Но в то же время строкой выше у other вызывается экземплярный метод RealFoliate. По крайней мере, такие случаи хотят отлавливать. В итоге, если other имеет значение null, будет сгенерировано исключение типа NullReferenceException до проверки через Assert.

Issue 32

private PropertyDescriptorCollection GetProperties(Attribute[] attributes)
{ .... foreach (Attribute attribute in attributes) { Attribute attr = property.Attributes[attribute.GetType()]; if ( (attr == null && !attribute.IsDefaultAttribute()) || !attr.Match(attribute)) { match = false; break; } } ....
}

Предупреждение PVS-Studio: V3080 Possible null dereference. Consider inspecting 'attr'. DbConnectionStringBuilder.cs 534

Match — экземплярный метод. Условное выражение оператора if выглядит несколько подозрительно. Следовательно, если поток исполнения дойдёт до правого операнда оператора || при условии, что attrnull, получим исключение типа NullReferenceException. Судя по проверке attr == null, null — допустимое (ожидаемое) значение для этой переменной.

Соответственно, условия возникновения исключения следующие:

  1. Значение attrnull. Вычисляется правый операнд оператора &&.
  2. Значение !attribute.IsDefaultAttribute()false. Общий результат выражения с оператором && — false.
  3. Так как левый операнд оператора || имеет значение false, вычисляется правый операнд.
  4. Так как attrnull, при вызове метода Match генерируется исключение.

Issue 33

private int ReadOldRowData( DataSet ds, ref DataTable table, ref int pos, XmlReader row)
{ .... if (table == null) { row.Skip(); // need to skip this element if we dont know about it, // before returning -1 return -1; } .... if (table == null) throw ExceptionBuilder.DiffgramMissingTable( XmlConvert.DecodeName(row.LocalName)); ....
}

Предупреждение PVS-Studio: V3021 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 XMLDiffLoader.cs 301

При этом then-ветви этих операторов содержат разные действия — в одном случае осуществляется выход из метода со значением -1, во втором — генерируется исключение. Есть два оператора if, содержащих одинаковое условное выражение — table == null. Таким образом, рассматриваемое исключение сгенерировано не будет. Между проверками переменная table не изменяется.

Issue 34

ComponentModel. Взглянем на интересный метод из проекта System. Точнее, сначала взглянем на описывающий его комментарий: TypeConverter.

(Remove last character in virtual string). Removes the last character from the formatted string. This position is relative to the test string. On exit the out param contains the position where the operation was actually performed. Returns true on success, false otherwise. The MaskedTextResultHint out param gives more information about the operation result.

Посмотрим, что происходит по факту.
Ключевой момент по возвращаемому значению: если операция проходит успешно, метод возвращает true, иначе — false.

public bool Remove(out int testPosition, out MaskedTextResultHint resultHint)
{ .... if (lastAssignedPos == INVALID_INDEX) { .... return true; // nothing to remove. } .... return true;
}

Предупреждение PVS-Studio: V3009 It's odd that this method always returns one and the same value of 'true'. MaskedTextProvider.cs 1529

По факту же получается, что единственное возвращаемое значение метода — true.

Issue 35

public void Clear()
{ if (_table != null) { .... } if (_table.fInitInProgress && _delayLoadingConstraints != null) { .... } ....
}

Предупреждение PVS-Studio: V3125 The '_table' object was used after it was verified against null. Check lines: 437, 423. ConstraintCollection.cs 437

По крайней мере, на этот случай перестраховываются. Проверка _table != null говорит сама за себя — переменная _table может иметь значение null. Однако ниже обращаются к экземплярному полю через _table уже без проверки на null_table .fInitInProgress.

Issue 36

Runtime. Теперь рассмотрим несколько предупреждений, выданных на код проекта System. Formatters.
Serialization.

private void Write(....)
{ .... if (memberNameInfo != null) { .... _serWriter.WriteObjectEnd(memberNameInfo, typeNameInfo); } else if ((objectInfo._objectId == _topId) && (_topName != null)) { _serWriter.WriteObjectEnd(topNameInfo, typeNameInfo); .... } else if (!ReferenceEquals(objectInfo._objectType, Converter.s_typeofString)) { _serWriter.WriteObjectEnd(typeNameInfo, typeNameInfo); }
}

Предупреждение PVS-Studio: V3038 The argument was passed to method several times. It is possible that other argument should be passed instead. BinaryObjectWriter.cs 262

WriteObjectEnd с двумя одинаковыми аргументами — typeNameInfo. Анализатор смутил последний вызов _serWriter. Решил посмотреть, что из себя представляет вызываемый метод WriteObjectEnd.
Вроде бы похоже на опечатку, но наверняка сказать нельзя.

internal void WriteObjectEnd(NameInfo memberNameInfo, NameInfo typeNameInfo) { }

Что ж… Идём дальше. 🙂

Issue 37

internal void WriteSerializationHeader( int topId, int headerId, int minorVersion, int majorVersion)
{ var record = new SerializationHeaderRecord( BinaryHeaderEnum.SerializedStreamHeader, topId, headerId, minorVersion, majorVersion); record.Write(this);
}

Глядя на этот код, сходу и не скажешь, что здесь не так или выглядит подозрительно. А вот анализатор вполне может сказать, что его насторожило.

BinaryFormatterWriter.cs 111 Предупреждение PVS-Studio: V3066 Possible incorrect order of arguments passed to 'SerializationHeaderRecord' constructor: 'minorVersion' and 'majorVersion'.

Посмотрим на вызываемый конструктор класса SerializationHeaderRecord.

internal SerializationHeaderRecord( BinaryHeaderEnum binaryHeaderEnum, int topId, int headerId, int majorVersion, int minorVersion)
{ _binaryHeaderEnum = binaryHeaderEnum; _topId = topId; _headerId = headerId; _majorVersion = majorVersion; _minorVersion = minorVersion;
}

Как видно, параметры конструктора следуют в порядке majorVersion, minorVersion; при вызове конструктора передаются же они в порядке minorVersion, majorVersion. Похоже на опечатку. Если же так и было задумано (а вдруг?) — думаю, следовало бы оставить поясняющий комментарий.

Issue 38

internal ObjectManager( ISurrogateSelector selector, StreamingContext context, bool checkSecurity, bool isCrossAppDomain)
{ _objects = new ObjectHolder[DefaultInitialSize]; _selector = selector; _context = context; _isCrossAppDomain = isCrossAppDomain;
}

Предупреждение PVS-Studio: V3117 Constructor parameter 'checkSecurity' is not used. ObjectManager.cs 33

Комментариев к нему нет. Параметр конструктора checkSecurity никак не используется. Предположу, что он оставлен для совместимости, но так или иначе, в контексте разговоров про безопасность в последние годы смотрится это интересно.

Issue 39

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

  • либо я недостаточно просветлён и не понял смысла такого дублирования;
  • либо ошибка распространялась по коду методом copy-paste.

Сам код:

private void EnlargeArray()
{ int newLength = _values.Length * 2; if (newLength < 0) { if (newLength == int.MaxValue) { throw new SerializationException(SR.Serialization_TooManyElements); } newLength = int.MaxValue; } FixupHolder[] temp = new FixupHolder[newLength]; Array.Copy(_values, 0, temp, 0, _count); _values = temp;
}

Предупреждения PVS-Studio:

  • V3022 Expression 'newLength == int.MaxValue' is always false. ObjectManager.cs 1423
  • V3022 Expression 'newLength == int.MaxValue' is always false. ObjectManager.cs 1511
  • V3022 Expression 'newLength == int.MaxValue' is always false. ObjectManager.cs 1558

Всё, что отличается в других методах — тип элементов массива temp (не FixupHolder, а long или object). Так что есть у меня всё-таки подозрения на copy-paste…

Issue 40

Data. Код из проекта System. Odbc.

public string UnquoteIdentifier(....)
{ .... if (!string.IsNullOrEmpty(quotePrefix) || quotePrefix != " ") { .... } ....
}

Предупреждение PVS-Studio: V3022 Expression '!string.IsNullOrEmpty(quotePrefix) || quotePrefix != " "' is always true. OdbcCommandBuilder.cs 338

И это действительно так. Анализатор считает, что приведённое выражение всегда имеет значение true. Давайте разбираться. Причём неважно, какое значение содержит quotePrefix по факту — неправильно записано само условие.

С левым всё понятно. Имеем оператор ||, следовательно, значением выражения будет true, если левый или правый (или оба) операнд будет иметь значение true. Следовательно, если выражение составлено так, что значение правого операнда всегда true, когда значение левого — false, результат всего выражения будет постоянно true. Правый операнд будет вычисляться только в случае, если левый имеет значение false.

IsNullOrEmpty(quotePrefix)true, следовательно, верно одно из утверждений: Из приведённого выше кода знаем, что если вычисляется правый операнд, то значение выражения string.

  • quotePrefix == null;
  • quotePrefix.Length == 0.

При истинности любого из этих утверждений также будет истинно выражение quotePrefix != " ", что мы и хотели доказать. Следовательно, значение всего выражения всегда — true, независимо от содержимого quotePrefix.

Issue 41

Возвращаясь к разговору о конструкторах с неиспользуемыми параметрами:

private sealed class PendingGetConnection
{ public PendingGetConnection( long dueTime, DbConnection owner, TaskCompletionSource<DbConnectionInternal> completion, DbConnectionOptions userOptions) { DueTime = dueTime; Owner = owner; Completion = completion; } public long DueTime { get; private set; } public DbConnection Owner { get; private set; } public TaskCompletionSource<DbConnectionInternal> Completion { get; private set; } public DbConnectionOptions UserOptions { get; private set; }
}

Предупреждение PVS-Studio: V3117 Constructor parameter 'userOptions' is not used. DbConnectionPool.cs 26

Очень похоже, что одно из свойств инициализировать забыли. Из кода и предупреждения анализатора видно, что не используется только один параметр конструктора — userOptions, при том, что другие используются для инициализации одноимённых свойств.

Issue 42

Паттерн одинаковый.
Есть подозрительный код, который встретился в этом проекте 2 раза.

private DataTable ExecuteCommand(....)
{ .... foreach (DataRow row in schemaTable.Rows) { resultTable.Columns .Add(row["ColumnName"] as string, (Type)row["DataType"] as Type); } ....
}

Предупреждения PVS-Studio:

  • V3051 An excessive type cast. The object is already of the 'Type' type. DbMetaDataFactory.cs 176
  • V3051 An excessive type cast. The object is already of the 'Type' type. OdbcMetaDataFactory.cs 1109

Подозрительно выглядит выражение (Type)row[«DataType»] as Type. Сначала будет выполнено явное приведение, затем — приведение через оператор as. Если значение row[«DataType»]null, оно успешно 'пройдёт' через оба приведения и пойдёт в качестве аргумента методу Add. Если row[«DataType»] возвращает значение, которое не удастся привести к типу Type, уже при явном приведении будет сгенерировано исключение типа InvalidCastException. В итоге, зачем здесь два приведения? Вопрос открытый.

Issue 43

Runtime. Посмотрим на подозрительный фрагмент из System. RuntimeInformation.
InteropServices.

public static string FrameworkDescription
{ get { if (s_frameworkDescription == null) { string versionString = (string)AppContext.GetData("FX_PRODUCT_VERSION"); if (versionString == null) { .... versionString = typeof(object).Assembly .GetCustomAttribute< AssemblyInformationalVersionAttribute>() ?.InformationalVersion; .... int plusIndex = versionString.IndexOf('+'); .... } .... } .... }
}

Предупреждение PVS-Studio: V3105 The 'versionString' variable was used after it was assigned through null-conditional operator. NullReferenceException is possible. RuntimeInformation.cs 29

При получении значения для переменной авторы кода используют оператор '?.', чтобы не получить исключения NullReferenceException при обращении к свойству InfromationalVersion. Анализатор предупреждает о возможном возникновении исключения типа NullReferenceException при вызове метода IndexOf для переменной versionString. Шутка в том, что если вызов GetCustomAttribute<...> вернёт null, исключение всё равно будет сгенерировано, но ниже — при вызове метода IndexOf, так как versionString будет иметь значение null.

Issue 44

ComponentModel. Обратимся к проекту System. Сразу 2 предупреждения было выдано на следующий код:
Composition
и посмотрим несколько предупреждений.

public static bool CanSpecialize(....)
{ .... object[] genericParameterConstraints = ....; GenericParameterAttributes[] genericParameterAttributes = ....; // if no constraints and attributes been specifed, anything can be created if ((genericParameterConstraints == null) && (genericParameterAttributes == null)) { return true; } if ((genericParameterConstraints != null) && (genericParameterConstraints.Length != partArity)) { return false; } if ((genericParameterAttributes != null) && (genericParameterAttributes.Length != partArity)) { return false; } for (int i = 0; i < partArity; i++) { if (!GenericServices.CanSpecialize( specialization[i], (genericParameterConstraints[i] as Type[]). CreateTypeSpecializations(specialization), genericParameterAttributes[i])) { return false; } } return true;
}

Предупреждения PVS-Studio:

  • V3125 The 'genericParameterConstraints' object was used after it was verified against null. Check lines: 603, 589. GenericSpecializationPartCreationInfo.cs 603
  • V3125 The 'genericParameterAttributes' object was used after it was verified against null. Check lines: 604, 594. GenericSpecializationPartCreationInfo.cs 604

В коде имеются проверки genericParameterAttributes != null и genericParameterConstraints != null. Следовательно, null — допустимые значения для этих переменных, учитываем. Если обе переменные имеют значение null, выходим из метода — вопросов нет. А что, если какая-нибудь из переменных null, но при этом не будет осуществлён выход из метода? Если возможно такое состояние, и исполнение дойдёт до прохода по циклу, получим исключение типа NullReferenceException.

Issue 45

А хотя, давайте поступим иначе — сначала опять воспользуемся классом, а затем посмотрим на код. Посмотрим ещё на одно интересное предупреждение из этого же проекта. 6. Подключим в проект одноимённый NuGet пакет последней доступной prerelease версии (я поставил пакет версии 4. 19303. 0-preview6. Напишем простенький код, например, такой:
8).

LazyMemberInfo lazyMemberInfo = new LazyMemberInfo();
var eq = lazyMemberInfo.Equals(null);
Console.WriteLine(eq);

Метод Equals не прокомментирован, на docs.microsoft.com я не нашёл описание этого метода для .NET Core, только для .NET Framework. Если взглянуть на него ("LazyMemberInfo.Equals(Object) Method") — ничего необычного не видно — возвращает true или false, информации о генерируемых исключениях нет. Запускаем код на исполнение и видим:

Picture 16

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

LazyMemberInfo lazyMemberInfo = new LazyMemberInfo();
var eq = lazyMemberInfo.Equals(typeof(String));
Console.WriteLine(eq);

Результат исполнения кода.

Picture 17

Заглянем же внутрь метода Equals.
Что интересно, оба этих исключения генерируются в одном и том же выражении.

public override bool Equals(object obj)
{ LazyMemberInfo that = (LazyMemberInfo)obj; // Difefrent member types mean different members if (_memberType != that._memberType) { return false; } // if any of the lazy memebers create accessors in a delay-loaded fashion, // we simply compare the creators if ((_accessorsCreator != null) || (that._accessorsCreator != null)) { return object.Equals(_accessorsCreator, that._accessorsCreator); } // we are dealing with explicitly passed accessors in both cases if(_accessors == null || that._accessors == null) { throw new Exception(SR.Diagnostic_InternalExceptionMessage); } return _accessors.SequenceEqual(that._accessors);
}

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

Исключения, однако, происходят выше — при выполнении выражения (LazyMemberInfo)obj. На самом деле, здесь анализатор немного дал маху, так как выдал предупреждение на выражение that._memberType. Уже взято на заметку.

Почему генерируется NullReferenceException? С InvalidCastException, думаю, всё понятно. А распаковка значения null как раз и приводит к возникновению исключения типа NullReferenceException. Дело в том, что LazyMemberInfo — структура, соответственно, выполняется распаковка. Ну и явное выкидывание исключения тоже остаётся на совести авторов. Ну а ещё здесь пара опечаток в комментариях — тоже заодно можно поправить.

Issue 46

Drawing. Что-то подобное, кстати, встретилось в System. Common, в структуре TriState.

public override bool Equals(object o)
{ TriState state = (TriState)o; return _value == state._value;
}

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

Проблемы такие же, как в описанном выше случае.

Issue 47

Text. Рассмотрим несколько фрагментов из System. Json.

Закрепим.
Помните, мы говорили о том, что ToString не должен возвращать null?

public override string ToString()
{ switch (TokenType) { case JsonTokenType.None: case JsonTokenType.Null: return string.Empty; case JsonTokenType.True: return bool.TrueString; case JsonTokenType.False: return bool.FalseString; case JsonTokenType.Number: case JsonTokenType.StartArray: case JsonTokenType.StartObject: { // null parent should have hit the None case Debug.Assert(_parent != null); return _parent.GetRawValueAsString(_idx); } case JsonTokenType.String: return GetString(); case JsonTokenType.Comment: case JsonTokenType.EndArray: case JsonTokenType.EndObject: default: Debug.Fail($"No handler for {nameof(JsonTokenType)}.{TokenType}"); return string.Empty; }
}

На первый взгляд этот метод null и не возвращает, но анализатор утверждает обратное.

JsonElement.cs 1460 Предупреждение PVS-Studio: V3108 It is not recommended to return 'null' from 'ToSting()' method.

Посмотрим на него:
Анализатор указывает на строку с вызовом метода GetString().

public string GetString()
{ CheckValidInstance(); return _parent.GetString(_idx, JsonTokenType.String);
}

Погружаемся глубже — в перегруженную версию метода GetString:

internal string GetString(int index, JsonTokenType expectedType)
{ .... if (tokenType == JsonTokenType.Null) { return null; } ....
}

И тут уже видим условие, при выполнении которого будет возвращено значение null — как из этого метода, так и из изначально рассматриваемого ToString.

Issue 48

Ещё одно интересное место:

internal JsonPropertyInfo CreatePolymorphicProperty(....)
{ JsonPropertyInfo runtimeProperty = CreateProperty(property.DeclaredPropertyType, runtimePropertyType, property.ImplementedPropertyType, property?.PropertyInfo, Type, options); property.CopyRuntimeSettingsTo(runtimeProperty); return runtimeProperty;
}

Предупреждение PVS-Studio: V3042 Possible NullReferenceException. The '?.' and '.' operators are used for accessing members of the 'property' object JsonClassInfo.AddProperty.cs 179

DeclaredPropertyType, property. При вызове метода CreateProperty несколько раз обращаются к свойствам через переменную property: property. PropertyInfo. ImplementedPropertyType, property?. Если он здесь не лишний и property может иметь значение null, этот оператор ничем не поможет, так как при прямом доступе будет сгенерировано исключение типа NullReferenceException. Как видите, в одном случае используют оператор '?.'.

Issue 49

Security. Следующие подозрительные места были найдены в проекте System. Xml и идут они парой, как это уже несколько раз было с другими предупреждениями. Cryptography. Код опять похож на copy-paste, сравните сами.

Первый фрагмент:

public void Write(StringBuilder strBuilder, DocPosition docPos, AncestralNamespaceContextManager anc)
{ docPos = DocPosition.BeforeRootElement; foreach (XmlNode childNode in ChildNodes) { if (childNode.NodeType == XmlNodeType.Element) { CanonicalizationDispatcher.Write( childNode, strBuilder, DocPosition.InRootElement, anc); docPos = DocPosition.AfterRootElement; } else { CanonicalizationDispatcher.Write(childNode, strBuilder, docPos, anc); } }
}

Второй фрагмент:

public void WriteHash(HashAlgorithm hash, DocPosition docPos, AncestralNamespaceContextManager anc)
{ docPos = DocPosition.BeforeRootElement; foreach (XmlNode childNode in ChildNodes) { if (childNode.NodeType == XmlNodeType.Element) { CanonicalizationDispatcher.WriteHash( childNode, hash, DocPosition.InRootElement, anc); docPos = DocPosition.AfterRootElement; } else { CanonicalizationDispatcher.WriteHash(childNode, hash, docPos, anc); } }
}

Предупреждения PVS-Studio:

  • V3061 Parameter 'docPos' is always rewritten in method body before being used. CanonicalXmlDocument.cs 37
  • V3061 Parameter 'docPos' is always rewritten in method body before being used. CanonicalXmlDocument.cs 54

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

Issue 50

Data. Рассмотрим несколько предупреждений на код проекта System. SqlClient.

private bool IsBOMNeeded(MetaType type, object value)
{ if (type.NullableType == TdsEnums.SQLXMLTYPE) { Type currentType = value.GetType(); if (currentType == typeof(SqlString)) { if (!((SqlString)value).IsNull && ((((SqlString)value).Value).Length > 0)) { if ((((SqlString)value).Value[0] & 0xff) != 0xff) return true; } } else if ((currentType == typeof(string)) && (((String)value).Length > 0)) { if ((value != null) && (((string)value)[0] & 0xff) != 0xff) return true; } else if (currentType == typeof(SqlXml)) { if (!((SqlXml)value).IsNull) return true; } else if (currentType == typeof(XmlDataFeed)) { return true; // Values will eventually converted to unicode string here } } return false;
}

Предупреждение PVS-Studio: V3095 The 'value' object was used before it was verified against null. Check lines: 8696, 8708. TdsParser.cs 8696

Похоже, что она затерялась там во время рефакторинга, так как выше value многократно разыменовывается. Анализатор смутила проверка value != null в одном из условий. Если же value может иметь значение null — дела обстоят плохо.

Issue 51

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

protected virtual TDSMessageCollection CreateQueryResponse(....)
{ .... if (....) { .... } else if ( lowerBatchText.Contains("name") && lowerBatchText.Contains("state") && lowerBatchText.Contains("databases") && lowerBatchText.Contains("db_name")) // SELECT [name], [state] FROM [sys].[databases] WHERE [name] = db_name() { // Delegate to current database response responseMessage = _PrepareDatabaseResponse(session); } ....
}

Предупреждение PVS-Studio: V3053 An excessive expression. Examine the substrings 'name' and 'db_name'. QueryEngine.cs 151

Contains(«name») и lowerBatchText. Дело в том, что в данном случае сочетание подвыражений lowerBatchText. В самом деле, если проверяемая строка содержит подстроку «db_name», то она будет содержать и подстроку «name». Contains(«db_name») является избыточным. В итоге получается, что проверка lowerBatchText. Если же строка не содержит «name», то и «db_name» она содержать не будет. Разве что она может сократить количество вычисляемых выражений, если проверяемая строка не содержит «name». Contains(«name») является избыточной.

Issue 52

Net. Подозрительный фрагмент кода из проекта System. Requests.

protected override PipelineInstruction PipelineCallback( PipelineEntry entry, ResponseDescription response, ....)
{ if (NetEventSource.IsEnabled) NetEventSource.Info(this, $"Command:{entry?.Command} Description:{response?.StatusDescription}"); // null response is not expected if (response == null) return PipelineInstruction.Abort; .... if (entry.Command == "OPTS utf8 on\r\n") .... ....
}

Предупреждение PVS-Studio: V3125 The 'entry' object was used after it was verified against null. Check lines: 270, 227. FtpControlStream.cs 270

Command и response?. При составлении интерполированной строки используются выражения entry?. Вместо оператора '.' используют '?.', чтобы не получить исключение типа NullReferenceException, если какой-то из соответствующих параметров будет иметь значение null. Description. Далее, как видно из кода, возможное null значение для response отсекают (выход из метода, если response == null), а вот для entry ничего подобного нет. В данном случае этот приём работает. Command (уже с использованием '.', а не '?.') будет сгенерировано исключение. В итоге, если entrynull, ниже, при вызове entry.

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

Picture 23

Тогда продолжим. Вернулись? 🙂

Issue 53

Collections. Теперь посмотрим на что-то интересное из проекта System. В этот раз немного поэкспериментируем со структурой System. Immutable. Immutable. Collections. Нас интересуют методы IStructuralEquatable. ImmutableArray<T>. CompareTo. Equals и IStructuralComparable.

Equals. Начнём с метода IStructuralEquatable. Код приведён ниже, предлагаю самостоятельно попробовать понять, что в нём не так:

bool IStructuralEquatable.Equals(object other, IEqualityComparer comparer)
{ var self = this; Array otherArray = other as Array; if (otherArray == null) { var theirs = other as IImmutableArray; if (theirs != null) { otherArray = theirs.Array; if (self.array == null && otherArray == null) { return true; } else if (self.array == null) { return false; } } } IStructuralEquatable ours = self.array; return ours.Equals(otherArray, comparer);
}

Удалось? Если да — опять же, принимайте поздравления. 🙂

Check lines: 1212, 1204. Предупреждение PVS-Studio: V3125 The 'ours' object was used after it was verified against null. ImmutableArray_1.cs 1212

Каким образом он пришёл к этому заключению? Анализатор смутил вызов экземплярного метода Equals через переменную ours, находящийся в последнем выражении return, так как он предполагает, что здесь возможно возникновение исключения типа NullReferenceException. Чтобы легче было объяснить, ниже привожу упрощённый код этого же метода.

bool IStructuralEquatable.Equals(object other, IEqualityComparer comparer)
{ .... if (....) { .... if (....) { .... if (self.array == null && otherArray == null) { .... } else if (self.array == null) { .... } } } IStructuralEquatable ours = self.array; return ours.Equals(otherArray, comparer);
}

По последним выражениям мы видим, что значение переменной ours получается из self.array. Выше же несколько раз выполняется проверка self.array == null. Следовательно, self.array, а значит и ours, могут иметь значение null. По крайней мере, в теории. Достижимо ли такое состояние на практике? Давайте попробуем выяснить. Для этого ещё раз привожу тело метода с расставленными ключевыми точками.

bool IStructuralEquatable.Equals(object other, IEqualityComparer comparer)
{ var self = this; // <= 1 Array otherArray = other as Array; if (otherArray == null) // <= 2 { var theirs = other as IImmutableArray; if (theirs != null) // <= 3 { otherArray = theirs.Array; if (self.array == null && otherArray == null) { return true; } else if (self.array == null) // <= 4 { return false; } } IStructuralEquatable ours = self.array; // <= 5 return ours.Equals(otherArray, comparer);
}

Ключевая точка 1. self.array == this.array (из-за self = this). Следовательно, перед вызовом метода необходимо достичь состояния this.array == null.

Мы можем проигнорировать этот if, что будет наиболее простым вариантом достижения цели, или же пойти внутрь. Ключевая точка 2. Тогда после использования оператора as в otherArray будет записана ненулевая ссылка, и мы проигнорируем первый оператор if. Чтобы проигнорировать этот if, достаточно, чтобы тип переменной other был типом Array или производным от него и other не имела значения null.

Этот пункт подразумевает более сложный путь. Ключевая точка 3. Если этого не случится и начнётся исполнение then-ветви, мы гарантированно не достигнем нужного нам пункта 5 при условии self.array == null за счёт ключевой точки 4. Нам обязательно нужно выйти на втором операторе if (тот, что с условным выражением theirs != null). Для того, чтобы не зайти в оператор if ключевой точки 3, необходимо выполнение одного из условий:

  • чтобы значение other было null;
  • чтобы фактический тип other не реализовывал интерфейс IImmutableArray.

Ключевая точка 5. Если мы достигли этой точки со значением self.array == null, значит, мы добились своей цели, и в методе будет сгенерировано исключение типа NullReferenceException.

Получаем следующие наборы данных, которые приведут нас к цели.

Первое: this.array — null.

Второе — одно из следующих:

  • othernull;
  • other имеет тип Array или производный от него;
  • other не имеет тип Array или производный от него, и при этом не реализует интерфейс IImmutableArray.

array — поле, объявленное следующим образом:

internal T[] array;

Так как ImmutableArray<T> — структура, она имеет конструктор по умолчанию (без аргументов), в результате исполнения которого поле array примет значение по умолчанию, то есть — null. А это то, что нам нужно.

Ну и не забудем, что мы исследовали явную реализацию интерфейсного метода, следовательно, перед вызовом нужно будет выполнить приведение.

Подключаем отладочную версию библиотеки, пишем код, исполняем, наблюдаем. Теперь мы имеем все карты на руках, чтобы добиться возникновения исключения тремя способами.

Фрагмент кода 1.

var comparer = EqualityComparer<String>.Default;
ImmutableArray<String> immutableArray = new ImmutableArray<string>();
((IStructuralEquatable)immutableArray).Equals(null, comparer);

Фрагмент кода 2.

var comparer = EqualityComparer<String>.Default;
ImmutableArray<String> immutableArray = new ImmutableArray<string>();
((IStructuralEquatable)immutableArray).Equals(new string[] { }, comparer);

Фрагмент кода 3.

var comparer = EqualityComparer<String>.Default;
ImmutableArray<String> immutableArray = new ImmutableArray<string>();
((IStructuralEquatable)immutableArray).Equals(typeof(Object), comparer);

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

Picture 18

Issue 54

🙂 Только в этот раз так подробно разбирать его не будем. Если вы не забыли, у нас ещё один метод, который нужно постараться скомпрометировать. Тем более, что часть информации нам уже известна из предыдущего примера.

int IStructuralComparable.CompareTo(object other, IComparer comparer)
{ var self = this; Array otherArray = other as Array; if (otherArray == null) { var theirs = other as IImmutableArray; if (theirs != null) { otherArray = theirs.Array; if (self.array == null && otherArray == null) { return 0; } else if (self.array == null ^ otherArray == null) { throw new ArgumentException( SR.ArrayInitializedStateNotEqual, nameof(other)); } } } if (otherArray != null) { IStructuralComparable ours = self.array; return ours.CompareTo(otherArray, comparer); // <= } throw new ArgumentException(SR.ArrayLengthsNotEqual, nameof(other));
}

Предупреждение PVS-Studio: V3125 The 'ours' object was used after it was verified against null. Check lines: 1265, 1251. ImmutableArray_1.cs 1265

Как видите, ситуация очень похожа на рассмотренный ранее пример.

Напишем следующий код:

Object other = ....;
var comparer = Comparer<String>.Default;
ImmutableArray<String> immutableArray = new ImmutableArray<string>();
((IStructuralComparable)immutableArray).CompareTo(other, comparer);

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

Значение: othernew String[]{ };

Результат:

Picture 22

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

Issue 55

Net. В проекте System. И вновь у меня закрадываются смутные сомнения по поводу copy-paste. HttpListener встретилось несколько мест, мало того, что подозрительных, так и очень похожих друг на друга. Так как паттерн одинаковый, рассмотрим один пример кода, для остальных случаев приведу предупреждения анализатора:

public override IAsyncResult BeginRead(byte[] buffer, ....)
{ if (NetEventSource.IsEnabled) { NetEventSource.Enter(this); NetEventSource.Info(this, "buffer.Length:" + buffer.Length + " size:" + size + " offset:" + offset); } if (buffer == null) { throw new ArgumentNullException(nameof(buffer)); } ....
}

Предупреждение PVS-Studio: V3095 The 'buffer' object was used before it was verified against null. Check lines: 51, 53. HttpRequestStream.cs 51

Однако, если значение выражения NetEventSource. Генерация исключения типа ArgumentNullException при условии buffer == null явно намекает на то, что null — недопустимое значение для этой переменной. Length. IsEnabledtrue, и при этом buffernull, будет сгенерировано исключение типа NullReferenceException при вычислении выражения buffer. Соответственно, до проверки buffer == null в этом случае дело даже не дойдёт.

Предупреждения PVS-Studio, выданные на другие методы с точно таким же паттерном:

  • V3095 The 'buffer' object was used before it was verified against null. Check lines: 49, 51. HttpResponseStream.cs 49
  • V3095 The 'buffer' object was used before it was verified against null. Check lines: 74, 75. HttpResponseStream.cs 74

Issue 56

Transactions. Подобный код встретился и в проекте System. Local.

internal override void EnterState(InternalTransaction tx)
{ if (tx._outcomeSource._isoLevel == IsolationLevel.Snapshot) { throw TransactionException.CreateInvalidOperationException( TraceSourceType.TraceSourceLtm, SR.CannotPromoteSnapshot, null, tx == null ? Guid.Empty : tx.DistributedTxId); } ....
}

Предупреждение PVS-Studio: V3095 The 'tx' object was used before it was verified against null. Check lines: 3282, 3285. TransactionState.cs 3282

При вызове метода для создания объекта исключения используют параметр tx, при этом его проверяют на равенство null, чтобы не сгенерировать исключение типа NullReferenceException при вычислении выражения tx. При определённом условии хотят сгенерировать исключение типа InvalidOperationException. Ирония в том, что, если txnull эта проверка не поможет, так как при вычислении условия оператора if выполняется обращение к экземплярным полям через переменную txtx._outcomeSource._isoLevel. DistributedTxId.

Issue 57

Runtime. Код из проекта System. Caching.

internal void SetLimit(int cacheMemoryLimitMegabytes)
{ long cacheMemoryLimit = cacheMemoryLimitMegabytes; cacheMemoryLimit = cacheMemoryLimit << MEGABYTE_SHIFT; _memoryLimit = 0; // never override what the user specifies as the limit; // only call AutoPrivateBytesLimit when the user does not specify one. if (cacheMemoryLimit == 0 && _memoryLimit == 0) { // Zero means we impose a limit _memoryLimit = EffectiveProcessMemoryLimit; } else if (cacheMemoryLimit != 0 && _memoryLimit != 0) { // Take the min of "cache memory limit" and // the host's "process memory limit". _memoryLimit = Math.Min(_memoryLimit, cacheMemoryLimit); } else if (cacheMemoryLimit != 0) { // _memoryLimit is 0, but "cache memory limit" // is non-zero, so use it as the limit _memoryLimit = cacheMemoryLimit; } ....
}

Предупреждение PVS-Studio: V3022 Expression 'cacheMemoryLimit != 0 && _memoryLimit != 0' is always false. CacheMemoryMonitor.cs 250

Так как _memoryLimit имеет значение 0 (выставляется перед оператором if), правый операнд оператора && имеет значение false, следовательно, результат всего выражения также false. Если внимательно посмотреть на код, можно заметить, что одно из выражений — cacheMemoryLimit != 0 && _memoryLimit != 0 — всегда будет иметь значение false.

Issue 58

Diagnostics. Ниже привожу подозрительный фрагмент кода из проекта System. TraceSource.

public override object Pop()
{ StackNode n = _stack.Value; if (n == null) { base.Pop(); } _stack.Value = n.Prev; return n.Value;
}

Предупреждение PVS-Studio: V3125 The 'n' object was used after it was verified against null. Check lines: 115, 111. CorrelationManager.cs 115

Из-за проверки n == null предположу, что null — возможное значение для этой локальной переменной. Интересный код. Prev. Если это так — ниже будет сгенерировано исключение типа NullReferenceException при доступе к экземплярному свойству — n. Pop() в данном контексте никогда не будет выполнен. Если n в данном случае никогда не может быть null, то вызов base.

Issue 59

Drawing. Интересное место нашлась в проекте System. И вновь предлагаю найти проблему самостоятельно. Primitives. Вот код:

public static string ToHtml(Color c)
{ string colorString = string.Empty; if (c.IsEmpty) return colorString; if (ColorUtil.IsSystemColor(c)) { switch (c.ToKnownColor()) { case KnownColor.ActiveBorder: colorString = "activeborder"; break; case KnownColor.GradientActiveCaption: case KnownColor.ActiveCaption: colorString = "activecaption"; break; case KnownColor.AppWorkspace: colorString = "appworkspace"; break; case KnownColor.Desktop: colorString = "background"; break; case KnownColor.Control: colorString = "buttonface"; break; case KnownColor.ControlLight: colorString = "buttonface"; break; case KnownColor.ControlDark: colorString = "buttonshadow"; break; case KnownColor.ControlText: colorString = "buttontext"; break; case KnownColor.ActiveCaptionText: colorString = "captiontext"; break; case KnownColor.GrayText: colorString = "graytext"; break; case KnownColor.HotTrack: case KnownColor.Highlight: colorString = "highlight"; break; case KnownColor.MenuHighlight: case KnownColor.HighlightText: colorString = "highlighttext"; break; case KnownColor.InactiveBorder: colorString = "inactiveborder"; break; case KnownColor.GradientInactiveCaption: case KnownColor.InactiveCaption: colorString = "inactivecaption"; break; case KnownColor.InactiveCaptionText: colorString = "inactivecaptiontext"; break; case KnownColor.Info: colorString = "infobackground"; break; case KnownColor.InfoText: colorString = "infotext"; break; case KnownColor.MenuBar: case KnownColor.Menu: colorString = "menu"; break; case KnownColor.MenuText: colorString = "menutext"; break; case KnownColor.ScrollBar: colorString = "scrollbar"; break; case KnownColor.ControlDarkDark: colorString = "threeddarkshadow"; break; case KnownColor.ControlLightLight: colorString = "buttonhighlight"; break; case KnownColor.Window: colorString = "window"; break; case KnownColor.WindowFrame: colorString = "windowframe"; break; case KnownColor.WindowText: colorString = "windowtext"; break; } } else if (c.IsNamedColor) { if (c == Color.LightGray) { // special case due to mismatch between Html and enum spelling colorString = "LightGrey"; } else { colorString = c.Name; } } else { colorString = "#" + c.R.ToString("X2", null) + c.G.ToString("X2", null) + c.B.ToString("X2", null); } return colorString;
}

Ладно-ладно, снова эти мои шутки… Или же вы смогли? В любом случае, сократим код, чтобы более явно обозначить проблему.

Вот сокращённая версия кода:

switch (c.ToKnownColor())
{ .... case KnownColor.Control: colorString = "buttonface"; break; case KnownColor.ControlLight: colorString = "buttonface"; break; ....
}

Предупреждение PVS-Studio: V3139 Two or more case-branches perform the same actions. ColorTranslator.cs 302

В остальных случаях, когда для нескольких элементов перечисления хотели вернуть одно и то же значение, использовали несколько case, следующих друг за другом. Не могу сказать наверняка, но думаю, что это всё-таки ошибка. А ошибиться при copy-paste здесь достаточно легко, как мне кажется.

Чтобы получить на выходе анализируемого метода ToHtml значение «buttonface», на вход можно передать одно из следующих значений (ожидаемо): Копнём ещё чуть глубже.

  • SystemColors.Control;
  • SystemColors.ControlLight.

Если для каждого из этих цветов посмотреть значения ARGB, можно увидеть следующее:

  • SystemColors.Control(255, 240, 240, 240);
  • SystemColors.ControlLight — (255, 227, 227, 227).

Если позвать на полученном значении («buttonface») обратный метод конвертации — FromHtml, получим цвет Control (255, 240, 240, 240). Можно ли получить из FromHtml цвет ControlLight? Да. В этом методе есть таблица цветов, на основании которой (в частном случае) получается цвет. В инициализаторе таблицы есть следующая строка:

s_htmlSysColorTable["threedhighlight"] = ColorUtil.FromKnownColor(KnownColor.ControlLight);

Соответственно, FromHtml возвращает цвет ControlLight (255, 227, 227, 227) для значения «threedhighlight». Я думаю, именно это значение и должно было использоваться в case KnownColor.ControlLight.

Issue 60

Text. Посмотрим на парочку интересных предупреждений из проекта System. RegularExpressions.

internal virtual string TextposDescription()
{ var sb = new StringBuilder(); int remaining; sb.Append(runtextpos); if (sb.Length < 8) sb.Append(' ', 8 - sb.Length); if (runtextpos > runtextbeg) sb.Append(RegexCharClass.CharDescription(runtext[runtextpos - 1])); else sb.Append('^'); sb.Append('>'); remaining = runtextend - runtextpos; for (int i = runtextpos; i < runtextend; i++) { sb.Append(RegexCharClass.CharDescription(runtext[i])); } if (sb.Length >= 64) { sb.Length = 61; sb.Append("..."); } else { sb.Append('$'); } return sb.ToString();
}

Предупреждение PVS-Studio: V3137 The 'remaining' variable is assigned but is not used by the end of the function. RegexRunner.cs 612

Возможно, какой-то код, использующий её, удалили, но про саму переменную забыли. В локальную переменную remaining записывают какое-то значение, но до конца метода её больше не используют. Или же здесь более серьёзная ошибка и эта переменная как-то должна использоваться.

Issue 61

public void AddRange(char first, char last)
{ _rangelist.Add(new SingleRange(first, last)); if (_canonical && _rangelist.Count > 0 && first <= _rangelist[_rangelist.Count - 1].Last) { _canonical = false; }
}

Предупреждение PVS-Studio: V3063 A part of conditional expression is always true if it is evaluated: _rangelist.Count > 0. RegexCharClass.cs 523

Count > 0 — будет всегда иметь значение true, если этот код будет исполнен. Анализатор справедливо заметил, что часть выражения — _rangelist. Add(....) — он таковым уже не будет. Даже если список, на который указывает _rangelist, был пустым, после добавления элемента — _rangelist.

Issue 62

Drawing. Посмотрим на срабатывания диагностического правила V3128 в проектах System. Transactions. Common и System. Local.

private class ArrayEnumerator : IEnumerator
{ private object[] _array; private object _item; private int _index; private int _startIndex; private int _endIndex; public ArrayEnumerator(object[] array, int startIndex, int count) { _array = array; _startIndex = startIndex; _endIndex = _index + count; _index = _startIndex; } ....
}

Предупреждение PVS-Studio: V3128 The '_index' field is used before it is initialized in constructor. PrinterSettings.Windows.cs 1679

Ниже поле _index уже инициализируется. При инициализации поля _endIndex используется другое поле — _index, которое на момент использования имеет стандартное значение — default(int), то есть 0. Если вдруг это не ошибка — переменную _index следовало бы опустить в этом выражении, чтобы не сбивала с толку.

Issue 63

internal class TransactionTable
{ .... private int _timerInterval; .... internal TransactionTable() { // Create a timer that is initially disabled by specifing // an Infinite time to the first interval _timer = new Timer(new TimerCallback(ThreadTimer), null, Timeout.Infinite, _timerInterval); .... // Store the timer interval _timerInterval = 1 << TransactionTable.timerInternalExponent; .... }
}

Предупреждение PVS-Studio: V3128 The '_timerInterval' field is used before it is initialized in constructor. TransactionTable.cs 151

Сначала используют значение поля _timerInterval (пока оно ещё равно default(int)) для инициализации _timer, и лишь затем инициализируют само поле _timerInterval. Ситуация аналогична описанной выше.

Issue 64

У него пока нет документации и окончательного сообщения, но с его помощью уже удалось найти пару интересных мест. Следующие предупреждения были получены диагностическим правилом, которое ещё находится в разработке. Опять эти места выглядят как copy-paste, так что мы рассмотрим только один фрагмент кода.

private bool ProcessNotifyConnection(....)
{ .... WeakReference reference = (WeakReference)( LdapConnection.s_handleTable[referralFromConnection]); if ( reference != null && reference.IsAlive && null != ((LdapConnection)reference.Target)._ldapHandle) { .... } ....
}

Предупреждение PVS-Studio (заглушка): VXXXX TODO_MESSAGE. LdapSessionOptions.cs 974

IsAlive будет выполнена сборка мусора и под неё попадёт объект, на который ссылается WeakReference, reference. Опасность здесь заключается в том, что если после проверки reference. Как следствие — при доступе к экземплярному полю _ldapHandle возникнет исключение типа NullReferenceException. Target вернёт значение null. Цитата с docs.microsoft.com — "WeakReference. Собственно, про эту ловушку с проверкой IsAlive предупреждает и сам Microsoft. IsAlive Property": Because an object could potentially be reclaimed for garbage collection immediately after the IsAlive property returns true, using this property is not recommended unless you are testing only for a false return value.

Обобщение по результатам анализа

Все ли это ошибки и интересные места, найденные в результате анализа? Конечно, нет! Начиная просматривать результаты анализа, я внимательно просматривал предупреждения. По мере того, как их количество увеличивалось и становилось понятно, что на статью предупреждений наберётся, я пролистывал результаты, стараясь отобрать только то, что покажется мне наиболее интересным. Когда добрался до последних (самых больших логов), сил оставалось уже только на то, что пролистывать все предупреждения, пока взгляд не зацепится за что-нибудь необычное. Так что, если покопаться, уверен, можно найти ещё много интересных мест.

Условно говоря, если встречался код вида:
Например, я игнорировал почти все предупреждения V3022 и V3063.

String str = null;
if (str == null) ....

Я его пропускал, так как были места поинтереснее, которые хотелось описать. Были срабатывания на небезопасную блокировку с использованием lock statement с блокировкой по this и т.п. — V3090; небезопасные вызовы событий — V3083; на объекты, типы которых реализуют IDisposable, но для которых не вызывается Dispose / Close — V3072 и аналогичные диагностики; и многое другое.

Кроме пары мест, показавшихся мне достаточно интересными, чтобы обратить на них внимание. Не выписывал (по крайней мере — старался, случайно мог что-то выписать) я проблемы, найденные в тестах. Но тестирующий код ведь тоже может содержать ошибки, из-за чего тесты будут работать неверно.

В общем, ещё есть, что поизучать — но выписать абсолютно все найденные проблемы целью перед собой я не ставил.

Одни проекты — идеально чистые, другие же содержат подозрительные места. Качество кода показалось мне неравномерным. Пожалуй, чистота проектов ожидаема, особенно в случае с наиболее часто используемыми библиотечными классами.

Но, как следует из этой статьи, без тёмных уголков не обошлось. Если обобщать — наверное, можно сказать, что код достаточно качественный ввиду того, объём кода был проанализирован всё же немалый.

Мне удалось найти ряд ложных / странных предупреждений, которые я отобрал для изучения и исправления. Кстати, проект такого объёма — это также хороший тест для анализатора. Так что в результате анализа удалось найти те точки, где стоит поработать и над самим PVS-Studio.

Заключение

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

Picture 24

Если возникнут какие-то вопросы или просто захочется поделиться интересными обнаруженными местами — смело пишите на support@viva64.com. Так или иначе, помощь статического анализа лишней не будет, поэтому предлагаю попробовать PVS-Studio на своём проекте и посмотреть, какие интересные места удастся найти с его помощью. 🙂

Всего хорошего!

P.S. Обращение к разработчикам библиотек .NET Core

Большое спасибо за то, что вы делаете! Вы — большие молодцы. Надеюсь, эта статья поможет сделать код немного лучше. Помните, что в статье я выписал не все подозрительные места, и лучше самостоятельно проверить проект с помощью анализатора — так можно более подробно изучить все предупреждения, да и работать будет удобнее, чем с простым текстовым логом / перечнем ошибок (чуть подробнее я писал об этом здесь).

Checking the . Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev. NET Core Libraries Source Code by the PVS-Studio Static Analyzer

Теги
Показать больше

Похожие статьи

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

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

Кнопка «Наверх»
Закрыть