Сегодня я вновь возвращаюсь к проекту Tizen. В своей недавней заметке "Эксперимент по поиску ошибок в коде C# компонентов Tizen" в нашем блоге я провел поверхностный анализ и пришел к выводу, что имеет смысл проверить весь код C# компонентов этого проекта на наличие ошибок при помощи анализатора PVS-Studio и написать про это статью. Не откладывая в долгий ящик, я проделал эту работу и хочу поделиться с вами её результатами. Сразу скажу, что на C# коде анализатор PVS-Studio показал себя слабо. Однако обо всём по порядку: давайте посмотрим, что смог найти анализатор, а затем займёмся статистикой и подведём итоги.
Мой коллега Андрей Карпов не так давно публиковал две эпические статьи, посвященные анализу кода проекта Tizen, написанного на языках Си и Си++:
Когда я заметил, что проект Tizen включает в себя код и на языке C#, мне захотелось сделать аналогичную статью о проверке компонентов, написанных на этом языке. К сожалению, в этот раз анализатор не смог продемонстрировать яркие успехи, но давайте не будем торопиться и изучим вопрос детально.
Исходный код Tizen доступен для загрузки. Репозиторий содержит около 1 000 проектов, каждый из которых состоит из архива с исходным кодом, а также вспомогательных файлов. По именам архивов или описанию не всегда возможно понять, что же находится внутри. Поэтому потребовалась загрузка, распаковка и изучение архивов для всего репозитория.
В предыдущей статье я приводил общее число файлов исходного кода C# (4 929, исключая *.Designer.cs) и строк кода в них (около 691 000), содержащееся в проекте Tizen. Сейчас нам потребуется более детальный анализ. Для начала попробуем найти файлы с расширением .sln или .csproj. Наличие таких файлов позволит проводить анализ в IDE Visual Studio, что значительно упростит работу.
Итак, в ходе поиска было найдено 227 решений (*.sln) и 166 проектов C# (*.csproj). Из файлов решений я выбрал те, которые содержали в своем составе проекты C#. Таких решений оказалось всего 3:
Первые два решения являются надстройкой Tizen над сторонним компонентом Xamarin.Forms, а третье содержит сам компонент. Чуть более года назад мы писали статью про проверку Xamarin.Forms. В своей работе я учту эти результаты и попробую найти новые ошибки.
Далее, исключив файлы проектов (*.csproj), входящие в состав этих решений, я получил 107 проектов C#, которые не были связаны ни с какими решениями. Практически все они находятся в папках верхнего уровня с именами вида "csapi-*". Исключив из этого числа 11 тестовых проектов, а также 9 проектов, которые имели неподдерживаемый Visual Studio формат, я получил в остатке 87 проектов. Каждый из них я проверял отдельно.
Для чистоты исследования я решил отделить результаты, полученные для внутренних C# компонентов Tizen (те самые 87 проектов), от результатов проверки компонентов на базе Xamarin.Forms. Сначала я не хотел учитывать Xamarin.Forms, но, поразмыслив, пришел к выводу, что раз уж Tizen использует этот компонент для своих целей, то и ошибки Xamarin.Forms могут оказывать влияние на Tizen.
Также я не буду описывать ошибки, которые уже приводил в предыдущей статье.
В ходе проверки этой части проекта Tizen анализатор PVS-Studio выдал 356 предупреждений, из них 18 - уровня достоверности High, 157 - уровня достоверности Medium и 181 - уровня достоверности Low. Было проанализировано около 325 000 строк кода.
Предупреждения уровня достоверности Low я не рассматривал, так как там обычно велик процент ложных срабатываний. Впрочем, к сожалению, много ложных срабатываний в этот раз находятся не только на уровне Low. Среди 175 предупреждений уровней High и Medium мне удалось обнаружить всего 12 ошибок. Давайте взглянем на наиболее интересные.
Предупреждение PVS-Studio: V3008 The '_scanData' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 138, 137. Tizen.Network.Bluetooth BluetoothLeAdapter.cs 138
CWE-563. Assignment to Variable without Use ('Unused Variable')
internal BluetoothLeDevice(BluetoothLeScanData scanData)
{
_scanData = new BluetoothLeScanData ();
_scanData = scanData;
....
}Полю _scanData дважды присваивают значение. Выглядит очень подозрительно. На всякий случай взглянем на объявление класса BluetoothLeScanData и его конструктор. Возможно, вызов конструктора содержит какие-то дополнительные действия. Класс небольшой, поэтому приведу его целиком, немного отформатировав оригинальный код:
internal class BluetoothLeScanData
{
internal string RemoteAddress { get; set; }
internal BluetoothLeDeviceAddressType AddressType { get; set; }
internal int Rssi { get; set; }
internal int AdvDataLength { get; set; }
internal byte[] AdvData { get; set; }
internal int ScanDataLength { get; set; }
internal byte[] ScanData { get; set; }
}Как видим, класс не содержит переопределённого конструктора по умолчанию, поэтому, по всей видимости, двойное присвоение значения полю _scanData является ошибкой.
Предупреждение PVS-Studio: V3009 It's odd that this method always returns one and the same value of '0'. Tizen.Applications.WidgetApplication WidgetType.cs 47
CWE-393. Return of Wrong Status Code
private int OnCreate(....)
{
WidgetBase b = Activator.CreateInstance(ClassType) as WidgetBase;
....
if (b == null)
return 0;
....
return 0;
}Метод всегда возвращает 0, вне зависимости от результата своей работы. Вероятно, допущена ошибка, которую можно исправить, например, так:
private int OnCreate(....)
{
WidgetBase b = Activator.CreateInstance(ClassType) as WidgetBase;
....
if (b == null)
return 0;
....
return 1;
}Предупреждения PVS-Studio:
CWE-570/CWE-571 Expression is Always False/True
private TEdge ProcessBound(TEdge E, bool LeftBoundIsForward)
{
....
if (LeftBoundIsForward)
{
....
if (!LeftBoundIsForward) Result = Horz.Prev;
....
}
else
{
....
if (!LeftBoundIsForward) Result = Horz.Next;
....
}
....
}Данный фрагмент кода содержит сразу 2 однотипные ошибочные проверки. При этом в первом случае переменная Result никогда не получит значения Horz.Prev, а во втором случае та же переменная Result всегда получит значение Horz.Next. Автору необходимо внимательно изучить данный код и самостоятельно исправить ошибку.
Предупреждение PVS-Studio: V3022 Expression 'e.OutIdx >= 0' is always true. clipper_library clipper.cs 3144
CWE-571 Expression is Always True
private void DoMaxima(TEdge e)
{
....
if(....)
{
....
} else if( e.OutIdx >= 0 && eMaxPair.OutIdx >= 0 )
{
if (e.OutIdx >= 0) AddLocalMaxPoly(e, eMaxPair, e.Top);
....
}
....
}Ещё один фрагмент кода с ошибочной проверкой. Возможно, если бы в условии e.OutIdx >= 0 && eMaxPair.OutIdx >= 0 был использован оператор "||", то проверка e.OutIdx >= 0 во вложенном блоке if имела бы смысл. Сейчас это выглядит подозрительно.
Предупреждение PVS-Studio: V3110 Possible infinite recursion inside 'InsertBefore' method. ElmSharp Toolbar.cs 288
CWE-674 Uncontrolled Recursion
public ToolbarItem InsertBefore(ToolbarItem before, string label)
{
return InsertBefore(before, label);
}Вызов метода InsertBefore порождает бесконечную рекурсию. Вероятно, ошибка допущена вследствие вызова не той перегрузки метода. В коде присутствует ещё один метод с именем InsertBefore:
public ToolbarItem InsertBefore(ToolbarItem before, string label,
string icon)
{
....
}Пожалуй, это все интересные ошибки данного раздела. Есть ещё несколько подозрительных участков кода, но я не буду на них останавливаться. Код от Samsung Electronics, написанный на C#, демонстрирует хорошее качество. Почему я так уверен, что проверенный код имеет авторство Samsung? Да потому, что каждый из проверенных файлов содержал в своём заголовке комментарий вида "Copyright (c) 2016 Samsung Electronics Co., Ltd All Rights Reserved".
Используемая в Tizen надстройка над Xamarin.Forms содержит около 242 000 строк кода. В ходе её проверки анализатор PVS-Studio выдал 163 предупреждения, из них 10 - уровня достоверности High, 46 - уровня достоверности Medium и 107 - уровня достоверности Low (не рассматривались).
Как и обещал, я попытаюсь найти ошибки, которые не были описаны в предыдущей статье про проверку Xamarin.Forms. Кстати, некоторые из приведенных там ошибок я не обнаружил при новой проверке. По всей видимости, они были исправлены после знакомства авторов со статьей.
Несмотря на небольшое число выданных предупреждений, мне удалось найти среди них несколько новых ошибок.
Предупреждение PVS-Studio: V3095 The 'context' object was used before it was verified against null. Check lines: 16, 18. Xamarin.Forms.Xaml XamlServiceProvider.cs 16
CWE-476 NULL Pointer Dereference
internal XamlServiceProvider(INode node, HydratationContext context)
{
....
if (node != null && node.Parent != null
&& context.Values.TryGetValue(node.Parent, // <=
out targetObject))
IProvideValueTarget = new XamlValueTargetProvider(....);
if (context != null) // <=
IRootObjectProvider =
new XamlRootObjectProvider(context.RootElement);
....
}Переменную context сначала используют, а только затем проверяют на равенство null.
Предупреждение PVS-Studio: V3095 The 'type' object was used before it was verified against null. Check lines: 147, 149. Xamarin.Forms.Xaml ExpandMarkupsVisitor.cs 147
CWE-476 NULL Pointer Dereference
public INode Parse(....)
{
....
var xmltype = new XmlType(namespaceuri, type.Name, null); // <=
if (type == null)
throw new NotSupportedException();
....
}Еще один пример возможного выброса исключения NullReferenceException. Переменную type используют для создания экземпляра класса XmlType, а уже затем проверяют на равенство null.
Другие аналогичные ошибки:
Предупреждение PVS-Studio: V3125 The 'e.NewItems' object was used after it was verified against null. Check lines: 999, 986. Xamarin.Forms.Core TemplatedItemsList.cs 999
CWE-476 NULL Pointer Dereference
void OnProxyCollectionChanged(....)
{
....
if (e.NewStartingIndex >= 0 && e.NewItems != null) // <=
maxindex = Math.Max(maxindex, e.NewStartingIndex +
e.NewItems.Count);
....
for (int i = e.NewStartingIndex; i < _templatedObjects.Count; i++)
SetIndex(_templatedObjects[i], i + e.NewItems.Count); // <=
....
}А здесь - обратная ситуация. Переменную e.NewItems проверяют на равенство null перед первым использованием. Однако при повторном использовании это сделать забывают.
Как писал мой коллега Андрей Карпов в одной из предыдущих статей, анализатор PVS-Studio выявляет около 0.4 ошибки на 1000 строк C/C++ кода проекта Tizen. Давайте подсчитаем, что у нас получается для C# кода.
Всего было проверено около 567 000 строк кода на языке C#.
На мой взгляд, найдено только 15 фрагментов кода, про которые можно сказать, что они содержат ошибки.
Получается, что анализатор обнаруживает 0.02 ошибки на 1000 строк кода. Или, другими словами, он находит 1 ошибку на 50 000 строк кода. Маловато.
Приходится признать, что данном случае анализатор не смог продемонстрировать свою полезность. Почему так получилось - сказать сложно. При проверке других открытых проектов анализатор часто демонстрировал хорошие результаты. Например, при проверке открытых компонентов Unity3D плотность обнаруживаемых ошибок составляла 0.5 ошибки на 1000 строк кода. Т.е. показатель был в 25 раз лучше.
Возможно, проверенные сейчас компоненты очень качественные, возможно, анализатор не умеет находить типы ошибок, свойственные этому проекту. Может быть причина тому - сложная инфраструктура Tizen. Зачастую проверялись проекты, не содержащие всего необходимого окружения, отсутствовали многие ссылки, что не позволило отработать некоторым диагностикам. Некоторые проекты мне вообще не удалось проверить.
Итак, результат проверки оказался не таким, как я ожидал для такого объема кода. Скажу честно: я предполагал найти как минимум несколько сотен ошибок, но нашлось около пятнадцати.
Однако важно то, что анализатор PVS-Studio справился с задачей по анализу C# кода проекта Tizen. А значит, может быть полезен если не сейчас, то в дальнейшем, когда в Tizen будут появляться новые компоненты, написанные с использованием языка C#. Подтверждением потенциальной пользы является огромное количество ошибок, которые анализатор уже обнаружил в других открытых проектах (см. список статей).
Более того, как мы не устаём повторять, сценарий единичных проверок с использованием анализатора не оптимален – ошибки уже заложены в систему контроля версий, что, согласитесь, плохо. Гораздо эффективнее пользоваться статическим анализатором регулярно, что позволит исправлять ошибки на этапе написания кода, до момента попадания в систему контроля версий, ведь в таком случае стоимость и сложность их исправления куда ниже.
Загрузить и попробовать PVS-Studio: http://www.viva64.com/ru/pvs-studio/