Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Ищем и анализируем ошибки в коде GitExt…

Ищем и анализируем ошибки в коде GitExtensions

13 Окт 2016

Как известно Ядро Git представляет собой набор утилит командной строки с параметрами. Для комфортной работы как правило используются утилиты, дающие нам привычный графический интерфейс. Вот и мне довелось в свое время поработать с такой утилитой для Git, как GitExtensions. Не скажу, что это самый удобный инструмент, которым мне доводилось пользоваться (к примеру, тот же TortoiseGit мне нравится больше), но он явно и не безосновательно занимает свою нишу в списке любимых и проверенных утилит для работы с Git.

0440_GitExtensions_ru/image1.png

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

GitExtensions

GitExtensions - кроссплатформенный визуальный клиент для работы с системой управления версиями Git с открытым исходным кодом.

Проект GitExtensions небольшой. В сумме это 10 основных проектов, 20 плагинов и 2 дополнительных проектов, компилируемых в вспомогательные библиотеки. В общей сложности 56 091 строк кода в 441 файле.

Давайте посмотрим, сможет ли что-то интересное найти в этом проекте статический анализатор PVS-Studio.

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

По итогам проверки было получено 121 предупреждение. Если рассматривать более подробно, то 16 предупреждений было получено на первом (высоком) уровне. 11 из них явно указывали на проблемные места или ошибки. На втором уровне (среднем) было получено 80 предупреждений. По моему субъективному мнению, 69 предупреждений были верными и указывали на места с ошибками или опечатками. Третий (низкий) уровень предупреждений мы рассматривать не будем, так как он в основном указывает на места, где возникновение ошибок маловероятно. И так, приступим к рассмотрению найденных ошибок.

0440_GitExtensions_ru/image2.png

Выражение всегда истинно

Начинает наш хит-парад довольно странный участок кода.

string rev1 = "";
string rev2 = "";

var revisions = RevisionGrid.GetSelectedRevisions();
if (revisions.Count > 0)
{
  rev1 = ....;
  rev2 = ....;
  ....
}
else

if (string.IsNullOrEmpty(rev1) || string.IsNullOrEmpty(rev2)) // <=
{
    MessageBox.Show(....);
    return;
}

V3022 Expression 'string.IsNullOrEmpty(rev1) || string.IsNullOrEmpty(rev2)' is always true. GitUI FormFormatPatch.cs 155

Анализатор выдал предупреждение, что выражение с проверкой переменных rev1 и rev2 всегда будет истинным. Сначала я подумал, что это обычная опечатка, оплошность в логике алгоритма, которая никак не повлияет на корректность работы программы. Но рассмотрев код более детально заметил, по всей видимости лишний оператор else. Он находится перед второй проверкой, которая будет выполнена только в случае неравенства первой.

Одно из сравнений избыточно

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

public void EditNotes(string revision)
{
  string editor = GetEffectivePathSetting("core.editor").ToLower();
  if (editor.Contains("gitextensions") || 
      editor.Contains("notepad") || // <=
      editor.Contains("notepad++")) // <=
  {
    RunGitCmd("notes edit " + revision);
  }
  ....
}

V3053 An excessive expression. Examine the substrings 'notepad' and 'notepad++'. GitCommands GitModule.cs 691

В выражении ищется более длинная (notepad++) и более короткая (notepad) подстроки. При этом проверка с более длинной строкой никогда не сработает, так как проверка с поиском более короткой строки будет ее предотвращать. Как я уже упомянул, это не ошибка, а всего лишь опечатка, но в другой ситуации невинная избыточная проверка могла бы превратиться в потенциально коварный баг.

Переменная используется перед проверкой на null

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

void DataReceived(string data)
{
  if (data.StartsWith(CommitBegin)) // <=
  {
      ....
  }
  
  if (!string.IsNullOrEmpty(data))
  {
      ....
  }
}

V3095 The 'data' object was used before it was verified against null. Check lines: 319, 376. GitCommands RevisionGraph.cs 319

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

Возможно возникновение исключения NullReferenceException в переопределенном методе Equals

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

public override bool Equals(object obj)
{
  return GetHashCode() == obj.GetHashCode(); // <=
}

V3115 Passing 'null' to 'Equals(object obj)' method should not result in 'NullReferenceException'. Git.hub User.cs 16

При дальнейшем вызове переопределенного метода Equals возможно возникновение исключения NullReferenceException, если параметр obj будет равен null. Для предотвращения данной ситуации необходимо использовать проверку на null. Например, так:

public override bool Equals(object obj)
{
  return GetHashCode() == obj?.GetHashCode(); // <=
}

Идентичные выражения в условии if

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

private void ConfigureRemotes()
{
  ....
  if (!remoteHead.IsRemote ||
    localHead.IsRemote ||
    !string.IsNullOrEmpty(localHead.GetTrackingRemote(localConfig)) ||
    !string.IsNullOrEmpty(localHead.GetTrackingRemote(localConfig)) ||
    remoteHead.IsTag ||
    localHead.IsTag ||
    !remoteHead.Name.ToLower().Contains(localHead.Name.ToLower()) ||
    !remoteHead.Name.ToLower().Contains(_remote.ToLower()))
    continue;
  ....
}

V3001 There are identical sub-expressions to the left and to the right of the '||' operator. GitUI FormRemotes.cs 192

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

Так же была найдена еще одна подобная ошибка.

if (!curItem.IsSourceEqual(item.NeutralValue) && // <=
  (!String.IsNullOrEmpty(curItem.TranslatedValue) && 
  !curItem.IsSourceEqual(item.NeutralValue))) // <=
{
  curItem.TranslatedValue = "";
}

V3001 There are identical sub-expressions to the left and to the right of the '&&' operator. TranslationApp TranslationHelpers.cs 112

Несоответствие количества параметров при форматировании строки

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

Debug.WriteLine("Loading plugin...", pluginFile.Name); // <=

V3025 Incorrect format. A different number of format items is expected while calling 'WriteLine' function. Arguments not used: pluginFile.Name. GitUI LoadPlugins.cs 35

В данной ситуации же могу предположить, что программист думал, что значение переменной pluginFile.Name будет добавлено в конец форматируемой строки при выводе в дебаг, но это не так. Корректный код будет выглядеть так:

Debug.WriteLine("Loading '{0}' plugin...", pluginFile.Name); // <=

Часть выражения всегда истинна

И в завершение еще одна опечатка, которой можно было бы избежать.

private void toolStripButton(...)
{
  var button = (ToolStripButton)sender;
  if (!button.Enabled)
  {
    StashMessage.ReadOnly = true;
  }
  else if (button.Enabled && button.Checked) // <=
  {
    StashMessage.ReadOnly = false;
  }
}

V3063 A part of conditional expression is always true if it is evaluated: button.Enabled. GitUI FormStash.cs 301

Поскольку мы проверяем, что button.Enabled равен false, то в else данной проверки мы можем смело утверждать, что button.Enabled будет равен true и таким образом исключить данную проверку повторно.

Заключение

В данном проекте были найдены и другие ошибки, опечатки, недочёты. Но мне они не показались интересными, чтобы описывать их в статье. Разработчики GitExtensions легко смогут найти все недочёты, воспользовавшись инструментом PVS-Studio. Вы так же можете поискать ошибки в своих проектах воспользовавшись предложенным выше статическим анализатором.

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

Популярные статьи по теме
PVS-Studio ROI

Дата: 30 Янв 2019

Автор: Андрей Карпов

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Зло живёт в функциях сравнения

Дата: 19 Май 2017

Автор: Андрей Карпов

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

Дата: 31 Май 2014

Автор: Андрей Карпов

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
Любите статический анализ кода!

Дата: 16 Окт 2017

Автор: Андрей Карпов

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

Дата: 21 Ноя 2018

Автор: Андрей Карпов

Краткое описание технологий, используемых в инструменте PVS-Studio, которые позволяют эффективно обнаруживать большое количество паттернов ошибок и потенциальных уязвимостей. Статья описывает реализа…
PVS-Studio для Java

Дата: 17 Янв 2019

Автор: Андрей Карпов

В седьмой версии статического анализатора PVS-Studio мы добавили поддержку языка Java. Пришло время немного рассказать, как мы начинали делать поддержку языка Java, что у нас получилось и какие дальн…
Главный вопрос программирования, рефакторинга и всего такого

Дата: 14 Апр 2016

Автор: Андрей Карпов

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

Автор: Андрей Карпов

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Как PVS-Studio оказался внимательнее, чем три с половиной программиста

Дата: 22 Окт 2018

Автор: Андрей Карпов

PVS-Studio, как и другие статические анализаторы кода, часто выдаёт ложные срабатывания. Но не стоит спешить считать странные срабатывания ложными. Это короткая история о том, как PVS-Studio вновь ок…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

Автор: Андрей Карпов

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

Комментарии (0)

Следующие комментарии
Этот сайт использует куки и другие технологии, чтобы предоставить вам более персонализированный опыт. Продолжая просмотр страниц нашего веб-сайта, вы принимаете условия использования этих файлов. Если вы не хотите, чтобы ваши данные обрабатывались, пожалуйста, покиньте данный сайт. Подробнее →
Принять