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

Время от времени пользователи присылают нам различный код на C++. С их точки зрения, анализатор PVS-Studio выдавал странные/ложные предупреждения по этим фрагментам. Затем мы улучшаем диагностику или записываем идеи для будущих исправлений. Другими словами, рутинная работа по поддержке.

Но не в этот раз. Разработчик, получивший письмо от пользователя, с радостью прибежал ко мне с идеей написать об этом случае. Мы не можем раскрыть имя пользователя или показать его код. Итак, вот переписанная и сокращенная версия:

char *SomeGlobalPointer {};

void foo()
{
  SomeGlobalPointer = new char[1024];
}

int main()
{
  foo();

  if (!SomeGlobalPointer)
  {
    delete[] SomeGlobalPointer;
  }

  return 0;
}

Разработчик из команды пользователя говорит, что PVS-Studio выдает «ложное срабатывание»:

V575 Нулевой указатель передается в «оператор delete[]». Проверьте аргумент.

Итак, почему мы думаем, что это нулевой указатель? Смотреть! Он явно инициализируется в функции.

Это интересный случай, когда код кажется настолько простым, что разработчик не видит ошибки. Код освобождения памяти скучный, что затрудняет внимательное изучение кода во время проверки кода. Я уже описывал подобный случай. Об этом вы можете прочитать в статье: Зло внутри функций сравнения».

Собственно, вот незаметная опечатка в условии:

if (!SomeGlobalPointer)
{
  delete[] SomeGlobalPointer;
}

Оператор delete[] вызывается, только если указатель равен нулю. В результате происходит утечка памяти. Действительно, условие необходимо обратить:

if (SomeGlobalPointer)
{
  delete[] SomeGlobalPointer;
}

На самом деле проверка ненужна. Оператор delete[] правильно обрабатывает нулевые указатели. Давайте упростим код:

char *SomeGlobalPointer {};

void foo()
{
  SomeGlobalPointer = new char[1024];
}

int main()
{
  foo();
  delete[] SomeGlobalPointer;
  return 0;
}

Поскольку мы говорим о рефакторинге, мы также предлагаем использовать здесь интеллектуальный указатель (при желании мы можем использовать std::unique_ptr или std::shared_ptr). Конечно, код выглядит странно. Но не забывайте, что пример, приведенный в статье, является синтетическим. Однако мы отвлеклись от главного.

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

Если бы PVS-Studio не помог, эта ошибка все равно осталась бы в коде и привела бы к утечке памяти.

Слава статическому анализу кода! Повысьте эффективность проверки кода и позвольте анализатору помочь вам!

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