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

Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
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 года перед одним из релизов. При регулярном использовании статический анализатор сэкономит массу времени на поиск опечаток и ошибок.

Популярные статьи по теме
Насколько хорошо защищены ваши пароли? Проверка проекта Bitwarden

Дата: 13 Май 2022

Автор: Никита Паневин

Bitwarden – менеджер паролей с открытым исходным кодом. Это программное обеспечение помогает генерировать уникальные пароли и управлять ими. Получится ли у анализатора PVS-Studio отыскать ошибки в та…
Зачем разработчикам игр на Unity использовать статический анализ?

Дата: 11 Май 2022

Автор: Артём Ровенский

С годами стоимость создания игр стала больше, вырос их масштаб, а следовательно, и их кодовая база. Разработчикам становится всё сложнее уследить за ошибками. А забагованная игра влечёт финансовые и …
Эволюция PVS-Studio: анализ потока данных для связанных переменных

Дата: 28 Апр 2022

Автор: Никита Липилин

Связанные переменные – одна из главных проблем статического анализа. Данная статья посвящена разбору этой темы и рассказу о том, как разработчики PVS-Studio сражаются с ложными срабатываниями, появив…
Зачем нужен статический анализ? Разбираем на примере ошибки из Akka.NET

Дата: 25 Апр 2022

Автор: Сергей Васильев

"Статический анализ нужно использовать регулярно, а не только перед релизами... Чем раньше найдена ошибка, тем дешевле её исправление..." – вы уже слышали это 100 раз. Сегодня ещё раз ответим на вопр…
Атака Trojan Source: скрытые уязвимости

Дата: 15 Апр 2022

Автор: Гость

Мы представляем новый тип атаки для внедрения в исходный код вредоносных изменений, по-разному выглядящих для компилятора и человека. Такая атака эксплуатирует тонкости стандартов кодирования символо…

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

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