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

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

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

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

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

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

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


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

>
>
>
Алло! Это FreeSWITCH? Тогда мы проверим…

Алло! Это FreeSWITCH? Тогда мы проверим вас!

11 Окт 2015

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

0351_FreeSWITCH_ru/image1.png

Введение

FreeSWITCH – масштабируемое кроссплатформенное программное обеспечение для телефонии с открытым кодом, предназначенное для маршрутизации и объединения популярных протоколов связи, использующих аудио-, видео-, текстовую и прочие виды информации. Платформа была разработана в 2006 году с целью восполнить дефицит подобных решений, образовавшийся с появлением закрытых коммерческими приложений. Кроме того, FreeSWITCH предоставляет разработчикам стабильную телефонную платформу, на основе которой они могут создавать собственные разнообразные приложения с помощью большого набора бесплатных инструментов.

С помощью анализатора PVS-Studio 5.29 проект FreeSWITCH был с лёгкостью проверен в Visual Studio 2015.

If (bug) then find_copy_paste();

0351_FreeSWITCH_ru/image2.png

V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_channel.c 493

typedef enum {
  SWITCH_STATUS_SUCCESS,
  SWITCH_STATUS_FALSE,
  SWITCH_STATUS_TIMEOUT,
  SWITCH_STATUS_RESTART,
  ....
} switch_status_t;


SWITCH_DECLARE(switch_status_t) switch_channel_queue_dtmf(....)
{
  ....
  switch_status_t status;
  ....
  if ((status = switch_core_session_recv_dtmf(channel->session,
                  dtmf) != SWITCH_STATUS_SUCCESS)) {
    goto done;
  }
  ....
}

Источником логических ошибок в программе может являться неправильно написанное условие. Например, в этом фрагменте кода приоритет оператора сравнения выше приоритета оператора присваивания. Таким образом, в 'status' сохраняется результат логической операции, а не результат функции switch_core_session_recv_dtmf(). В коде присутствуют оператор перехода goto, поэтому испорченное значение переменной 'status' в дальнейшем может использоваться где угодно.

К сожалению, таких мест много:

  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_core_db.c 208
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_core_db.c 211
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_core_db.c 214
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_core_db.c 217
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_event.c 2986
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. switch_ivr.c 3905
  • V593 Consider reviewing the expression of the 'A = B == C' kind. The expression is calculated as following: 'A = (B == C)'. fsodbc.cpp 285
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. mod_db.c 653

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

static switch_status_t load_config(void)
{
  ....
  if (globals.db_dsn) {                                     // <=
    ....
  } else if (globals.db_dsn) {                              // <=
    switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_CRIT,
      "Cannot Open ODBC Connection (did you enable it?!)\n");
  }
  ....
}

В каскаде условий проверяется одна и та же переменная "globals.db_dsn ", поэтому сообщение о неудачном подключении к базе данных не будет залогировано.

V523 The 'then' statement is equivalent to the 'else' statement. sofia_glue.c 552

char *sofia_overcome_sip_uri_weakness(....)
{
  ....
  if (strchr(stripped, ';')) {
    if (params) {
      new_uri = switch_core_session_sprintf(session, "....",
        uri_only ? "" : "<", stripped, sofia_glue_transport2str(
        transport), params, uri_only ? "" : ">");
    } else {
      new_uri = switch_core_session_sprintf(session, "....",
        uri_only ? "" : "<", stripped, sofia_glue_transport2str(
        transport), uri_only ? "" : ">");
    }
  } else {
    if (params) {
      new_uri = switch_core_session_sprintf(session, "....",
        uri_only ? "" : "<", stripped, sofia_glue_transport2str(
        transport), params, uri_only ? "" : ">");
    } else {
      new_uri = switch_core_session_sprintf(session, "....",
        uri_only ? "" : "<", stripped, sofia_glue_transport2str(
        transport), uri_only ? "" : ">");
    }
  }
  ....
}

Большой фрагмент кода, содержащий много идентичного текста. В случае отсутствия ошибки этот фрагмент кода можно сократить в два раза, иначе здесь очередной забытый copy-paste.

V590 Consider inspecting the '* data == ' ' && * data != '\0'' expression. The expression is excessive or contains a misprint. mod_curl.c 306

static char *print_json(switch_memory_pool_t *pool, ....)
{
  ....
  while (*data == ' ' && *data != '\0') {
    data++;
  }
  ....
}

Здесь ошибки нет, но выражение избыточно, что может затруднять чтение кода. Проверка " *data != '\0' " не имеет смысла. Сокращенный вариант кода:

while (*data == ' ') {
  data++;

V646 Consider inspecting the application's logic. It's possible that 'else' keyword is missing. conference_api.c 1532

switch_status_t conference_api_sub_vid_logo_img(....)
{
  ....
  if (!strcasecmp(text, "allclear")) {
    switch_channel_set_variable(member->channel, "....", NULL);
    member->video_logo = NULL;
  } if (!strcasecmp(text, "clear")) {                       // <=
    member->video_logo = NULL;
  } else {
    member->video_logo = switch_core_strdup(member->pool, text);
  }
  ....
}

По коду видно, что планировалось написать "else if'. Но видимо случайно пропустили ключевое слово 'else' и это привело к изменению логики программы.

Что-бы понять суть ошибки давайте рассмотрим упрощенный вариант кода. В начале правильный вариант:

if (A == 1) {
  X();
} else if (A == 2) {
  Y();
} else {
  Z();
}

В зависти от значения переменной A будет вызвана одна из функций: X, Y или Z. Посмотрим, что будет если забыть 'else':

if (A == 1) {
  X();
} if (A == 2) {
  Y();
} else {
  Z();
}

Теперь если A равно единице, то вызовется не только функция X, но ещё и Z!

Использование типа SOCKET

0351_FreeSWITCH_ru/image3.png

V605 Consider verifying the expression: context->curlfd > - 1. An unsigned value is compared to the number -1. mod_shout.c 151

typedef SOCKET curl_socket_t;
curl_socket_t curlfd;

static inline void free_context(shout_context_t *context)
{
  ....
  if (context->curlfd > -1) {
    shutdown(context->curlfd, 2);
    context->curlfd = -1;
  }
  ....
}

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

V547 Expression is always false. Unsigned type value is never < 0. esl.c 690

typedef SOCKET ws_socket_t;

static ws_socket_t prepare_socket(ips_t *ips) 
{
  ws_socket_t sock = ws_sock_invalid;
  
  ....
  if ((sock = socket(family, SOCK_STREAM, IPPROTO_TCP)) < 0) {
    die("Socket Error!\n");
  }
  ....
}

Похожий пример неправильной работы с переменными типа SOCKET. Это беззнаковый тип, а для проверки статуса ошибки существуют специально определённые константы, например, SOCKET_ERROR.

Двойные присваивания

0351_FreeSWITCH_ru/image4.png

V570 The variable is assigned to itself. skypopen_protocol.c 1512

struct SkypopenHandles {
  HWND win32_hInit_MainWindowHandle;
  HWND win32_hGlobal_SkypeAPIWindowHandle;
  ....
};

LRESULT APIENTRY skypopen_present(...., WPARAM uiParam, ....)
{
 ....
 if (!tech_pvt->SkypopenHandles.currentuserhandle) {
   tech_pvt->SkypopenHandles.api_connected = 1;
   tech_pvt->SkypopenHandles.win32_hGlobal_SkypeAPIWindowHandle =
    (HWND) uiParam;
   tech_pvt->SkypopenHandles.win32_hGlobal_SkypeAPIWindowHandle =
    tech_pvt->SkypopenHandles.win32_hGlobal_SkypeAPIWindowHandle;
 }
 ....
}

Анализатор обнаружил, что переменная присваивается сама себе. Можно предположить, что во время написания кода, во втором присваивании было случайно выбрано не то поле структуры: "win32_hGlobal_SkypeAPIWindowHandle" вместо "win32_hInit_MainWindowHandle".

Возможно, код функции должен быть таким:

if (!tech_pvt->SkypopenHandles.currentuserhandle) {
  tech_pvt->SkypopenHandles.api_connected = 1;
  tech_pvt->SkypopenHandles.win32_hGlobal_SkypeAPIWindowHandle =
   (HWND) uiParam;
  tech_pvt->SkypopenHandles. win32_hInit_MainWindowHandle =
   tech_pvt->SkypopenHandles.win32_hGlobal_SkypeAPIWindowHandle;
}

V519 The 'status' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 365, 368. fscoredb.cpp 368

JS_COREDB_FUNCTION_IMPL(BindInt)
{
  bool status;
  ....
  /* convert args */
  status = !info[0].IsEmpty() && info[0]->IsInt32() ? true:false;
  param_index = info[0]->Int32Value();

  status = !info[1].IsEmpty() && info[1]->IsInt32() ? true:false;
  param_value = info[1]->Int32Value();

  if (param_index < 1) {
    info.GetIsolate()->ThrowException(....);
    return;
  }
  ....
}

Анализатор обнаружил потенциально возможную ошибку, связанную с тем, что одной и той же переменной дважды подряд присваивается значение. Причем между этими присваиваниями сама переменная не используется. С помощью анализатора удалось найти пропущенную проверку: значение переменной 'status' нигде не используется.

Возможно код должен быть таким:

....
param_index = status ? info[0]->Int32Value() : 0;
....
param_value = status ? info[1]->Int32Value() : 0;

V519 The 'status' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1239, 1240. switch_core_io.c 1240

SWITCH_DECLARE(switch_status_t)
switch_core_session_write_frame(...., int stream_id)
{
  ....
  if (ptime_mismatch && status != SWITCH_STATUS_GENERR) {
    status = perform_write(session, frame, flags, stream_id);
    status = SWITCH_STATUS_SUCCESS;
    goto error;
  }
  ....
}

Почему здесь статус записи просто перезаписывают на успешный не понятно. Оставим изучение этого фрагмента авторам кода.

Ошибки в строках

0351_FreeSWITCH_ru/image5.png

V694 The condition (mode + 5) is only false if there is pointer overflow which is undefined behaviour anyway. mod_ilbc.c 51

static switch_status_t switch_ilbc_fmtp_parse(....)
{
  ....
  if (fmtp && (mode = strstr(fmtp, "mode=")) && (mode + 5)) {
      codec_ms = atoi(mode + 5);
    }
    if (!codec_ms) {
      /* default to 30 when no mode is defined for ilbc ONLY */
      codec_ms = 30;
    }
  ....
}

На первый взгляд здесь реализован простой алгоритм:

  • Найти подстроку "mode=";
  • Убедиться, что после подстроки не стоит нулевой символ;
  • Конвертировать следующий символ в число.

Ошибка кроется в этапе N2: убедившись, что указатель 'mode' на подстроку ненулевой, в коде смещают указатель на 5 символов, но он так и останется ненулевым. В операции (mode + 5) пропущено разыменование смещённого указателя. Из-за такой ошибки стали возможны ситуации, когда символ завершения строки конвертируют в число и получают значение ноль. Благодаря проверке "if (!codec_ms) { codec_ms = 30;}" нулевое значение всегда сбрасывается на значение по умолчанию.

V519 The '* e' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1438, 1439. switch_xml.c 1439

static int preprocess(....)
{
  ....
  if ((e = strstr(tcmd, "/>"))) {
    *e += 2;
    *e = '\0';
    if (fwrite(e, 1, (unsigned) strlen(e),
          write_fd) != (int) strlen(e)) {
      switch_log_printf(....);
    }
  }
  ....
}

А в этом месте возникает ошибка как в предыдущем примере, только наоборот. Здесь в случае нахождения подстроки планируют сместить указатель и записать символ конца строки, но в операции "*e += 2" изменяется не указатель, а код символа, на который он указывает. Потом в этот символ просто записывается терминальный ноль.

Правильный код выглядит так:

if ((e = strstr(tcmd, "/>"))) {
    e += 2;
    *e = '\0';
    ....
  }

V600 Consider inspecting the condition. The 'name' pointer is always not equal to NULL. fsodbc.cpp 323

JS_ODBC_FUNCTION_IMPL(GetData)
{
  ....
  SQLCHAR name[1024] = "";                                  // <=
  SQLCHAR *data = _colbuf;
  SQLLEN pcbValue;
  
  SQLDescribeCol(_stmt, x, name, sizeof(name), ....);       // <=
  SQLGetData(_stmt, x, SQL_C_CHAR, _colbuf, _cblen, &pcbValue);

  if (name) {                                               // <=
    if (SQL_NULL_DATA == pcbValue) {
      arg->Set(String::NewFromUtf8(GetIsolate(),
        (const char *)name), Null(info.GetIsolate()));
    } else {
      arg->Set(String::NewFromUtf8(GetIsolate(),
        (const char *)name), String::NewFromUtf8(GetIsolate(),
        data ? (const char *)data : ""));
    }
  }
  ....
}

В этой функции выделяется память на стеке для массива символов "name". В начало записывается нулевой символ и с массивом выполняются какие-то действия. В условии "if (name) {....} хотели проверить, осталась ли строка пустой (признаком является нулевой символ в начале строки). Но из-за пропущенного символа разыменования указателя, проверяют значение указателя, который всегда не равен нулю.

V595 The 'val' pointer was utilized before it was verified against nullptr. Check lines: 2496, 2499. switch_ivr.c 2496

static int
switch_ivr_set_xml_chan_var(...., const char *val, int off)
{
  char *data;
  switch_size_t dlen = strlen(val) * 3 + 1;            // <=
  switch_xml_t variable;

  if (!val) val = "";                                  // <=
  ....
}

В функцию может прийти нулевой указатель на массив символов "val", о чём свидетельствует наличие проверки. Но до этого такой указатель будет передан в функцию "strlen()" для вычисления длины строки, где выполнится разыменование нулевого указателя.

Опасные указатели

0351_FreeSWITCH_ru/image6.png

V713 The pointer codec->cur_frame was utilized in the logical expression before it was verified against nullptr in the same logical expression. mod_opus.c 631

static switch_status_t
switch_opus_decode(switch_codec_t *codec, ....)
{
  ....
  if (opus_packet_get_bandwidth(codec->cur_frame->data) !=  // <=
        OPUS_BANDWIDTH_FULLBAND && codec->cur_frame &&      // <=
        (jb = switch_core_session_get_jb(....))) {
    ....
  }
  ....
}

Было непросто, но анализатор нашёл потенциальное разыменование нулевого указателя. Проблема из-за неправильного порядка логических выражений в условии. Сначала используется переменная "codec->cur_frame->data", а потом выполняется проверка указателя "codec->cur_frame" на равенство нулю.

V595 The 'a_engine' pointer was utilized before it was verified against nullptr. Check lines: 6024, 6052. switch_core_media.c 6024

SWITCH_DECLARE(switch_status_t)
switch_core_media_activate_rtp(switch_core_session_t *session)
{
  ....
  switch_port_t remote_rtcp_port = a_engine->remote_rtcp_port;
  ....
  if (session && a_engine) {
    check_dtls_reinvite(session, a_engine);
  }
  ....
}

В отличии от диагностики V713, диагностика V595 ищет потенциальное разыменование нулевого указателя в пределах всех функции. Обратите внимание, как используется указатель "a_engine".

Ещё список опасных мест:

  • V595 The 'session' pointer was utilized before it was verified against nullptr. Check lines: 6027, 6052. switch_core_media.c 6027
  • V595 The 'session' pointer was utilized before it was verified against nullptr. Check lines: 6689, 6696. switch_core_media.c 6689
  • V595 The 'v_engine' pointer was utilized before it was verified against nullptr. Check lines: 6677, 6696. switch_core_media.c 6677
  • V595 The 'stream.data' pointer was utilized before it was verified against nullptr. Check lines: 2409, 2411. switch_event.c 2409
  • V595 The 'stack' pointer was utilized before it was verified against nullptr. Check lines: 461, 466. switch_ivr_menu.c 461
  • V595 The 'smin' pointer was utilized before it was verified against nullptr. Check lines: 3269, 3277. switch_utils.c 3269
  • V595 The 'key' pointer was utilized before it was verified against nullptr. Check lines: 111, 124. switch_xml.c 111

V547 Expression 'fftstate->Perm == ((void *) 0)' is always false. Pointer 'fftstate->Perm' != NULL. fft.c 339

typedef struct {
  unsigned int SpaceAlloced;
  unsigned int MaxPermAlloced;
  double Tmp0[MAXFFTSIZE];
  double Tmp1[MAXFFTSIZE];
  double Tmp2[MAXFFTSIZE];
  double Tmp3[MAXFFTSIZE];
  int Perm[MAXFFTSIZE];
  int factor [NFACTOR];

} FFTstr;

static int   FFTRADIX (...., FFTstr *fftstate)
{
  ....
  if (fftstate->Tmp0 == NULL || fftstate->Tmp1 == NULL ||
      fftstate->Tmp2 == NULL || fftstate->Tmp3 == NULL ||
      fftstate->Perm == NULL) {
    return -1;
  }
  ....
}

В коде присутствует большое, но бессмысленное условие, в котором проверяются адреса 5 массивов, являющихся членами класса FFTstr. При этом не важно, на стеке создан объект класса или в куче. Адреса массивов всегда будут отличаться он нуля. Даже если указатель 'fftstate' будет равен 0, всё равно проверки не имеют смысла, так члены Tmp0..Tmp3 имеют смещение относительно начала структуры.

Двойная защита

0351_FreeSWITCH_ru/image7.png

V530 The return value of function 'LoadLibraryExA' is required to be utilized. switch_dso.c 42

V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 41, 45. switch_dso.c 45

SWITCH_DECLARE(switch_dso_lib_t) switch_dso_open(....)
{
  HINSTANCE lib;

  lib = LoadLibraryEx(path, NULL, 0);

  if (!lib) {
    LoadLibraryEx(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
  }

  if (!lib) {
    DWORD error = GetLastError();
    *err = switch_mprintf("dll open error [%ul]\n", error);
  }

  return lib;
}

На этом фрагменте кода возникла интересная ситуация, когда в одном месте сработали 2 разные диагностики анализатора. В V530 говорится, что у функции "LoadLibraryEx()" не используется возвращаемое значение, а в V581 говорится, что присутствуют 2 проверки с одинаковым логическим выражением.

Первая проверка дескриптора "lib" проверяет загрузку модуля функцией "LoadLibraryEx()", если дескриптор нулевой, то надо попробовать загрузить модуль ещё раз. В этот момент забывают перезаписать дескриптор 'lib' новым значением, возвращаемым функцией и при второй проверке дескриптор всё равно остаётся нулевым.

Правильный фрагмент кода:

lib = LoadLibraryEx(path, NULL, 0);
if (!lib) {
    lib = LoadLibraryEx(path, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
}

Про память

0351_FreeSWITCH_ru/image8.png

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

void WebRtcIsac_InitializePitch(const double *in,
                                const double old_lag,
                                const double old_gain,
                                PitchAnalysisStruct *State,
                                double *lags)
{
  ....
  for(k = 0; k < 2*PITCH_BW+3; k++)
  {
    CorrSurf[k] = &corrSurfBuff[10 + k * (PITCH_LAG_SPAN2+4)];
  }
  /* reset CorrSurf matrix */
  memset(corrSurfBuff, 0, sizeof(double) * (10 + (2*PITCH_BW+3)
    * (PITCH_LAG_SPAN2+4)));
  ....
}

Приведенный код может оставить матрицу неочищенной. Обратите внимание, что массив "corrSurfBuff" очищается в конце и более не используется. Поэтому при сборке Release версии программы, компилятор, скорее всего, удалит вызов функции memset(). На это компилятор имеет полное право. Анализатор предлагает использовать функцию RtlSecureZeroMemory() для Windows, но так как проект кроссплатформенный, следует найти ещё способ, чтобы избежать оптимизации других компиляторов.

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

V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'abuf' is lost. Consider assigning realloc() to a temporary pointer. switch_ivr_play_say.c 1535

SWITCH_DECLARE(switch_status_t) switch_ivr_play_file(....)
{
  ....
  if (buflen > write_frame.buflen) {
    abuf = realloc(abuf, buflen);
    write_frame.data = abuf;
    write_frame.buflen = buflen;
  }
  ....
}

Данный код является потенциально опасным: рекомендуется результат функции realloc() сохранять в другой переменной. Функция realloc() производит изменение размера некоторого блока памяти. В случае, если изменение размера блока памяти в данный момент невозможно, функция вернёт нулевой указатель. Главная проблема заключается в том, что при использовании конструкции вида "ptr = realloc(ptr, ...)" указатель ptr на этот блок данных может быть утерян.

Ещё такие места:

  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'buf' is lost. Consider assigning realloc() to a temporary pointer. switch_event.c 1556
  • V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 'buf' is lost. Consider assigning realloc() to a temporary pointer. switch_event.c 1582

Остальные предупреждения

0351_FreeSWITCH_ru/image9.png

V665 Possibly, the usage of '#pragma warning(default: X)' is incorrect in this context. The '#pragma warning(push/pop)' should be used instead. Check lines: 802, 837. switch_utils.h 837

#ifdef _MSC_VER
#pragma warning(disable:6011)
#endif
static inline char *switch_lc_strdup(const char *it)
{
  ....
}


static inline char *switch_uc_strdup(const char *it)
{
  ....
}
#ifdef _MSC_VER
#pragma warning(default:6011)
#endif

Программисты часто считают, что после директивы "pragma warning(default : X)" опять начнут действовать предупреждения, отключенные ранее помощью "pragma warning(disable: X)". Это не так. Директива 'pragma warning(default : X)' устанавливает предупреждение с номером 'X' в состояние, которое действует ПО УМОЛЧАНИЮ. Это далеко не одно и то же.

Корректный вариант кода:

#pragma warning(push)
#pragma warning(disable: 6011)
....
// Correct code triggering the 6011 warning
....
#pragma warning(pop)

V555 The expression 'parser->maxlen - parser->minlen > 0' will work as 'parser->maxlen != parser->minlen'. switch_ivr.c 2342

typedef uintptr_t switch_size_t;

switch_size_t maxlen;
switch_size_t buflen;
switch_size_t minlen;

SWITCH_DECLARE(void *) switch_ivr_digit_stream_parser_feed(....)
{
  ....
  if (parser->maxlen - parser->minlen > 0 && ....) {
    len = 0;
  }
  ....
}

Разность беззнаковых чисел всегда больше нуля, если они не равны. Так здесь ошибка или имели ввиду проверку 'parser->maxlen != parser->minlen' ?

V612 An unconditional 'goto' within a loop. mod_verto.c 112

static void json_cleanup(void)
{
  ....
top:

  for (hi = switch_core_hash_first_iter(....); hi;) {
    switch_core_hash_this(hi, &var, NULL, &val);
    json = (cJSON *) val;
    cJSON_Delete(json);
    switch_core_hash_delete(json_GLOBALS.store_hash, var);
    goto top;
  }
  switch_safe_free(hi);

  switch_mutex_unlock(json_GLOBALS.store_mutex);
}

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

Ещё несколько мест:

  • V612 An unconditional 'break' within a loop. mod_event_socket.c 1643
  • V612 An unconditional 'goto' within a loop. mod_verto.c 328
  • V612 An unconditional 'break' within a loop. mod_verto.c 1993

V652 The '!' operation is executed 3 or more times in succession. mod_verto.c 3032

static switch_bool_t verto__modify_func(....)
{
  ....
  switch_core_media_toggle_hold(session,
    !!!switch_channel_test_flag(tech_pvt->channel, ....));
  ....
}

Непонятное место с использованием целых трёх операторов отрицания, возможно, имеет место опечатка.

V567 Unspecified behavior. The order of argument evaluation is not defined for 'strtol' function. Consider inspecting the 'exp' variable. switch_utils.c 3759

SWITCH_DECLARE(int) switch_number_cmp(const char *exp, int val)
{
  for (;; ++exp) {
    int a = strtol(exp, (char **)&exp, 10);
    if (*exp != '-') {
      if (a == val)
        return 1;
    } else {
      int b = strtol(++exp, (char **)&exp, 10);        // <=
      ....
    }
    if (*exp != ',')
      return 0;
  }
}

Неизвестно, будет сначала изменён указатель 'exp' или взят его адрес. Соответственно, правильно работает выражение или нет, зависит от везения.

V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. switch_core.c 3014

SWITCH_DECLARE(int) switch_max_file_desc(void)
{
  int max = 0;                                   // <=

#ifndef WIN32
#if defined(HAVE_GETDTABLESIZE)
  max = getdtablesize();
#else
  max = sysconf(_SC_OPEN_MAX);
#endif
#endif

  return max;

}

SWITCH_DECLARE(void) switch_close_extra_files(....)
{
  int open_max = switch_max_file_desc();
  int i, j;

  for (i = 3; i < open_max; i++) {               // <=
    ....
    close(i);

  skip:

    continue;

  }
}

Неизвестно, ошибка это или нет, но анализатор нашёл код-заглушку для Windows в функции "switch_max_file_desc()". Если эта функция всегда возвращает ноль в Windows, то цикл после её вызова никогда не выполняется.

Заключение

В статье я рассказал о самых подозрительных на мой взгляд участках кода проекта FreeSWITCH, найденных с помощью статического анализатора PVS-Studio. Это ещё один проект, связанный с компьютерной телефонией, как когда-то мною был проверен похожий проект Asterisk. Проект FreeSWITCH довольно больший и анализатором было выдано много интересных предупреждений, хотя предупреждений на используемые библиотеки в этом проекте было ещё больше, но это уже совсем другая история. До публикации статьи разработчики уже были уведомлены о проверке их проекта и получили полный отчёт анализатора. Поэтому какие-то участки кода могут быть исправлены.

Популярные статьи по теме
Бесплатный PVS-Studio для тех, кто развивает открытые проекты

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

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

Дата: 14 Апр 2016

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

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

Дата: 21 Ноя 2018

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

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

Дата: 20 Мар 2017

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

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

Дата: 17 Янв 2019

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

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

Дата: 27 Июн 2017

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

Проект Unreal Engine развивается - добавляется новый код и изменятся уже написанный. Неизбежное следствие развития проекта - появление в коде новых ошибок, которые желательно выявлять как можно раньш…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
PVS-Studio ROI

Дата: 30 Янв 2019

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

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

Дата: 22 Окт 2018

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

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

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

Следующие комментарии

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