После того, как я написал довольно большую статью об анализе кода ОС Tizen, я получил большое количество вопросов, касающихся процента ложных срабатываний и плотности ошибок (сколько ошибок обнаруживает PVS-Studio на 1000 строк кода). Видимо, моих рассуждений о том, что это сильно зависит от анализируемого проекта и настроек анализатора, оказалось недостаточно. Поэтому я решил привести конкретные цифры, проведя более тщательное расследование одного из проектов ОС Tizen. Я решил, что взять EFL Core Libraries будет достаточно интересно, потому что один из разработчиков, Carsten Haitzler, принимал активное участие в обсуждении моих статей. Я надеюсь, что эта статья докажет Карстену, что PVS-Studio — достойный инструмент.

Предыстория

Если нашлись люди, которые пропустили новость, то просто сообщаю, что недавно я написал открытое письмо разработчикам Tizen, а затем монументальную статью 27000 ошибок в операционной системе Tizen.

После этого было несколько новостных постов на разных ресурсах и довольно оживленные обсуждения. Вот некоторые из них:

Выражаю особую благодарность Carsten Haitzler еще раз за внимание к моему посту и активное его обсуждение.

Поднимались разные темы, некоторые из них были более подробно освещены в посте Тизен: подведение итогов.

Однако есть два вечных вопроса, которые продолжают преследовать меня.

  • Каков процент ложных срабатываний?
  • Сколько ошибок находит PVS-Studio на 1000 строк кода?

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

Поэтому дам ответ на примере конкретного проекта. Я выбрал основные библиотеки EFL. Во-первых, этот проект является частью Tizen. Во-вторых, как я уже сказал, одним из разработчиков является Карстен Хейтцлер, которому, вероятно, эти результаты были бы интересны.

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

The Enlightenment Foundation Libraries (EFL) — это набор графических библиотек, выросших из разработки Enlightenment, оконного менеджера и компоновщика Wayland.

Для проверки основных библиотек EFL я использовал свежий код, взятый из репозитория https://git.enlightenment.org/.

Стоит отметить, что данный проект проверяется статическим анализатором кода Coverity. Вот комментарий на эту тему:

Я скажу, что мы серьезно относимся к проверке. Coverity сообщает об уровне ошибок 0 для Enlightenment upstream (мы исправили все проблемы, на которые указывает Coverity, или отклонили их как ложные после тщательного изучения), а уровень ошибок для EFL составляет 0,04 проблемы на 1 тыс. строк кода, что довольно мало ( найти проблемы достаточно просто, если кодовая база большая). В основном они не так сильно влияют на вещи. В каждом выпуске, который мы выпускаем, процент ошибок снижается, и мы, как правило, пытаемся «исправить проблемы» за несколько недель до выпуска.

Итак, давайте посмотрим, что нам может показать PVS-Studio.

Характеристики

После правильной настройки PVS-Studio будет выдавать 10–15% ложных срабатываний при анализе основных библиотек EFL.

На данный момент плотность обнаруживаемых ошибок в основных библиотеках EFL составляет 0,71 ошибки на 1000 строк кода.

Как я делал расчеты

Проект EFL Core Libraries на момент анализа насчитывает около 1 616 000 строк кода, написанного на C и C++. 17,7% из них — комментарии. Таким образом, количество строк кода без комментариев — 1 330 000.

После первого запуска я увидел следующее количество предупреждений общего анализа (GA):

  • Высокий уровень достоверности: 605
  • Средний уровень достоверности: 3924
  • Низкий уровень уверенности: 1186

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

Практически весь проект написан на C, и в результате в нем широко используются макросы. Они являются причиной большинства ложных срабатываний. Я потратил около 40 минут на беглый просмотр отчета и придумал файл efl_settings.txt.

Файл содержит необходимые настройки. Для их использования при анализе проекта необходимо указать в конфигурационном файле анализатора (например, в PVS-Studio.cfg) следующее:

rules-config=/path/to/efl_settings.txt

Анализатор можно запустить следующим образом:

pvs-studio-analyzer analyze ... --cfg /path/to/PVS-Studio.cfg ...

или вот так:

pvs-studio ... --cfg /patn/to/PVS-Studio.cfg ...

в зависимости от способа интеграции.

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

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

Второй запуск дал следующие результаты:

  • Высокий уровень достоверности: 189
  • Средний уровень достоверности: 1186
  • Низкий уровень уверенности: 1186

Число 1186 повторяется дважды. Это не опечатка. Эти цифры действительно оказались одинаковыми.

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

Всего у меня получилось 189 +1186 = 1375 сообщений (High + Medium), с которыми я начал работать.

Изучив эти предупреждения, я полагаю, что анализатор обнаружил 950 фрагментов кода, содержащих ошибки. Другими словами, я нашел 950 фрагментов, требующих исправления. Подробнее об этих ошибках я расскажу в следующей главе.

Оценим плотность обнаруженных ошибок.

950*1000/1330000 = около 0,71 ошибки на 1000 строк кода.

Теперь оценим процент ложных срабатываний:

((1375–950) / 1375) * 100% = 30%

Ну, подожди! В начале статьи было число 10–15% ложных срабатываний. Здесь 30%.

Позволь мне объяснить. Итак, просмотрев отчет о 1375 предупреждениях, я пришел к выводу, что 950 из них указывают на ошибки. Осталось 425 предупреждений.

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

Рассмотрим один пример сообщения, которое я решил пропустить.

....
uint64_t callback_mask;
....
static void
_check_event_catcher_add(void *data, const Efl_Event *event)
{
  ....
  Evas_Callback_Type type = EVAS_CALLBACK_LAST;
  ....
  else if ((type = _legacy_evas_callback_type(array[i].desc)) !=
           EVAS_CALLBACK_LAST)
  {
    obj->callback_mask |= (1 << type);
  }
  ....
}

Предупреждение PVS-Studio: V629 Попробуйте проверить выражение ‘1 ‹‹ type’. Битовый сдвиг 32-битного значения с последующим расширением до 64-битного типа. evas_callbacks.c 709

Давайте внимательно посмотрим на эту строку:

obj->callback_mask |= (1 << type);

Используется для записи 1 в нужный бит переменной callback_mask. Обратите внимание, что переменная callback_mask имеет 64-битный тип.

Оператор (1 ‹‹ type) имеет тип int, поэтому вы можете изменить только биты в младшей части переменной. Биты [32–63] изменить нельзя.

Чтобы понять, есть ошибка или нет, нам нужно понять, какой диапазон значений может возвращать функция _legacy_evas_callback_type. Может ли он вернуть значение больше 31? Я не знаю, поэтому я пропускаю это предупреждение.

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

Комментарий Карстена Хейтцлера. Выше — на самом деле ошибка, которая является результатом оптимизации, которая устанавливает биты, чтобы решить, следует ли пытаться сопоставлять новые типы событий со старыми (мы рефакторим огромные куски наших внутренностей вокруг новой объектной системы, и поэтому мы должны сделать это, чтобы сохранить совместимость, но, как и при любом рефакторинге… всякое случается). Да — он переносит битовый сдвиг и выполняет дополнительную работу целой кучи if, потому что одни и те же биты в маске повторно используются для двух событий из-за переноса. Таким образом, это не приводит к ошибке, просто немного меньше микрооптимизаций при установке, так как сейчас этот бит означает «у него есть обратный вызов события для типа A OR B», а не просто «тип A» … следующий код фактически выполняет полную проверку /отображение. Он, конечно, не предназначался для обертывания, так что это была ловушка, но то, как он использовался, означает, что он был на самом деле довольно безвредным.

Среди оставшихся 425 будут предупреждения, указывающие на ошибки. Пока я их просто пропустил.

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

После внимательного просмотра остальных предупреждений и дополнительных настроек будет 10–15% ложных срабатываний. Это хороший результат.

Найдены ошибки

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

Читатель также может посмотреть все предупреждения, открыв файл отчета: zip-архив с отчетом. Обратите внимание, что я оставил только общие предупреждения высокого и среднего уровня достоверности.

Я просмотрел этот отчет в Windows с помощью утилиты PVS-Studio Standalone.

В Linux вы можете использовать утилиту Plog Converter, которая преобразует отчет в один из следующих форматов:

  • xml — удобный формат для дальнейшей обработки результатов анализа, который поддерживается плагином для SonarQube;
  • csv — текстовый формат для предоставления данных в виде таблицы;
  • файл_ошибок – формат вывода gcc и clang;
  • tasklist — формат ошибки, который можно открыть в QtCreator.

Далее для просмотра отчетов можно использовать QtCreator, Vim/gVim, GNU Emacs, LibreOffice Calc. Документация Как запустить PVS-Studio в Linux подробно описывает этот процесс. (см. Фильтрация и просмотр отчета анализатора).

V501 (1 ошибка)

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

static int
_ephysics_body_evas_stacking_sort_cb(const void *d1,
                                     const void *d2)
{
   const EPhysics_Body_Evas_Stacking *stacking1, *stacking2;
   stacking1 = (const EPhysics_Body_Evas_Stacking *)d1;
   stacking2 = (const EPhysics_Body_Evas_Stacking *)d2;
   if (!stacking1) return 1;
   if (!stacking2) return -1;
   if (stacking1->stacking < stacking2->stacking) return -1;
   if (stacking2->stacking > stacking2->stacking) return 1;
   return 0;
}

Предупреждение PVS-Studio: V501 Слева и справа от оператора «›» одинаковые подвыражения stacking2-›stacking. ephysics_body.cpp 450

Опечатка. Последнее сравнение должно быть следующим:

if (stacking1->stacking > stacking2->stacking) return 1;

V512 (8 ошибок)

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

typedef struct _Eina_Array Eina_Array;
struct _Eina_Array
{
   int version;
   void **data;
   unsigned int total;
   unsigned int count;
   unsigned int step;
   Eina_Magic __magic;
};

Нет необходимости смотреть на это очень внимательно. Это просто структура с некоторыми полями.

Теперь давайте посмотрим на определение структуры Eina_Accessor_Array:

typedef struct _Eina_Accessor_Array Eina_Accessor_Array;
struct _Eina_Accessor_Array
{
   Eina_Accessor accessor;
   const Eina_Array *array;
   Eina_Magic __magic;
};

Обратите внимание, что указатель на структуру Eina_Array хранится в структуре Eina_Accessor_Array. Кроме этого, эти структуры никак не связаны друг с другом и имеют разные размеры .

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

static Eina_Accessor *
eina_array_accessor_clone(const Eina_Array *array)
{
   Eina_Accessor_Array *ac;
   EINA_SAFETY_ON_NULL_RETURN_VAL(array, NULL);
   EINA_MAGIC_CHECK_ARRAY(array);
   ac = calloc(1, sizeof (Eina_Accessor_Array));
   if (!ac) return NULL;
   memcpy(ac, array, sizeof(Eina_Accessor_Array));
   return &ac->accessor;
}

Предупреждение PVS-Studio: V512 Вызов функции memcpy приведет к выходу буфера массива за пределы допустимого диапазона. eina_array.c 186

Позвольте мне удалить все ненужные детали, чтобы сделать его проще:

.... eina_array_accessor_clone(const Eina_Array *array)
{
   Eina_Accessor_Array *ac = calloc(1, sizeof (Eina_Accessor_Array));
   memcpy(ac, array, sizeof(Eina_Accessor_Array));
}

Память выделяется для объекта типа Eina_Accessor_Array. Дальше происходит странная вещь.

Объект типа Eina_Array копируется в выделенный буфер памяти.

Я не знаю, что эта функция должна делать, но она делает что-то странное.

Во-первых, есть индекс за пределами источника (структуры Eina_Array).

Во-вторых, в этом копировании нет никакого смысла. Структуры имеют набор членов совершенно разных типов.

Комментарий Карстена Хайтцлера. Содержимое функции правильное — введен неверный параметр. На самом деле это не имело значения, потому что функция назначается func ptr, который имеет правильный тип, и, поскольку это общий «родительский класс», назначение приводит к универсальному типу доступа, поэтому компилятор не жаловался, и это, казалось, работало .

Рассмотрим следующую ошибку:

static Eina_Bool _convert_etc2_rgb8_to_argb8888(....)
{
   const uint8_t *in = src;
   uint32_t *out = dst;
   int out_step, x, y, k;
   unsigned int bgra[16];
   ....
   for (k = 0; k < 4; k++)
     memcpy(out + x + k * out_step, bgra + k * 16, 16);
   ....
}

Предупреждение PVS-Studio: V512 Вызов функции memcpy приведет к переполнению буфера bgra + k * 16. draw_convert.c 318

Все очень просто. Обычный индекс массива выходит за границы.

Массив bgra состоит из 16 элементов типа unsigned int.

Переменная k принимает в цикле значения от 0 до 3.

Взгляните на выражение: bgra + k * 16.

Когда переменная k принимает значение больше 0, мы получим оценку указателя, указывающего за пределы массива.

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

#define MATRIX_XX(m) (m)->xx
typedef struct _Eina_Matrix4 Eina_Matrix4;
struct _Eina_Matrix4
{
   double xx;
   double xy;
   double xz;
   double xw;
   double yx;
   double yy;
   double yz;
   double yw;
   double zx;
   double zy;
   double zz;
   double zw;
   double wx;
   double wy;
   double wz;
   double ww;
};
EAPI void
eina_matrix4_array_set(Eina_Matrix4 *m, const double *v)
{
   memcpy(&MATRIX_XX(m), v, sizeof(double) * 16);
}

Предупреждение PVS-Studio: V512 Вызов функции memcpy приведет к переполнению буфера ‘&(m)-›xx’. eina_matrix.c 1003

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

Комментарий Карстена Хайтцлера. Вышеупомянутые и связанные с ними memcpy — не ошибка: использование гарантированного макета памяти в структурах.

Мне это не нравится, на самом деле. Я рекомендую написать что-то вроде этого:

struct _Eina_Matrix4
{
  union {
    struct {
      double xx;
      double xy;
      double xz;
      double xw;
      double yx;
      double yy;
      double yz;
      double yw;
      double zx;
      double zy;
      double zz;
      double zw;
      double wx;
      double wy;
      double wz;
      double ww;
    };
    double RawArray[16];
  };
};
EAPI void
void eina_matrix4_array_set(Eina_Matrix4 *m, const double *v)
{
  memcpy(m->RawArray, v, sizeof(double) * 16);
}

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

Первый метод. Добавьте комментарий к коду:

memcpy(&MATRIX_XX(m), v, sizeof(double) * 16); //-V512

Второй метод. Добавьте строку в файл настроек:

//-V:MATRIX_:512

Третий способ. Использовать базу разметки.

Другие ошибки:

  • V512 Вызов функции memcpy приведет к переполнению буфера ‘&(m)-›xx’. eina_matrix.c 1098
  • V512 Вызов функции memcpy приведет к переполнению буфера ‘&(m)-›xx’. eina_matrix.c 1265
  • V512 Вызов функции «memcpy» приведет к тому, что буфер «&pd-›projection.xx» выйдет за пределы допустимого диапазона. evas_canvas3d_camera.c 120
  • V512 Вызов функции «memcpy» приведет к тому, что буфер «&pd-›projection.xx» выйдет за пределы допустимого диапазона. evas_canvas3d_light.c 270
  • V512 Вызов функции memcpy приведет к переполнению буфера bgra+k*16. draw_convert.c 350

V517 (3 ошибки)

static Eina_Bool
evas_image_load_file_head_bmp(void *loader_data,
                              Evas_Image_Property *prop,
                              int *error)
{
  ....
  if (header.comp == 0) // no compression
  {
    // handled
  }
  else if (header.comp == 3) // bit field
  {
    // handled
  }
  else if (header.comp == 4) // jpeg - only printer drivers
    goto close_file;
  else if (header.comp == 3) // png - only printer drivers
    goto close_file;
  else
    goto close_file;
  ....
}

Предупреждение PVS-Studio: V517 Обнаружено использование паттерна if (A) {…} else if (A) {…}. Существует вероятность наличия логической ошибки. Строки проверки: 433, 439. evas_image_load_bmp.c 433

Переменная header.comp дважды сравнивается с константой 3.

Другие ошибки:

  • V517 Обнаружено использование шаблона «if (A) {…} else if (A) {…}». Существует вероятность наличия логической ошибки. Строки проверки: 1248, 1408. evas_image_load_bmp.c 1248
  • V517 Обнаружено использование шаблона «if (A) {…} else if (A) {…}». Существует вероятность наличия логической ошибки. Проверить строки: 426, 432. parser.c 426

V519 (1 ошибка)

EOLIAN static Efl_Object *
_efl_net_ssl_context_efl_object_finalize(....)
{
  Efl_Net_Ssl_Ctx_Config cfg;
  ....
  cfg.load_defaults = pd->load_defaults;                // <=
  cfg.certificates = &pd->certificates;
  cfg.private_keys = &pd->private_keys;
  cfg.certificate_revocation_lists =
          &pd->certificate_revocation_lists;
  cfg.certificate_authorities = &pd->certificate_authorities;
  cfg.load_defaults = pd->load_defaults;                // <=
  ....
}

Предупреждение PVS-Studio: V519 Переменной cfg.load_defaults дважды подряд присваиваются значения. Возможно, это ошибка. Проверьте строки: 304, 309. efl_net_ssl_context.c 309

Повторное задание. Здесь одно задание лишнее, или что-то еще просто не скопировано.

Комментарий Карстена Хайтцлера. Не ошибка. Просто слишком усердное копирование и вставка строки.

Еще один простой случай:

EAPI Eina_Bool
edje_edit_size_class_add(Evas_Object *obj, const char *name)
{
  Eina_List *l;
  Edje_Size_Class *sc, *s;
  ....
  /* set default values for max */
  s->maxh = -1;
  s->maxh = -1;
  ....
}

Предупреждение PVS-Studio: V519 Переменной ‘s-›maxh’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 8132, 8133. edje_edit.c 8133

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

  • V519 Переменной ‘pdata-›seat-›object.in’ дважды подряд присваиваются значения. Возможно, это ошибка. Контрольные строки: 1519, 1521. evas_events.c 1521
  • V519 Переменной ‘pdata-›seat-›object.in’ дважды подряд присваиваются значения. Возможно, это ошибка. Контрольные строки: 2597, 2599. evas_events.c 2599
  • V519 Переменной b-›buffer[r] дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 348, 353. evas_image_load_pmaps.c 353
  • V519 Переменной attr_amount дважды подряд присваиваются значения. Возможно, это ошибка. Контрольные строки: 13891, 13959. edje_edit.c 13959
  • V519 Переменной async_loader_running дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 152, 165. evas_gl_preload.c 165
  • V519 Переменной textlen дважды подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 86, 87. elm_code_widget_undo.c 87
  • V519 Переменной «content» дважды подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 313, 315. elm_dayselector.c 315
  • V519 Переменной ‘wd-›resize_obj’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 3099, 3105. elm_entry.c 3105
  • V519 Переменной ‘wd-›resize_obj’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 3125, 3131. elm_entry.c 3131
  • V519 Переменной idata-›values ​​дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 128, 129. elm_view_list.c 129
  • V519 Переменной ‘wd-›resize_obj’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 2602, 2608. efl_ui_text.c 2608
  • V519 Переменной ‘wd-›resize_obj’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 2628, 2634. efl_ui_text.c 2634
  • V519 Переменной ‘finfo’ дважды подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 706, 743. evas_image_load_gif.c 743
  • V519 Переменной «current_program_lookups» два раза подряд присваиваются значения. Возможно, это ошибка. Строки проверки: 15819, 15820. edje_cc_handlers.c 15820

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

V522 (563 ошибки)

У проекта EFL есть большая проблема — проверка выделена память или нет. В общем, такие проверки есть. Пример:

if (!(el = malloc(sizeof(Evas_Stringshare_El) + slen + 1)))
  return NULL;

Более того, иногда они находятся в тех местах, где они особо не нужны (о предупреждении V668 см. ниже).

Но в огромном количестве случаев проверок вообще нет. Рассмотрим пару предупреждений анализатора.

Комментарий Карстена Хайтцлера. Хорошо, так что это общепринятое мнение, что, по крайней мере, в Linux, который всегда был нашим основным направлением и долгое время был нашей единственной целью, возврату от malloc/calloc/realloc нельзя доверять, особенно для небольших сумм. Linux по умолчанию выделяет больше памяти. Это означает, что вы получаете новую память, но ядро ​​еще не назначило ей реальные страницы физической памяти. Только виртуальное пространство. Пока не прикоснешься. Если ядро ​​​​не может обслужить этот запрос, ваша программа все равно падает, пытаясь получить доступ к памяти в том, что выглядит как действительный указатель. Таким образом, в целом ценность проверки возвратов allocs, которые малы, по крайней мере, в Linux, невелика. Иногда мы это делаем… иногда нет. Но в целом нельзя доверять возвратам, ЕСЛИ это не очень большой объем памяти, и ваш alloc никогда не будет обслуживаться - например. ваш alloc вообще не может поместиться в виртуальном адресном пространстве (иногда это происходит на 32-битной версии). Да, overcommit можно настроить, но это обходится дорого, что большинство людей никогда не хотят платить, или никто даже не знает, что можно настроить. Во-вторых, может произойти сбой выделения небольшого участка памяти, например. узел связанного списка... реально, если возвращается NULL... сбой - это почти все, что вы можете сделать. У вас настолько мало памяти, что вы можете зависнуть, вызовите abort(), как это делает glib с g_malloc, потому что, если вы не можете выделить 20–40 байт … ваша система все равно рухнет, так как у вас все равно не останется рабочей памяти. Я говорю здесь не о крошечных встроенных системах, а о больших машинах с виртуальной памятью и несколькими мегабайтами памяти и т. д., которые были нашей целью. Я понимаю, почему PVS-Studio это не нравится. Строго говоря, это на самом деле правильно, но на самом деле код, потраченный на обработку этого материала, является пустой тратой кода, учитывая реальность ситуации. Я расскажу об этом позже.

static Eina_Debug_Session *
_session_create(int fd)
{
   Eina_Debug_Session *session = calloc(1, sizeof(*session));
   session->dispatch_cb = eina_debug_dispatch;
   session->fd = fd;
   // start the monitor thread
   _thread_start(session);
   return session;
}

Комментарий Карстена Хайтцлера. Это совершенно новый код, который появился 2 месяца назад, но все еще находится в стадии разработки и тестирования и не готов к использованию в прайм-тайм. Это часть нашей инфраструктуры отладки в режиме реального времени, где любое приложение, использующее EFL, может управляться демоном отладчика (если оно запущено) и контролироваться (проверка всех объектов в памяти и дерева объектов и их состояния с самоанализом в реальном времени во время его выполнения), сбор выполнения журналы временной шкалы (сколько времени тратится в каком дереве вызовов функций, где при рендеринге, в каком потоке — какие потоки используют какое время процессора, в каких слотах до уровня мс и ниже, коррелирует с вызовами функций, состоянием системы анимации и когда пробуждение происходят события и отметка времени устройства, вызвавшая пробуждение, и так далее… так что, учитывая этот сценарий… если вы не можете вызвать крошечную структуру сеанса во время отладки сбоя, доступ к первой странице памяти в значительной степени так же хорош, как и все остальное… выше о памяти и прерываниях и т. д.

Комментарий Андрея Карпова. Не очень понятно, почему здесь упоминается, что это новый и непроверенный код. В первую очередь статические анализаторы предназначены для обнаружения ошибок в новом коде :).

Предупреждение PVS-Studio: V522 Возможно, происходит разыменование потенциального сеанса нулевого указателя. eina_debug.c 440

Программист выделил память с помощью функции calloc и сразу же использовал ее.

Другой пример:

static Reference *
_entry_reference_add(Entry *entry, Client *client,
                     unsigned int client_entry_id)
{
   Reference *ref;
   // increase reference for this file
   ref = malloc(sizeof(*ref));
   ref->client = client;
   ref->entry = entry;
   ref->client_entry_id = client_entry_id;
   ref->count = 1;
   entry->references = eina_list_append(entry->references, ref);
   return ref;
}

Предупреждение PVS-Studio: V522 Возможно, произошло разыменование потенциального нулевого указателя ref. evas_cserve2_cache.c 1404

Одна и та же ситуация повторилась 563 раза. Я не могу привести их все в статье. Вот ссылка на файл: EFL_V522.txt.

V547 (39 ошибок)

static void
_ecore_con_url_dialer_error(void *data, const Efl_Event *event)
{
   Ecore_Con_Url *url_con = data;
   Eina_Error *perr = event->info;
   int status;
   status = 
     efl_net_dialer_http_response_status_get(url_con->dialer);
   if ((status < 500) && (status > 599))
   {
      DBG("HTTP error %d reset to 1", status);
      status = 1; /* not a real HTTP error */
   }
 
   WRN("HTTP dialer error url='%s': %s",
       efl_net_dialer_address_dial_get(url_con->dialer),
       eina_error_msg_get(*perr));
   _ecore_con_event_url_complete_add(url_con, status);
}

Предупреждение PVS-Studio: V547 Выражение ‘(status ‹ 500) && (status › 599)’ всегда ложно. ecore_con_url.c 351

Правильный вариант проверки должен быть таким:

if ((status < 500) || (status > 599))

Фрагмент кода с этой ошибкой был скопирован в два других фрагмента:

  • V547 Выражение ‘(статус ‹ 500) && (статус › 599)’ всегда ложно. ecore_con_url.c 658
  • V547 Выражение ‘(статус ‹ 500) && (статус › 599)’ всегда ложно. ecore_con_url.c 1340

Еще одна ошибочная ситуация:

EAPI void
eina_rectangle_pool_release(Eina_Rectangle *rect)
{
  Eina_Rectangle *match;  
  Eina_Rectangle_Alloc *new;
  ....
  match = (Eina_Rectangle *) (new + 1);
  if (match)
    era->pool->empty = _eina_rectangle_skyline_list_update(
                          era->pool->empty, match);
  ....
}

Предупреждение PVS-Studio: V547 Выражение match всегда истинно. eina_rectangle.c 798

После того, как указатель был добавлен 1, нет смысла проверять его на NULL.

Указатель match может стать равным нулю, только если при добавлении происходит переполнение. Однако переполнение указателя считается поведением undefined, поэтому этот вариант не следует рассматривать.

И еще один случай.

EAPI const void *
evas_object_smart_interface_get(const Evas_Object *eo_obj,
                                const char *name)
{
  Evas_Smart *s;
  ....
  s = evas_object_smart_smart_get(eo_obj);
  if (!s) return NULL;
  if (s)
  ....
}

Предупреждение PVS-Studio: V547 Выражение ‘s’ всегда истинно. evas_object_smart.c 160

Если указатель равен NULL, то есть выход из функции. Повторная проверка не имеет смысла.

Другие ошибки: EFL_V547.txt.

Есть предупреждения V547, которые я пропустил и не записал, так как не очень интересно было в них разбираться. Среди них может быть еще несколько ошибок.

V556 (8 ошибок)

Все они выдаются за один фрагмент кода. Сначала давайте взглянем на объявление двух перечислений.

typedef enum _Elm_Image_Orient_Type
{
  ELM_IMAGE_ORIENT_NONE = 0,
  ELM_IMAGE_ORIENT_0 = 0,
  ELM_IMAGE_ROTATE_90 = 1,
  ELM_IMAGE_ORIENT_90 = 1,
  ELM_IMAGE_ROTATE_180 = 2,
  ELM_IMAGE_ORIENT_180 = 2,
  ELM_IMAGE_ROTATE_270 = 3,
  ELM_IMAGE_ORIENT_270 = 3,
  ELM_IMAGE_FLIP_HORIZONTAL = 4,
  ELM_IMAGE_FLIP_VERTICAL = 5,
  ELM_IMAGE_FLIP_TRANSPOSE = 6,
  ELM_IMAGE_FLIP_TRANSVERSE = 7
} Elm_Image_Orient;
typedef enum
{
  EVAS_IMAGE_ORIENT_NONE = 0,
  EVAS_IMAGE_ORIENT_0 = 0,
  EVAS_IMAGE_ORIENT_90 = 1,
  EVAS_IMAGE_ORIENT_180 = 2,
  EVAS_IMAGE_ORIENT_270 = 3,
  EVAS_IMAGE_FLIP_HORIZONTAL = 4,
  EVAS_IMAGE_FLIP_VERTICAL = 5,
  EVAS_IMAGE_FLIP_TRANSPOSE = 6,
  EVAS_IMAGE_FLIP_TRANSVERSE = 7
} Evas_Image_Orient;

Как видите, имена этих констант в перечислениях похожи. Это то, что терпело неудачу для программиста.

EAPI void
elm_image_orient_set(Evas_Object *obj, Elm_Image_Orient orient)
{
  Efl_Orient dir;
  Efl_Flip flip;
  EFL_UI_IMAGE_DATA_GET(obj, sd);
  sd->image_orient = orient;
  switch (orient)
  {
    case EVAS_IMAGE_ORIENT_0:
    ....
    case EVAS_IMAGE_ORIENT_90:
    ....
    case EVAS_IMAGE_FLIP_HORIZONTAL:
    ....
    case EVAS_IMAGE_FLIP_VERTICAL:
    ....
}

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

  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2141
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2145
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2149
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2153
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2157
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2161
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2165
  • V556 Сравниваются значения различных типов перечисления: switch(ENUM_TYPE_A) { case ENUM_TYPE_B: … }. efl_ui_image.c 2169

Экземпляры из разных перечислений сравниваются восемь раз.

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

  • ELM_IMAGE_ORIENT_NONE = 0 ; EVAS_IMAGE_ORIENT_NONE = 0,
  • ELM_IMAGE_ORIENT_0 = 0 ; EVAS_IMAGE_ORIENT_0 = 0
  • ELM_IMAGE_ROTATE_90 = 1 ; EVAS_IMAGE_ORIENT_90 = 1
  • и так далее.

Функция будет работать корректно, но все равно это ошибки.

Комментарий Карстена Хайтцлера. Все вышеперечисленное с ориентацией/поворотом перечисления сделано намеренно. Нам пришлось устранить дублирование перечислений, и мы обеспечили их одинаковые значения, чтобы они были взаимозаменяемыми — мы перешли от вращения к ориентации и сохранили совместимость. Это часть нашего перехода к новой объектной системе, автогенерации большого количества кода и т. д., которые все еще находятся в стадии разработки и находятся в стадии бета-тестирования. Это не ошибка, а намерение сделать это как часть перехода, поэтому это ложное срабатывание.

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

V558 (4 ошибки)

accessor_iterator<T>& operator++(int)
{
  accessor_iterator<T> tmp(*this);
  ++*this;
  return tmp;
}

Предупреждение PVS-Studio: Функция V558 возвращает ссылку на временный локальный объект: tmp. eina_accessor.hh 519

Чтобы исправить код, вы должны удалить & из объявления функции:

accessor_iterator<T> operator++(int)

Другие ошибки:

  • Функция V558 возвращает ссылку на временный локальный объект: tmp. eina_accessor.hh 535
  • Функция V558 возвращает ссылку на временный локальный объект: tmp. eina_accessor.hh 678
  • Функция V558 возвращает ссылку на временный локальный объект: tmp. eina_accessor.hh 694

V560 (32 ошибки)

static unsigned int read_compressed_channel(....)
{
  ....
  signed char headbyte;
  ....
  if (headbyte >= 0)
  {
    ....
  }
  else if (headbyte >= -127 && headbyte <= -1)     // <=
  ....
}

Предупреждение PVS-Studio: V560 Часть условного выражения всегда истинна: ​​headbyte ‹= — 1. evas_image_load_psd.c 221

Если переменная headbyte была ›= 0, то нет смысла в проверке ‹= -1.

Давайте рассмотрим другой случай.

static Eeze_Disk_Type
_eeze_disk_type_find(Eeze_Disk *disk)
{
  const char *test;
  ....
  test = udev_device_get_property_value(disk->device, "ID_BUS");
  if (test)
  {
    if (!strcmp(test, "ata")) return EEZE_DISK_TYPE_INTERNAL;
    if (!strcmp(test, "usb")) return EEZE_DISK_TYPE_USB;
    return EEZE_DISK_TYPE_UNKNOWN;
  }
  if ((!test) && (!filesystem))                     // <=
  ....
}

Предупреждение PVS-Studio: V560 Часть условного выражения всегда истинна: ​​(!test). eeze_disk.c 55

Второе условие избыточно. Если бы указатель test был ненулевым, функция завершилась бы.

Другие ошибки: EFL_V560.txt.

V568 (3 ошибки)

EOLIAN static Eina_Error
_efl_net_server_tcp_efl_net_server_fd_socket_activate(....)
{
  ....
  struct sockaddr_storage *addr;
  socklen_t addrlen;
  ....
  addrlen = sizeof(addr);
  if (getsockname(fd, (struct sockaddr *)&addr, &addrlen) != 0)
  ....
}

Предупреждение PVS-Studio: V568 Странно, что оператор sizeof() оценивает размер указателя на класс, а не размер объекта класса addr. efl_net_server_tcp.c 192

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

addrlen = sizeof(*addr);

Другие ошибки:

  • V568 Странно, что оператор sizeof() оценивает размер указателя на класс, а не размер объекта класса addr. efl_net_server_udp.c 228
  • V568 Странно, что оператор sizeof() оценивает размер указателя на класс, а не размер объекта класса addr. efl_net_server_unix.c 198

V571 (6 ошибок)

EAPI void eeze_disk_scan(Eeze_Disk *disk)
{
  ....
  if (!disk->cache.vendor)
    if (!disk->cache.vendor)
      disk->cache.vendor = udev_device_get_sysattr_value(....);
  ....
}

Предупреждение PVS-Studio: V571 Периодическая проверка. Условие if (!disk-›cache.vendor) уже проверено в строке 298. eeze_disk.c 299

Излишняя или неправильная проверка.

Другие ошибки:

  • V571 Периодическая проверка. Условие «if (!disk-›cache.model)» уже проверено в строке 302. eeze_disk.c 303
  • V571 Периодическая проверка. Условие if (priv-›last_buffer) уже проверено в строке 150.motion_sink.c 152
  • V571 Периодическая проверка. Условие if (pd-›editable) уже проверено в строке 892. elm_code_widget.c 894
  • V571 Периодическая проверка. Условие «if (mnh ›= 0)» уже проверено в строке 279. els_box.c 281
  • V571 Периодическая проверка. Условие «if (mnw ›= 0)» уже проверено в строке 285. els_box.c 287

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

V575 (126 ошибок)

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

static void
free_buf(Eina_Evlog_Buf *b)
{
   if (!b->buf) return;
   b->size = 0;
   b->top = 0;
# ifdef HAVE_MMAP
   munmap(b->buf, b->size);
# else
   free(b->buf);
# endif
   b->buf = NULL;
}

Предупреждение PVS-Studio: V575 Функция munmap обрабатывает 0 элементов. Проверьте второй аргумент. eina_evlog.c 117

Сначала в переменную b-›size был записан 0, затем он был передан в функцию munmap.

Мне кажется, что это должно быть написано по-другому:

static void
free_buf(Eina_Evlog_Buf *b)
{
   if (!b->buf) return;
   b->top = 0;
# ifdef HAVE_MMAP
   munmap(b->buf, b->size);
# else
   free(b->buf);
# endif
   b->buf = NULL;
   b->size = 0;
}

Давай продолжим.

EAPI Eina_Bool
eina_simple_xml_parse(....)
{
  ....
  else if ((itr + sizeof("<!>") - 1 < itr_end) &&
           (!memcmp(itr + 2, "", sizeof("") - 1)))
  ....
}

Предупреждение PVS-Studio: V575 Функция memcmp обрабатывает нулевые элементы. Проверьте третий аргумент. eina_simple_xml_parser.c 355

Непонятно, почему программист сравнивает 0 байт памяти.

Давай продолжим.

static void
_edje_key_down_cb(....)
{
  ....
  char *compres = NULL, *string = (char *)ev->string;
  ....
  if (compres)
  {
    string = compres;
    free_string = EINA_TRUE;
  }
  else free(compres);
  ....
}

Предупреждение PVS-Studio: V575 Нулевой указатель передается в «свободную» функцию. Проверьте первый аргумент. edje_entry.c 2306

Если указатель compress равен нулю, освобождать память не нужно. Линия

else free(compres);

можно удалить.

Комментарий Карстена Хейтцлера. Не ошибка, а действительно какой-то дополнительный код, похожий на паранойю, который не нужен. Опять микрооптимизация?

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

Так же:

  • V575 Нулевой указатель передается в «свободную» функцию. Проверьте первый аргумент. efl_ui_internal_text_interactive.c 1022
  • V575 Нулевой указатель передается в «свободную» функцию. Проверьте первый аргумент. edje_cc_handlers.c 15962

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

static void _fill_all_outs(char **outs, const char *val)
{
  size_t vlen = strlen(val);
  for (size_t i = 0; i < (sizeof(_dexts) / sizeof(char *)); ++i)
  {
    if (outs[i])
      continue;
    size_t dlen = strlen(_dexts[i]);
    char *str = malloc(vlen + dlen + 1);
    memcpy(str, val, vlen);
    memcpy(str + vlen, _dexts[i], dlen);
    str[vlen + dlen] = '\0';
    outs[i] = str;
  }
}

Предупреждение PVS-Studio: V575 В функцию memcpy передается потенциальный нулевой указатель. Проверьте первый аргумент. main.c 112

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

Другие ошибки: EFL_V575.txt.

V587 (2 ошибки)

void
_ecore_x_event_handle_focus_in(XEvent *xevent)
{
  ....
   e->time = _ecore_x_event_last_time;
   _ecore_x_event_last_time = e->time;
  ....
}

Предупреждение PVS-Studio: V587 Нечетная последовательность заданий типа: A = B; В = А;. Контрольные строки: 1006, 1007. ecore_x_events.c 1007

Комментарий Карстена Хейтцлера. Не баги как таковые — похоже, просто переусердствовали с хранением последней метки времени. Это добавляет временную метку к событию, когда исходной временной метки не существует, поэтому мы можем сохранить согласованную структуру для событий с временными метками, но это беспорядок в коде и микрооптимизация.

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

Еще ошибка: V587 Нечетная последовательность присвоений такого вида: A = B; В = А;. Контрольные строки: 1050, 1051. ecore_x_events.c 1051

V590 (3 ошибки)

static int command(void)
{
  ....
  while (*lptr == ' ' && *lptr != '\0')
    lptr++; /* skip whitespace */
  ....
}

Предупреждение PVS-Studio: V590 Попробуйте проверить выражение ‘* lptr == ‘ ‘ && * lptr != ‘\0’’. Выражение является избыточным или содержит опечатку. эмбрион_cc_sc2.c 944

Избыточная проверка. Можно упростить:

while (*lptr == ' ')

Еще два подобных предупреждения:

  • V590 Рассмотрите возможность проверки ‘sym-›ident == 9 || sym-›ident != 10’ выражение. Выражение является избыточным или содержит опечатку. эмбрион_cc_sc3.c 1782
  • V590 Рассмотрите возможность проверки ‘* p == ‘\n’ || * p != ‘\»’’ выражение. Выражение является избыточным или содержит опечатку. cpplib.c 4012

V591 (1 ошибка)

_self_type& operator=(_self_type const& other)
{
  _base_type::operator=(other);
}

Предупреждение PVS-Studio: V591 Непустая функция должна возвращать значение. eina_accessor.hh 330

V595 (4 ошибки)

static void
eng_image_size_get(void *engine EINA_UNUSED, void *image,
int *w, int *h)
{
   Evas_GL_Image *im;
   if (!image)
     {
        *w = 0;                                    // <=
        *h = 0;                                    // <=
        return;
     }
   im = image;
   if (im->orient == EVAS_IMAGE_ORIENT_90 ||
       im->orient == EVAS_IMAGE_ORIENT_270 ||
       im->orient == EVAS_IMAGE_FLIP_TRANSPOSE ||
       im->orient == EVAS_IMAGE_FLIP_TRANSVERSE)
     {
        if (w) *w = im->h;
        if (h) *h = im->w;
     }
   else
     {
        if (w) *w = im->w;
        if (h) *h = im->h;
     }
}

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

  • V595 Указатель «w» использовался до того, как он был проверен на соответствие nullptr. Строки проверки: 575, 585. evas_engine.c 575
  • V595 Указатель «h» использовался до того, как он был проверен на соответствие nullptr. Строки проверки: 576, 586. evas_engine.c 576

Проверки if (w) и if (h) подсказывают анализатору, что входные аргументы w и hможет быть равно NULL. Опасно, что в начале функции они используются без проверки.

Если вы вызываете функцию eng_image_size_get следующим образом:

eng_image_size_get(NULL, NULL, NULL, NULL);

он не будет к этому готов и произойдет разыменование нулевого указателя.

Предупреждения, которые, на мой взгляд, тоже указывают на ошибки:

  • V595 Указатель cur-›node использовался до того, как он был проверен на соответствие nullptr. Контрольные строки: 9889, 9894. evas_object_textblock.c 9889
  • V595 Указатель «подтип» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 2200, 2203. eet_data.c 2200

V597 (6 ошибок)

EAPI Eina_Binbuf *
emile_binbuf_decipher(Emile_Cipher_Algorithm algo,
                      const Eina_Binbuf *data,
                      const char *key,
                      unsigned int length)
{
  ....
  Eina_Binbuf *result = NULL;
  unsigned int *over;
  EVP_CIPHER_CTX *ctx = NULL;
  unsigned char ik[MAX_KEY_LEN];
  unsigned char iv[MAX_IV_LEN];
  ....
on_error:
   memset(iv, 0, sizeof (iv));
   memset(ik, 0, sizeof (ik));
   if (ctx)
     EVP_CIPHER_CTX_free(ctx);
   eina_binbuf_free(result);
   return NULL;
}

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

  • V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «iv». Функцию memset_s() следует использовать для удаления личных данных. emile_cipher_openssl.c 293
  • V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «ik». Функцию memset_s() следует использовать для удаления личных данных. emile_cipher_openssl.c 294

Я уже много раз писал в статьях, почему компилятор может удалять вызовы memset функций в таком коде. Вот почему я не хочу повторяться. Если кто-то не знаком с этим вопросом, предлагаю прочитать описание диагностики V597.

Комментарий Карстена Хайтцлера. Выше 2 — полностью знаком с проблемой. Большая проблема заключается в том, что memset_s не является переносимым или легкодоступным, поэтому мы еще не используем его. Вы должны сделать специальные проверки, чтобы увидеть, существует ли он, поскольку он существует не везде. Просто в качестве простого примера добавьте AC_CHECK_FUNCS([memset_s]) в ваш configure.ac, и memset_s не будет найден, вам нужно пройти еще несколько обручей, например, определить __STDC_WANT_LIB_EXT1__ 1 перед включением системных заголовков… и он все еще не объявлен. В моей довольно современной системе Arch memset_s не определяется никакими системными заголовками, то же самое при тестировании Debian… предупреждение: неявное объявление функции «memset_s»; Вы имели в виду мемсет? [-Wimplicit-function-declaration], а затем ошибка компиляции… что бы я ни делал. Команда grep -r всех включенных в мою систему систем не показывает, что memset_s объявлены… поэтому я думаю, что советовать людям использовать memset_s — это только жизнеспособный совет, если он широко доступен и применим. Помните об этом.

Другие ошибки:

  • V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «key_material». Функцию memset_s() следует использовать для удаления личных данных. emile_cipher_openssl.c 144
  • V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «iv». Функцию memset_s() следует использовать для удаления личных данных. emile_cipher_openssl.c 193
  • V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «ik». Функцию memset_s() следует использовать для удаления личных данных. emile_cipher_openssl.c 194
  • V597 Компилятор может удалить вызов функции «memset», который используется для очистки буфера «key_material». Функцию memset_s() следует использовать для удаления личных данных. emile_cipher_openssl.c 249

V609 (1 ошибка)

Прежде всего, давайте взглянем на функцию eina_value_util_type_size:

static inline size_t
eina_value_util_type_size(const Eina_Value_Type *type)
{
   if (type == EINA_VALUE_TYPE_INT)
     return sizeof(int32_t);
   if (type == EINA_VALUE_TYPE_UCHAR)
     return sizeof(unsigned char);
   if ((type == EINA_VALUE_TYPE_STRING) ||
       (type == EINA_VALUE_TYPE_STRINGSHARE))
     return sizeof(char*);
   if (type == EINA_VALUE_TYPE_TIMESTAMP)
     return sizeof(time_t);
   if (type == EINA_VALUE_TYPE_ARRAY)
     return sizeof(Eina_Value_Array);
   if (type == EINA_VALUE_TYPE_DOUBLE)
     return sizeof(double);
   if (type == EINA_VALUE_TYPE_STRUCT)
     return sizeof(Eina_Value_Struct);
   return 0;
}

Обратите внимание, что функция может возвращать 0. Теперь посмотрим, как эта функция используется.

static inline unsigned int
eina_value_util_type_offset(const Eina_Value_Type *type,
                            unsigned int base)
{
   unsigned size, padding;
   size = eina_value_util_type_size(type);
   if (!(base % size))
     return base;
   padding = ( (base > size) ? (base - size) : (size - base));
   return base + padding;
}

Предупреждение PVS-Studio: V609 Mod by zero. Диапазон знаменателя [0..24]. eina_inline_value_util.x 60

Возможное деление на ноль. Я не уверен, может ли быть реальная ситуация, когда eina_value_util_type_sizefunctionвозвращает0. В любом случае код опасен.

Комментарий Карстена Хайтцлера. Возврат 0 произойдет только в том случае, если вы введете совершенно неверный ввод, например снова strdup(NULL)… Поэтому я называю это ложным срабатыванием, поскольку у вас не может быть универсального значения eina_value, которое недействительно без каких-либо плохих вещей — проверьте сначала вы передаете правильное значение. Кстати, eina_value чувствительна к производительности, поэтому каждая проверка здесь чего-то стоит. это похоже на добавление проверок if() к коду операции добавления.

V610 (1 ошибка)

void fetch_linear_gradient(....)
{
  ....
  if (t + inc*length < (float)(INT_MAX >> (FIXPT_BITS + 1)) &&
      t+inc*length > (float)(INT_MIN >> (FIXPT_BITS + 1)))
  ....
}

Предупреждение PVS-Studio: V610 Неопределенное поведение. Проверьте оператора смены ››. Левый операнд ‘(- 0x7ffffffff — 1)’ отрицательный. ector_software_gradient.c 412

V614 (1 ошибка)

extern struct tm *gmtime (const time_t *__timer)
  __attribute__ ((__nothrow__ , __leaf__));
static void
_set_headers(Evas_Object *obj)
{
  static char part[] = "ch_0.text";
  int i;
  struct tm *t;
  time_t temp;
  ELM_CALENDAR_DATA_GET(obj, sd);
  elm_layout_freeze(obj);
  sd->filling = EINA_TRUE;
  t = gmtime(&temp);            // <=
  ....
}

Предупреждение PVS-Studio: V614 Используется неинициализированная переменная temp. Рассмотрите возможность проверки первого фактического аргумента функции gmtime. elm_calendar.c 720

V621 (1 ошибка)

static void
_opcodes_unregister_all(Eina_Debug_Session *session)
{
  Eina_List *l;
  int i;
  _opcode_reply_info *info = NULL;
  if (!session) return;
  session->cbs_length = 0;
  for (i = 0; i < session->cbs_length; i++)
    eina_list_free(session->cbs[i]);
  ....
}

Предупреждение PVS-Studio: V621 Подумайте о проверке оператора for. Возможно, что цикл будет выполняться некорректно или вообще не будет выполняться. eina_debug.c 405

V630 (2 ошибки)

Существует обычный класс btVector3 с конструктором. Однако этот конструктор ничего не делает.

class btVector3
{
public:
  ....
  btScalar m_floats[4];
  inline btVector3() { }
  ....
};

Также есть такая структура Simulation_Msg:

typedef struct _Simulation_Msg Simulation_Msg;
struct _Simulation_Msg {
     EPhysics_Body *body_0;
     EPhysics_Body *body_1;
     btVector3 pos_a;
     btVector3 pos_b;
     Eina_Bool tick:1;
};

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

_ephysics_world_tick_dispatch(EPhysics_World *world)
{
   Simulation_Msg *msg;
   if (!world->ticked)
     return;
   world->ticked = EINA_FALSE;
   world->pending_ticks++;
   msg = (Simulation_Msg *) calloc(1, sizeof(Simulation_Msg));
   msg->tick = EINA_TRUE;
   ecore_thread_feedback(world->cur_th, msg);
}

Предупреждение PVS-Studio: V630 Функция calloc используется для выделения памяти под массив объектов, являющихся классами, содержащими конструкторы. ephysics_world.cpp 299

Структура, содержащая не-члены POD, создается с помощью вызова функции calloc.

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

Другая ошибка: V630 Функция calloc используется для выделения памяти для массива объектов, которые являются классами, содержащими конструкторы. ephysics_world.cpp 471

Комментарий Карстена Хайтцлера. Поскольку на другом конце конвейера находится код C, который передает необработанный ptr как результат от потока A к потоку B, это смешанная среда C и C++. В конце концов, мы будем рассылать необработанные PTR, несмотря ни на что…

V654 (2 ошибки)

int
evas_mem_free(int mem_required EINA_UNUSED)
{
   return 0;
}
int
evas_mem_degrade(int mem_required EINA_UNUSED)
{
   return 0;
}
void * 
evas_mem_calloc(int size)
{
   void *ptr;
   ptr = calloc(1, size);
   if (ptr) return ptr;
   MERR_BAD();
   while ((!ptr) && (evas_mem_free(size))) ptr = calloc(1, size);
   if (ptr) return ptr;
   while ((!ptr) && (evas_mem_degrade(size))) ptr = calloc(1, size);
   if (ptr) return ptr;
   MERR_FATAL();
   return NULL;
}

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

  • V654 Условие цикла ‘(!ptr) && (evas_mem_free(size))’ всегда ложно. main.c 44
  • V654 Условие цикла ‘(!ptr) && (evas_mem_degrade(size))’ всегда ложно. main.c 46

Очевидно, это неполный код.

Комментарий Карстена Хайтцлера. Старый старый код, потому что было реализовано кеширование, поэтому в основном было много NOP, ожидающих заполнения. поскольку evas спекулятивно кэшировал данные (их мегабайты), идея заключалась в том, что в случае сбоя allocs — освободить немного кеша и попробовать еще раз… если это не удается, попробуйте уничтожить некоторые некэшированные данные, которые можно перезагрузить/перестроить, но с большими затратами… и только после этого потерпеть неудачу. Но из-за чрезмерной фиксации это не оказалось практичным, так как allocs были успешными, а затем достаточно часто падали, если вы сталкивались с действительно нехваткой памяти, поэтому я сдался. это не ошибка. это часть истории :).

Следующий случай более интересен и странен.

EAPI void evas_common_font_query_size(....)
{
  ....
  size_t cluster = 0;
  size_t cur_cluster = 0;
  ....
  do
  {
    cur_cluster = cluster + 1;
    
    glyph--;
    if (cur_w > ret_w)
    {
      ret_w = cur_w;
    }
  }
  while ((glyph > first_glyph) && (cur_cluster == cluster));
  ....
}

Предупреждение PVS-Studio: V654 Условие цикла всегда ложно. evas_font_query.c 376

Это присваивание выполняется в цикле:

cur_cluster = cluster + 1;

Это означает, что сравнение (cur_cluster == cluster) всегда оценивается как false.

Комментарий Карстена Хайтцлера. Выше… похоже, вы построили без поддержки harfbuzz… мы настоятельно не рекомендуем это делать. это не проверено. Сборка практически без ядерной бомбы уничтожает почти всю интересную поддержку unicode/intl для текстового макета. Вы должны явно отключить его … потому что с поддержкой harfbuzz у нас включен opentype, и другой фрагмент кода выполняется из-за ifdefs.. если вы действительно проверяете историю кода перед добавлением поддержки opentype, он не зацикливается на кластерах в все или даже глифы .. так что на самом деле ifdef просто гарантирует, что цикл повторяет только один цикл и избегает других ifdef позже в условиях цикла, что упрощает поддержку кода - остерегайтесь ifdefs!

V668 (21 ошибка)

Как мы выяснили ранее, существуют сотни фрагментов кода, где не выполняется проверка указателя после выделения памяти с помощью функции malloc/ calloc.

На этом фоне проверки после использования оператора new кажутся шуткой.

Есть несколько безобидных ошибок:

static EPhysics_Body *
_ephysics_body_rigid_body_add(....)
{
  ....
  motion_state = new btDefaultMotionState();
  if (!motion_state)
  {
    ERR("Couldn't create a motion state.");
    goto err_motion_state;
  }
  ....
}

Предупреждение PVS-Studio: V668 Проверять указатель motion_state на null нет смысла, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти. ephysics_body.cpp 837

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

Комментарий Карстена Хейтцлера. Достаточно справедливо, но имейте в виду, что некоторые компиляторы НЕ генерируют исключения… они возвращают NULL при новом… так что не совсем бесполезный код зависит от компилятора. Я полагаю, что VSC6 не выдавал исключения — так что до появления исключений это действительно было правильным поведением, также я зависел от функции распределителя, выбрасывает ли он исключение или нет, так что в целом очень незначительный безвредный код.

Комментарий Андрея Карпова. Я не согласен. Смотрите следующий комментарий.

Есть и более серьезные ошибки:

EAPI EPhysics_Constraint *
ephysics_constraint_linked_add(EPhysics_Body *body1,
                               EPhysics_Body *body2)
{
  ....
  constraint->bt_constraint = new btGeneric6DofConstraint(
     *ephysics_body_rigid_body_get(body1),
     *ephysics_body_rigid_body_get(body2),
     btTransform(), btTransform(), false);
  if (!constraint->bt_constraint)
  {
    ephysics_world_lock_release(constraint->world);
    free(constraint);
    return NULL;
  }
  ....
}

Предупреждение PVS-Studio: V668 Нет смысла тестировать указатель constraint-›bt_constraint на null, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти. ephysics_constraints.cpp 382

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

Комментарий Карстена Хейтцлера. То же, что и предыдущий новый + проверка NULL.

Комментарий Андрея Карпова. Мне непонятно, почему здесь говорится о Visual C++ 6.0. Да не через исключение при использовании нового оператора, как и у других старых компиляторов. Да, если использовать старый компилятор, программа будет работать корректно. Но Tizen никогда не будет компилироваться с помощью Visual C++ 6.0! Нет смысла думать об этом. Новый компилятор будет через исключение, и это приведет к ошибкам. Подчеркну еще раз, что это не дополнительная проверка. Программа работает не так, как ожидает программист. Более того, во втором примере происходит утечка памяти. Если рассматривать случай, когда new не проходит через исключение, следует использовать new(nothrow). Тогда анализатор не будет жаловаться. В любом случае этот код неверен.

Другие ошибки: EFL_V668.txt.

V674 (2 ошибки)

Во-первых, давайте посмотрим, как объявляется функция abs:

extern int abs (int __x) __attribute__ ((__nothrow__ , __leaf__))
                         __attribute__ ((__const__)) ;

Как видите, он содержит и возвращает значения int.

Теперь давайте посмотрим, как эта функция используется.

#define ELM_GESTURE_MINIMUM_MOMENTUM 0.001
typedef int Evas_Coord;
struct _Elm_Gesture_Momentum_Info
{
  ....
  Evas_Coord mx;
  Evas_Coord my;
  ....
};
static void
_momentum_test(....)
{
  ....
  if ((abs(st->info.mx) > ELM_GESTURE_MINIMUM_MOMENTUM) ||
      (abs(st->info.my) > ELM_GESTURE_MINIMUM_MOMENTUM))
    state_to_report = ELM_GESTURE_STATE_END;
  ....
}

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

  • V674 Литерал 0.001 типа double сравнивается со значением типа int. Рассмотрите возможность проверки выражения «abs(st-›info.mx) › 0,001». elm_gesture_layer.c 2533
  • V674 Литерал 0.001 типа double сравнивается со значением типа int. Попробуйте проверить выражение «abs(st-›info.my) › 0,001». elm_gesture_layer.c 2534

Странно сравнивать значения int с константой 0,001. Здесь определенно есть ошибка.

V686 (3 ошибки)

static Image_Entry *
_scaled_image_find(Image_Entry *im, ....)
{
   size_t               pathlen, keylen, size;
   char                 *hkey;
   Evas_Image_Load_Opts  lo;
   Image_Entry          *ret;
   if (((!im->file) || ((!im->file) && (!im->key))) || (!im->data1) ||
       ((src_w == dst_w) && (src_h == dst_h)) ||
       ((!im->flags.alpha) && (!smooth))) return NULL;
  ....
}

Предупреждение PVS-Studio: V686 Обнаружен паттерн: (!im-›file) || ((!im-›файл) && …). Выражение является избыточным или содержит логическую ошибку. evas_cache2.c 825

Скорее всего это не настоящая ошибка, а избыточный код. Позвольте мне объяснить это на простом примере.

если (А || (А && В) || С)

Выражение можно упростить до:

if (A || C)

Хотя, возможно, в выражении опечатка и не проверено что-то важное. В данном случае это ошибка. Мне трудно судить об этом незнакомом коде.

Аналогичные избыточные условия:

  • V686 Обнаружен шаблон: (!im-›file) || ((!im-›файл) && …). Выражение является избыточным или содержит логическую ошибку. evas_cache2.c 905
  • V686 Обнаружен шаблон: (nextc == ‘*’) || ((nextc == ‘*’) && …). Выражение является избыточным или содержит логическую ошибку. cpplib.c 1022

V694 (2 ошибки)

#define CPP_PREV_BUFFER(BUFFER) ((BUFFER)+1)
static void
initialize_builtins(cpp_reader * pfile)
{
  ....
  cpp_buffer *pbuffer = CPP_BUFFER(pfile);
  while (CPP_PREV_BUFFER(pbuffer))
    pbuffer = CPP_PREV_BUFFER(pbuffer);
  ....
}

Предупреждение PVS-Studio: V694 Условие ((pbuffer) + 1) ложно только в случае переполнения указателя, что в любом случае является неопределённым поведением. cpplib.c 2496

Я расширю макрос, чтобы было понятнее.

cpp_buffer *pbuffer = ....;
while (pbuffer + 1)
  ....

Условие цикла всегда истинно. Формально он может стать ложным в случае, если указатель равен его пределу. Кроме того, происходит переполнение. Это считается неопределённым поведением, значит, компилятор имеет право не учитывать такую ​​ситуацию. Компилятор может удалить условную проверку, и тогда мы получим:

while (true)
  pbuffer = CPP_PREV_BUFFER(pbuffer);

Это очень интересный баг.

Еще одна неверная проверка: V694 Условие ((ip) + 1) ложно только в том случае, если есть переполнение указателя, что в любом случае является неопределенным поведением. cpplib.c 2332

Комментарий Карстена Хайтцлера. В этом старом коде действительно есть проблемы. Должны быть проверки против CPP_NULL_BUFFER(pfile), потому что, если это связанный список, это нулевой чек, если это статический буферный массив в виде стека, он проверяет конечную позицию стека - интересно, за десятилетия он никогда не срабатывал, насколько я знаю.

V701 (69 ошибок)

static void
_efl_vg_gradient_efl_gfx_gradient_stop_set(
                          ...., Efl_VG_Gradient_Data *pd, ....)
{
   pd->colors = realloc(pd->colors,
                        length * sizeof(Efl_Gfx_Gradient_Stop));
   if (!pd->colors)
     {
        pd->colors_count = 0;
        return ;
     }
   memcpy(pd->colors, colors,
          length * sizeof(Efl_Gfx_Gradient_Stop));
   pd->colors_count = length;
   _efl_vg_changed(obj);
}

Предупреждение PVS-Studio: V701 возможна утечка realloc(): когда realloc() не удается выделить память, теряется исходный указатель ‘pd-›colors’. Рассмотрите возможность назначения realloc() временному указателю. evas_vg_gradient.c 14

Эта строка содержит ошибку:

pd->colors = realloc(pd->colors, ....);

Старое значение указателя pd-›colors нигде не сохраняется. Утечка памяти произойдет, если невозможно выделить новый блок памяти. Адрес предыдущего буфера будет потерян, так как в pd-›colors будет записано значение NULL.

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

EOLIAN void _evas_canvas_key_lock_add(
  Eo *eo_e, Evas_Public_Data *e, const char *keyname)
{
   if (!keyname) return;
   if (e->locks.lock.count >= 64) return;
   evas_key_lock_del(eo_e, keyname);
   e->locks.lock.count++;
   e->locks.lock.list =
     realloc(e->locks.lock.list,
             e->locks.lock.count * sizeof(char *));
   e->locks.lock.list[e->locks.lock.count - 1] = strdup(keyname);
   eina_hash_free_buckets(e->locks.masks);
}

Предупреждение PVS-Studio: возможная утечка V701 realloc(): когда realloc() не удается выделить память, исходный указатель e-›locks.lock.list теряется. Рассмотрите возможность назначения realloc() временному указателю. evas_key.c 142

Другие ошибки: EFL_701.txt.

V728 (4 ошибки)

static Eina_Bool
_evas_textblock_node_text_adjust_offsets_to_start(....)
{
  Evas_Object_Textblock_Node_Format *last_node, *itr;
  ....
  if (!itr || (itr && (itr->text_node != n)))
  ....
}

Предупреждение PVS-Studio: V728 Излишнюю проверку можно упростить. Оператор || окружен противоположными выражениями !itr и itr. evas_object_textblock.c 9505

Это не ошибка, а излишне сложное условие. Это выражение можно упростить:

if (!itr || (itr->text_node != n))

Другие избыточные условия:

  • V728 Излишняя проверка может быть упрощена. Оператор «||» окружен противоположными выражениями «!p» и «p». elm_theme.c 447
  • V728 Излишняя проверка может быть упрощена. Оператор «||» окружен противоположными выражениями «!ss» и «ss». config.c 3932
  • V728 Излишняя проверка может быть упрощена. Оператор «||» окружен противоположными выражениями «!icon_version» и «icon_version». efreet_icon_cache_create.c 917

V769 (11 ошибок)

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

EAPI Eina_Bool
edje_edit_sound_sample_add(
  Evas_Object *obj, const char *name, const char *snd_src)
{
   ....
   ed->file->sound_dir->samples =
     realloc(ed->file->sound_dir->samples,
             sizeof(Edje_Sound_Sample) *
             ed->file->sound_dir->samples_count);
   sound_sample = ed->file->sound_dir->samples +
     ed->file->sound_dir->samples_count - 1;
   sound_sample->name = (char *)eina_stringshare_add(name);
   ....
}

Предупреждение PVS-Studio: V769 Указатель ed-›file-›sound_dir-›samples в выражении может быть nullptr. В таком случае результирующее значение арифметических операций над этим указателем будет бессмысленным и его не следует использовать. edje_edit.c 1271

Вызывается функция выделения памяти. Результирующий указатель не разыменовывается, а используется в выражении. К указателю добавляется значение, поэтому говорить, что используется нулевой указатель, становится некорректным. Указатель в любом случае не нулевой, но он может быть недействительным, если память не была выделена. Именно этот код показывает рассматриваемая диагностика.

Кстати, такие указатели более опасны, чем нулевые. Операционная система обязательно обнаружит разыменование нулевого указателя. С помощью указателя (NULL + N) можно попасть в область памяти, которая может быть изменена и хранит некоторые данные.

Другие ошибки:

  • V769 Указатель «new_txt» в выражении «new_txt + outlen» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. eina_str.c 539
  • V769 Указатель «new_txt» в выражении «new_txt + outlen» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. eina_str.c 611
  • V769 Указатель «tmp» в выражении «tmp ++» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. evas_object_textblock.c 11131
  • V769 Указатель dst в выражении dst += sizeof (int) может иметь значение nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. evas_font_compress.c 218
  • V769 Указатель «контент» в выражении «контент + позиция» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. elm_code_line.c 78
  • V769 Указатель «newtext» в выражении «newtext + length1» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. elm_code_line.c 102
  • V769 Указатель tmp в выражении tmp + dirlen может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. elm_code_file.c 101
  • V769 Указатель «ptr» в выражении «ptr += strlen(first) + newline_len» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. elm_code_widget_text.c 72
  • V769 Указатель «content» в выражении «content + 319» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. test_store.c 198
  • V769 Указатель «pos» в выражении «pos += sizeof (msg)» может быть nullptr. В таком случае полученное значение будет бессмысленным и его не следует использовать. evas_cserve2_cache.c 2534

V779 (19 ошибок)

Иногда диагностика V779 указывает на безобидный, но неверно написанный код. Например:

EAPI Eina_Bool
ecore_x_xinerama_screen_geometry_get(int screen,
                                     int *x, int *y,
                                     int *w, int *h)
{
  LOGFN(__FILE__, __LINE__, __FUNCTION__);
#ifdef ECORE_XINERAMA
  if (_xin_info)
  {
    int i;
    for (i = 0; i < _xin_scr_num; i++)
    {
      if (_xin_info[i].screen_number == screen)
      {
        if (x) *x = _xin_info[i].x_org;
        if (y) *y = _xin_info[i].y_org;
        if (w) *w = _xin_info[i].width;
        if (h) *h = _xin_info[i].height;
        return EINA_TRUE;
      }
    }
  }
#endif /* ifdef ECORE_XINERAMA */
  if (x) *x = 0; 
  if (y) *y = 0;
  if (w) *w = DisplayWidth(_ecore_x_disp, 0);
  if (h) *h = DisplayHeight(_ecore_x_disp, 0);
  return EINA_FALSE;
  screen = 0;                          // <=
}

Предупреждение PVS-Studio: Обнаружен недостижимый код V779. Возможно, что ошибка присутствует. ecore_x_xinerama.c 92

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

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

Теперь рассмотрим более сложный случай.

extern void _exit (int __status) __attribute__ ((__noreturn__));
static void _timeout(int val)
{
  _exit(-1);
  if (val) return;
}

Предупреждение PVS-Studio: Обнаружен недостижимый код V779. Возможно, что ошибка присутствует. тайм-аут.с 30

Функция _exit не возвращает элемент управления. Этот код невероятно странный. Мне кажется, функцию нужно писать так:

static void _timeout(int val)
{
  if (val) return;
  _exit(-1);
}

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

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

Другие ошибки: EFL_V779.txt.

V1001 (6 ошибок)

static Elocation_Address *address = NULL;
EAPI Eina_Bool
elocation_address_get(Elocation_Address *address_shadow)
{
   if (!address) return EINA_FALSE;
   if (address == address_shadow) return EINA_TRUE;
   address_shadow = address;
   return EINA_TRUE;
}

Предупреждение PVS-Studio: V1001 Переменная address_shadow присваивается, но не используется до конца функции. 1122

Это очень странный код. Мне кажется, что вообще-то тут надо что-то написать:

*address_shadow = *address;

Другие ошибки:

  • V1001 Переменная «экран» назначена, но не используется до конца функции. ecore_x_xinerama.c 92
  • V1001 Переменная «ret» назначена, но не используется до конца функции. edje_edit.c 12774
  • V1001 Переменная «ret» назначена, но не используется до конца функции. edje_edit.c 15884
  • V1001 Переменная position_shadow назначена, но не используется до конца функции. 1133
  • V1001 Переменная «status_shadow» назначена, но не используется до конца функции. 1144

Комментарий Карстена Хайтцлера

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

Вывод

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

Среди описанных в статье ошибок нет ничего эпического, но это и не удивительно. Как я уже говорил, проект EFL регулярно проверяется с помощью Coverity. Несмотря на это, PVS-Studio Analyzer все же смог выявить множество ошибок. Думаю, PVS-Studio показал бы себя лучше, если бы можно было вернуться в прошлое и поменять местами анализаторы :). То есть, если бы сначала использовали PVS-Studio, а потом Coverity, PVS-Studio показал бы себя ярче.

Я предлагаю скачать и попробовать PVS-Studio для проверки собственных проектов.

Спасибо за внимание и желаю меньше ошибок в вашем коде.