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

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

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

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

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

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

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


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

>
>
>
Asterisk: PVS-Studio заинтересовался те…

Asterisk: PVS-Studio заинтересовался телефонией

26 Авг 2014

Asterisk — свободное решение компьютерной телефонии с открытым исходным кодом от компании Digium. Приложение работает на таких операционных системах, как Linux, FreeBSD, OpenBSD, Solaris. Asterisk в комплексе с необходимым оборудованием обладает всеми возможностями классической АТС, поддерживает множество VoIP-протоколов и предоставляет богатые функции управления звонками.

В данной статье будут рассмотрены результаты проверки Asterisk, полученные с помощью PVS-Studio 5.18.

Проект, по всей видимости, проверяется анализатором Coverity, о чём свидетельствуют комментарии вида:

/* Ignore check_return warning from Coverity for ast_exists_extension below */

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

0276_asterisk_ru/image1.png

Опечатка #1

V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 2513, 2516. chan_sip.c 2516

static void sip_threadinfo_destructor(void *obj)
{
  struct sip_threadinfo *th = obj;
  struct tcptls_packet *packet;

  if (th->alert_pipe[1] > -1) {            // <=
    close(th->alert_pipe[0]);
  }
  if (th->alert_pipe[1] > -1) {
    close(th->alert_pipe[1]);
  }
  th->alert_pipe[0] = th->alert_pipe[1] = -1;
  ....
}

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

Опечатка #2

V503 This is a nonsensical comparison: pointer < 0. parking_manager.c 520

static int manager_park(....)
{
  ....
  const char *timeout = astman_get_header(m, "Timeout");
  ....
  int timeout_override = -1;
  ....
  if (sscanf(timeout, "%30d", &timeout_override) != 1 ||
    timeout < 0) {                                          // <=
      astman_send_error(s, m, "Invalid Timeout value.");
      return 0;
  }
}

В этом месте выполняется бессмысленное сравнение указателя с нулём. Скорее всего, хотели проверить переменную timeout_override, которую вернула функция sscanf.

Опечатка #3

V568 It's odd that the argument of sizeof() operator is the 'data[0] * 2' expression. channel.c 8853

static int redirecting_reason_build_data(....)
{
  ....
  if (datalen < pos + sizeof(data[0] * 2) + length) {       // <=
    ast_log(LOG_WARNING, "No space left for %s string\n", label);
    return -1;
  }
  ....
}

Оператор sizeof() вычисляет тип выражения и возвращает размер этого типа, но само выражение не вычисляется. Сложные выражения являются признаком наличия ошибки. Чаще всего эти ошибки связаны с опечатками. Как в данном примере: скорее всего, умножение на два должно быть за скобкой оператора sizeof().

Опечатка #4

V653 A suspicious string consisting of two parts is used for array initialization. It is possible that a comma is missing. Consider inspecting this literal: "KW_INCLUDES" "KW_JUMP". ael.y 736

static char *token_equivs1[] =
{
  ....
  "KW_IF",
  "KW_IGNOREPAT",
  "KW_INCLUDES"          // <=
  "KW_JUMP",
  "KW_MACRO",
  "KW_PATTERN",
  ....
};

static char *ael_token_subst(const char *mess)
{
  ....
  int token_equivs_entries = sizeof(token_equivs1)/sizeof(char*);
  ....
  for (i=0; i<token_equivs_entries; i++) {
    ....
  }
  ....
}

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

Вот как на самом деле выглядят элементы массива token_equivs1:

0276_asterisk_ru/image2.png

Ещё одно такое место:

  • V653 A suspicious string consisting of two parts is used for array initialization. It is possible that a comma is missing. Consider inspecting this literal: "includes" "jump". ael.y 776

Опечатка #5

V501 There are identical sub-expressions 'strcasecmp(item->u1.str, "endwhile") == 0' to the left and to the right of the '||' operator. pval.c 2513

void check_pval_item(pval *item, ....)
{
  ....
  if (strcasecmp(item->u1.str,"GotoIf") == 0
      || strcasecmp(item->u1.str,"GotoIfTime") == 0
      || strcasecmp(item->u1.str,"while") == 0
      || strcasecmp(item->u1.str,"endwhile") == 0           // <=
      || strcasecmp(item->u1.str,"random") == 0
      || strcasecmp(item->u1.str,"gosub") == 0
      || strcasecmp(item->u1.str,"gosubif") == 0
      || strcasecmp(item->u1.str,"continuewhile") == 0
      || strcasecmp(item->u1.str,"endwhile") == 0           // <=
      || strcasecmp(item->u1.str,"execif") == 0
      || ....)
  {....}
}

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

Идентичные сравнения

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 851, 853. manager_channels.c 851

static void channel_hangup_handler_cb(....)
{
  const char *event;
  ....
  if (!strcmp(action, "type")) {
    event = "HangupHandlerRun";
  } else if (!strcmp(action, "type")) {
    event = "HangupHandlerPop";
  } else if (!strcmp(action, "type")) {
    event = "HangupHandlerPush";
  } else {
    return;
  }
  ....
}

Крайне подозрительное место: тут либо переменной 'event' присваивается строка "HangupHandlerRun", либо выполняется выход из функции.

Всегда ложь

V547 Expression is always false. Unsigned type value is never < 0. enum.c 309

static int ebl_callback(....)
{
  unsigned int i;
  ....
  if ((i = dn_expand((unsigned char *)fullanswer,
     (unsigned char *)answer + len,
     (unsigned char *)answer, c->apex, sizeof(c->apex) - 1)) < 0)
  {
    ast_log(LOG_WARNING, "Failed to expand hostname\n");
    return 0;
  }
}

Переменная 'i' имеет беззнаковый тип и никогда не будет меньше нуля. Функция dn_expand() возвращает значение -1 в случае неудачи, тип переменной 'i' не должен быть 'unsigned'.

Коварная оптимизация

V597 The compiler could delete the 'memset' function call, which is used to flush 'buf' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. channel.c 7742

static int silence_generator_generate(....)
{
  short buf[samples];

  struct ast_frame frame = {
    .frametype = AST_FRAME_VOICE,
    .data.ptr = buf,
    .samples = samples,
    .datalen = sizeof(buf),
  };
  frame.subclass.format = ast_format_slin;
  
  memset(buf, 0, sizeof(buf));      // <=
  ....
}

Так как массив 'buf' больше не используется после вызова функции 'memset', то компилятор может удалить вызов функции для оптимизации, и массив не будет обнулён, как планировалось.

Часто предупреждение V597 остаётся непонятым. Предлагаю дополнительные материалы, раскрывающие суть проблемы:

Указатели

V595 The 'object_wizard->wizard' pointer was utilized before it was verified against nullptr. Check lines: 683, 686. sorcery.c 683

static void sorcery_object_wizard_destructor(void *obj)
{
  struct ast_sorcery_object_wizard *object_wizard = obj;

  if (object_wizard->data) {
    object_wizard->wizard->close(object_wizard->data);      // <=
  }

  if (object_wizard->wizard) {                              // <=
    ast_module_unref(object_wizard->wizard->module);
  }

  ao2_cleanup(object_wizard->wizard);                       // <=
}

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

Избыточный код

Думаю, следующие два примера не являются ошибками, но могут быть упрощены.

V584 The '1' value is present on both sides of the '==' operator. The expression is incorrect or it can be simplified. chan_unistim.c 1095

static void check_send_queue(struct unistimsession *pte)
{
  if (pte->last_buf_available == 1) {
    ....
  }
  else if (pte->last_seq_ack + 1 == pte->seq_server + 1) {  // <=
    ....
  }
}

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

V571 Recurring check. The 'wizard->wizard->retrieve_fields' condition was already verified in line 1520. sorcery.c 1521

void *ast_sorcery_retrieve_by_fields(....)
{
  ....
  if ((flags & AST_RETRIEVE_FLAG_MULTIPLE)) {
  ....
  } else if (fields && wizard->wizard->retrieve_fields) {  // <=
      if (wizard->wizard->retrieve_fields) {               // <=
        object = wizard->wizard->retrieve_fields(....);
      }
  }
}

Не ошибка, но одну проверку указателя явно можно убрать.

Заключение

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

Также об опечатках есть интересная статья: Эффект последней строки.

Популярные статьи по теме
Как различить C и C++ разработчиков по их коду

Дата: 12 Май 2022

Автор: Гость

Так уж случилось, что я пишу код для разных IoT-железок, связанных с электричеством, типа зарядных станций автомобилей. Поскольку аппаратных ресурсов, как правило, вполне достаточно, то основным фоку…
Отладочный вывод на микроконтроллерах: как Concepts и Ranges отправили мой printf на покой

Дата: 06 Май 2022

Автор: Гость

Здравствуйте! Меня зовут Александр, и я работаю программистом микроконтроллеров.
Нереальный baselining или доработки PVS-Studio для Unreal Engine проектов

Дата: 26 Апр 2022

Автор: Валерий Комаров

Статический анализатор PVS-Studio постоянно развивается: улучшаются различные механизмы, происходит интеграция с игровыми движками, IDE, CI/CD и другими системами и сервисами. Благодаря этому несколь…
Разбор некоторых вредных советов для С++ программиста

Дата: 21 Апр 2022

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

Юмор юмором, но осторожность не повредит. Вдруг кому-то не до конца понятно, почему какой-то из советов является вредным. Здесь можно найти соответствующие пояснения.
Четыре причины проверять, что вернула функция malloc

Дата: 20 Апр 2022

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

Некоторые разработчики пренебрежительно относятся к проверкам: удалось ли выделить память с помощью функции malloc или нет. Их логика проста – памяти всегда должно хватить. А если не хватит, всё равн…

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

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