PVS-Studio - это инструмент для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#.

PVS-Studio выполняет статический анализ кода и генерирует отчёт, помогающий программисту находить и устранять ошибки. PVS-Studio выполняет широкий спектр проверок кода, но наиболее силён в поисках опечаток и последствий неудачного Copy-Paste. Показательные примеры таких ошибок: V501, V517, V522, V523, V3001.

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

Проверка Firebird 3.0

Запись от el_programmer размещена 12.05.2016 в 10:57

Показов 2193 Комментарии 0

Метки c++, cpp, firebird, open source, programming

Автор: Павел Беликов

Не так давно вышла новая версия СУБД Firebird. Релиз стал одним из масштабных в истории проекта: была сильно переработана архитектура, добавлена поддержка многопоточности, улучшена производительность. Такое значительное обновление и послужило поводом для повторной проверки Firebird с помощью статического анализатора кода PVS-Studio.

Нажмите на изображение для увеличения
Название: image1.png
Просмотров: 875
Размер: 63.2 Кб
ID: 3791

Введение

Firebird - это кроссплатформенная свободная система управлениями базами данных. Проект написан на C++ и работает на Microsoft Windows, Linux, Mac OS X и многих Unix-like операционных системах. СУБД полностью бесплатна для использования и распространения. Подробнее о Firebird можно узнать на официальном сайте.

Ранее Firebird уже проверялся анализатором. С отчётом о предыдущей проверке можно ознакомиться в статье "Побочный результат: проверяем Firebird с помощью PVS-Studio". Для проверки был взят код с GitHub из ветви master. Сборка подробно описана в соответствующей статье на сайте проекта. Анализ исходных файлов производился в PVS-Studio Standalone версии 6.03 с помощью перехвата вызова компиляторов (Compiler Monitoring). Данная технология позволяет проверять проекты без интеграции в сборочную систему. Полученный отчёт можно просмотреть как в Standalone версии, так и в Visual Studio.

Опечатки

C++
1
2
3
4
5
6
7
void advance_to_start()
{
  ....
  if (!isalpha(c) && c != '_' && c != '.' && c != '_')
    syntax_error(lineno, line, cptr);
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'c != '_'' to the left and to the right of the '&&' operator. reader.c 1203

Анализатор обнаружил, что в логической операции присутствуют два одинаковых подвыражения c != '_'. В последнем условии допущена опечатка и переменная c должна сравниваться с другим символом. В других функциях используется проверка с символом '$', возможно здесь нужно использовать его:

C++
1
if (!isalpha(c) && c != '_' && c != '.' && c != '$')

Ещё один пример ошибки от невнимательности:

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
int put_message(....)
{
  if (newlen <= MAX_UCHAR)
    {
    put(tdgbl, attribute);
    put(tdgbl, (UCHAR) newlen);
  }
  else if (newlen <= MAX_USHORT)
  {
    if (!attribute2)
      BURP_error(314, "");
    ....
  }
  else
    BURP_error(315, "");
  ....
}

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

V601 The string literal is implicitly cast to the bool type. Inspect the second argument. backup.cpp 6113
V601 The string literal is implicitly cast to the bool type. Inspect the second argument. backup.cpp 6120

Здесь неверно используется функция BURP_error. Она объявлена следующим образом:

C++
1
2
3
4
void BURP_error(USHORT errcode, bool abort,
     const MsgFormat::SafeArg& arg = MsgFormat::SafeArg());
 
void BURP_error(USHORT errcode, bool abort, const char* str);

Второй аргумент имеет логический тип, а строка стоит третьим аргументом. В результате строковый литерал приводится к true. Вызов функции должен быть переписан как BURP_error(315, true, "") или BURP_error(315, false, "").

Однако, есть случаи, когда распознать ошибку может только разработчик проекта.

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
void IDX_create_index(....)
{
  ....
  index_fast_load ifl_data;
  ....
  if (!ifl_data.ifl_duplicates)
    scb->sort(tdbb);
 
  if (!ifl_data.ifl_duplicates)
    BTR_create(tdbb, creation, selectivity);
 
  ....
}

Предупреждение PVS-Studio: V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 506, 509. idx.cpp 509

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

В следующем примере рассмотрим ситуацию, связанную с указателями.

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
static void string_to_datetime(....)
{
  ....
 
  const char* p = NULL;
  const char* const end = p + length;
 
  ....
 
  while (p < end)
  {
    if (*p != ' ' && *p != '\t' && p != 0)
    {
      CVT_conversion_error(desc, err);
      return;
    }
    ++p;
  }
 
  ....
}

Предупреждение PVS-Studio: V713 The pointer p was utilized in the logical expression before it was verified against nullptr in the same logical expression. cvt.cpp 702

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

Если посмотреть выше по коду, то можно найти аналогичный фрагмент:

C++
1
2
3
4
5
while (++p < end)
{
  if (*p != ' ' && *p != '\t' && *p != 0)
    CVT_conversion_error(desc, err);
}

Для того, чтобы избежать подобной ошибки, для сравнения с нулём лучше использовать соответствующие литералы: '\0' для типа char, 0 для чисел и nullptr для указателей. Если взять это за правило, можно избежать множества подобных глупых ошибок.

Опасное использование memcmp

C++
1
2
3
4
5
6
7
8
9
10
11
12
SSHORT TextType::compare(ULONG len1, const UCHAR* str1,
                         ULONG len2, const UCHAR* str2)
{
  ....
 
  SSHORT cmp = memcmp(str1, str2, MIN(len1, len2));
 
  if (cmp == 0)
    cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0));
 
  return cmp;
}

Предупреждение PVS-Studio: V642 Saving the 'memcmp' function result inside the 'short' type variable is inappropriate. The significant bits could be lost breaking the program's logic. texttype.cpp 3

Функция memcmp возвращает следующие значения:

< 0, если str1 меньше str2
0, если str1 равен str2
> 0, если str1 больше str2

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

Лишние проверки

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
void Trigger::compile(thread_db* tdbb)
{
  SET_TDBB(tdbb);
 
  Database* dbb = tdbb->getDatabase();
  Jrd::Attachment* const att = tdbb->getAttachment();
 
  if (extTrigger)
    return;
 
  if (!statement /*&& !compile_in_progress*/)
  {
    if (statement)
      return;
 
    ....
  }
}

Предупреждение PVS-Studio: V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 778, 780. jrd.cpp 778

Анализатор нашёл проверку двух противоположных условий. Скорее всего, второе условие здесь стало не нужным в результате изменения первого и его можно удалить, но решение тут стоит принимать автору.

Ещё одним примером странного ветвления может послужить следующий фрагмент кода.

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
static void asgn_from( ref* reference, int column)
{
  TEXT variable[MAX_REF_SIZE];
  TEXT temp[MAX_REF_SIZE];
 
  for (; reference; reference = reference->ref_next)
  {
    const gpre_fld* field = reference->ref_field;
    ....
 
    if (!field || field->fld_dtype == dtype_text)
      ....
    else if (!field || field->fld_dtype == dtype_cstring)
      ....
    else
      ....
  }
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always false: !field. int_cxx.cpp 217

Если field не нулевой указатель, то код никогда не достигнет условия в else if. Либо данная проверка лишняя, либо на её месте должно было быть другое сравнение. Неизвестно, противоречит ли данное условие логике приложения.

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

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
bool XnetServerEndPoint::server_init(USHORT flag)
{
  ....
 
  xnet_connect_mutex = CreateMutex(ISC_get_security_desc(),
                          FALSE, name_buffer);
  if (!xnet_connect_mutex ||
          (xnet_connect_mutex && ERRNO == ERROR_ALREADY_EXISTS))
  {
    system_error::raise(ERR_STR("CreateMutex"));
  }
 
  ....
}

Предупреждение PVS-Studio: V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!xnet_connect_mutex' and 'xnet_connect_mutex'. xnet.cpp 2231

Проверку if (!xnet_connect_mutex || (xnet_connect_mutex && ERRNO == ERROR_ALREADY_EXISTS)) можно упростить до if (!xnet_connect_mutex || ERRNO == ERROR_ALREADY_EXISTS). Это можно легко доказать с помощью таблицы истинности.

Опасное сравнение беззнаковой переменной

C++
1
2
3
4
5
6
static bool write_page(thread_db* tdbb, BufferDesc* bdb, ....)
{
  ....
  if (bdb->bdb_page.getPageNum() >= 0)
  ....
}

Предупреждение PVS-Studio: V547 Expression 'bdb->bdb_page.getPageNum() >= 0' is always true. Unsigned type value is always >= 0. cch.cpp 4827

Условие bdb->bdb_page.getPageNum() >= 0 всегда окажется верным, так как функция возвращает значение беззнакового типа. Возможно, программист неверно проверил значение. Принимая во внимания аналогичные сравнения в проекте, можно предположить, что код должен выглядеть следующим образом:

C++
1
if (bdb->bdb_page.getPageNum() != 0)

Разыменовывание нулевых указателей

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
static bool initializeFastMutex(FAST_MUTEX* lpMutex, 
  LPSECURITY_ATTRIBUTES lpAttributes, BOOL bInitialState,
  LPCSTR lpName)
{
  if (pid == 0)
    pid = GetCurrentProcessId();
  
  LPCSTR name = lpName;
 
  if (strlen(lpName) + strlen(FAST_MUTEX_EVT_NAME) - 2
                                                   >= MAXPATHLEN)
  {
    SetLastError(ERROR_FILENAME_EXCED_RANGE);
    return false;
  }
 
  setupMutex(lpMutex);
 
  char sz[MAXPATHLEN]; 
  if (lpName)
  ....
}

Предупреждение PVS-Studio: V595 The 'lpName' pointer was utilized before it was verified against nullptr. Check lines: 2814, 2824. isc_sync.cpp 2814

Предупреждение V595 является самым часто встречаемым в проектах, проверенных PVS-Studio, и Firebird не стал исключением. Всего обнаружено более 30 мест с данным предупреждением.

В этом примере вызов strlen(lpName) идёт перед проверкой указателя на nullptr. Это приведёт к неопределённому поведению при попытке передать в функцию нулевой указатель. Разыменовывание указателя здесь спрятано в вызове strlen, что затрудняет обнаружение ошибки, если не пользоваться статическим анализатором.

Проверка на nullptr после new

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
rem_port* XnetServerEndPoint::get_server_port(....)
{
  ....
  XCC xcc = FB_NEW struct xcc(this);
 
  try {
 
    ....
  }
  catch (const Exception&)
  {
    if (port)
      cleanup_port(port);
    else if (xcc)
      cleanup_comm(xcc);
 
    throw;
  }
 
  return port;
}

Предупреждение PVS-Studio: V668 There is no sense in testing the 'xcc' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. xnet.cpp 2533

Анализатор предупреждает, что оператор new не может вернуть nullptr - для проверки необходимо использовать блок try-catch или new (std::nothrow). Однако, в данном примере всё несколько сложнее. Для выделения памяти используется макрос FB_NEW. Он объявлен в файле alloc.h:

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
#ifdef USE_SYSTEM_NEW
#define OOM_EXCEPTION std::bad_alloc
#else
#define OOM_EXCEPTION Firebird::BadAlloc
#endif
 
#define FB_NEW new(__FILE__, __LINE__)
 
inline void* operator new(size_t s ALLOC_PARAMS)
throw (OOM_EXCEPTION)
{
  return MemoryPool::globalAlloc(s ALLOC_PASS_ARGS);
}

Сложно сказать, является ли этот конкретный пример ошибкой или нет, так как используется нестандартный аллокатор. Но из-за того, что в определении оператора указан throw (std::bad_alloc), подобная проверка выглядит подозрительно.

Опасное использование realloc

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
int mputchar(struct mstring *s, int ch)
{
  if (!s || !s->base) return ch;
  if (s->ptr == s->end) {
    int len = s->end - s->base;
    if ((s->base = realloc(s->base, len+len+TAIL))) {
      s->ptr = s->base + len;
      s->end = s->base + len+len+TAIL; }
    else {
      s->ptr = s->end = 0;
      return ch; } }
  *s->ptr++ = ch;
  return ch;
}

Предупреждение PVS-Studio: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 's->base' is lost. Consider assigning realloc() to a temporary pointer. mstring.c 42

Выражение вида ptr = realloc(ptr, size) плохо тем, что в случае, когда realloc вернёт nullptr, указатель на память будет потерян. Чтобы этого избежать, следует сохранить результат realloc в временную переменную и после проверки на nullptr присвоить ptr её значение.

C++
1
2
3
4
5
6
temp_ptr = realloc(ptr, new_size);
if (temp_ptr == nullptr) {
  //handle exception
} else {
  ptr = temp_ptr;
}

Неиспользованные значения enum в switch

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
template <typename CharType>
LikeEvaluator<CharType>::LikeEvaluator(....)
{
  ....
  PatternItem *item = patternItems.begin();
  ....
  switch (item->type)
  {
  case piSkipFixed:
  case piSkipMore:
    patternItems.grow(patternItems.getCount() + 1);
    item = patternItems.end() - 1;
    // Note: fall into
    case piNone:
      item->type = piEscapedString;
      item->str.data = const_cast<CharType*>
                        (pattern_str + pattern_pos - 2);
      item->str.length = 1;
      break;
    case piSearch:
      item->type = piEscapedString;
      // Note: fall into
    case piEscapedString:
      item->str.length++;
      break;
  }
  ....
}

Предупреждение PVS-Studio: V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch. evl_string.h 324

В конструкции switch были использованы не все значения enum, и отсутствует блок default. Возможно, что здесь забыли добавить обработку piDirectMatch. Список мест с аналогичным предупреждением:

V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch, piSkipMore. evl_string.h 351
V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch. evl_string.h 368
V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch. evl_string.h 387

Переполнение буфера

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
const int GDS_NAME_LEN = 32;
....
bool get_function(BurpGlobals* tdgbl)
{
  ....
  struct isc_844_struct {
    ....
    short isc_870; /* gds__null_flag */
    ....
    char  isc_874 [125]; /* RDB$PACKAGE_NAME */
    ....
  } isc_844;
 
  att_type attribute;
  TEXT    temp[GDS_NAME_LEN * 2];
  ....
  SSHORT prefixLen = 0;
  if (!/*X.RDB$PACKAGE_NAME.NULL*/
       isc_844.isc_870)
  {
    prefixLen = static_cast<SSHORT>(strlen(/*X.RDB$PACKAGE_NAME*/
                                           isc_844.isc_874));
    memcpy(temp, /*X.RDB$PACKAGE_NAME*/
                 isc_844.isc_874, prefixLen);
    temp[prefixLen++] = '.';
  }
  ....
 
}

Предупреждение PVS-Studio: V557 Array overrun is possible. The value of 'prefixLen ++' index could reach 124. restore.cpp 10040

Размер буфера isc_844.isc_874 равен 125, соответственно, максимальное возможное значение strlen(isc_844.isc_874) - 124. Размер temp - 64, что меньше этого значения. Запись по такому индексу может привести к переполнению буфера. Безопасней будет выделить больший объём памяти для переменной temp.

Сдвиг отрицательных чисел

C++
1
2
3
4
5
6
7
8
9
10
11
static ISC_STATUS stuff_literal(gen_t* gen, SLONG literal)
{
  ....
 
  if (literal >= -32768 && literal <= 32767)
    return stuff_args(gen, 3, isc_sdl_short_integer, literal, 
                      literal >> 8);
 
  ....
 
}

Предупреждение PVS-Studio: V610 Unspecified behavior. Check the shift operator '>>'. The left operand is negative ('literal' = [-32768..32767]). array.cpp 848

Код содержит сдвиг вправо отрицательного числа. В стандарте C++ указано, что такое действие имеет неуточняемое поведение, значит на разных компиляторах и платформах может выдавать разный результат. Лучше переписать его так:

C++
1
2
3
if (literal >= -32768 && literal <= 32767)
  return stuff_args(gen, 3, isc_sdl_short_integer, literal, 
                    (ULONG)literal >> 8);

Ещё одно место, где сработало предупреждение:

V610 Unspecified behavior. Check the shift operator '>>'. The left operand is negative ('i64value' = [-2147483648..2147483647]). exprnodes.cpp 6382

Переопределение переменной

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg)
{
  int exit_code = -1;
  try
  {
    Service* svc = (Service*)arg;
    RefPtr<SvcMutex> ref(svc->svc_existence);
    int exit_code = svc->svc_service_run->serv_thd(svc);
 
    svc->started();
    svc->svc_sem_full.release();
    svc->finish(SVC_finished);
  }
  catch (const Exception& ex)
  {
    // Not much we can do here
    iscLogException("Exception in Service::run():", ex);
  }
 
  return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
}

Предупреждение PVS-Studio: V561 It's probably better to assign value to 'exit_code' variable than to declare it anew. Previous declaration: svc.cpp, line 1893. svc.cpp 1898

В этом примере вместо присвоения переменная exit_code переопределяется. Это скрывает из области видимости предыдущую переменную, и в итоге функция возвращает неверное значение, всегда равное -1.

Корректный код:

C++
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg)
{
  int exit_code = -1;
  try
  {
    Service* svc = (Service*)arg;
    RefPtr<SvcMutex> ref(svc->svc_existence);
    exit_code = svc->svc_service_run->serv_thd(svc);
 
    svc->started();
    svc->svc_sem_full.release();
    svc->finish(SVC_finished);
  }
  catch (const Exception& ex)
  {
    // Not much we can do here
    iscLogException("Exception in Service::run():", ex);
  }
 
  return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
}

Заключение

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

Метки c++, cpp, firebird, open source, programming

Размещено в Без категории

« Пояснение про ограничения демонстрационной версии PVS-Studio Главная страница Демонстрация возможностей анализатора PVS-Studio на примере открытых проектов компании Microsoft »

Надоела реклама? Зарегистрируйтесь и она исчезнет полностью.

Всего комментариев 0

Комментарии

Новые блоги и статьи Все статьи Все блоги /
Nekobox - outbounds[0].transport: unknown transport type: raw damix 01.10.2026 Фикс ошибки Правым кликом по серверу -> отладочная информация -> edit Заменить "net": "raw", на "net": "tcp", Нажать кнопку reload.	Программный домашний кинотеатр russiannick 27.09.2026 Сподобился на программный домашний кинотеатр. В качестве ЯВУ по традиции выбрал js. В помощники взял Яндекс-Алису. Было создано три зала на разные интересы. исторические и ретро сериал Хичкок. . .	Беседа с ИИ о программистах, недопускающих к созданию и правке кода генеративные ИИ и причины этого zorxor 21.09.2026 Раньше я радовался или получал некоторые эмоции, пусть небольшие, но всё же, от самого процесса написания кода, рекомпиляции и запуска, видя постепенное развитие программы и прочее. А теперь лень. . .	Мобильное приложение ColorStep pavlinmavlin 17.09.2026 Реализовал приложение Красный, Зеленый, Синий в Unity3d + c#. Название изменил на ColorStep. Приложение прошло модерацию и теперь доступно для скачивания. Делал его сам, шаг за шагом — и вот,. . .
Запрет дублирования строк в табличной части Maks 13.09.2026 Реализация из решения ниже выполнена на нетиповом справочнике "Нормы ТО" с табличной часть "Виды ТО", разработанного в КА2, со следующими реквизитами: - ВидТО (СправочникСсылка. ВидыТО); - ВидГСМ. . .	Скрипты Tampermonkey для CyberForum, ChatGPT, Claude и пр. Jin X 06.09.2026 Скрипты Tampermonkey для CyberForum, ChatGPT, Claude и пр. Работая с форумом и нейросетями в браузере часто хочется что-то подкорректировать или добавить какого-то функционала. Ниже прикреплён. . .	Программа опроса у.з. расходомера SLS-720F Argus19 02.09.2026 Программа опроса у. з. расходомера SLS-720F Программа опрашивает один раз в минуту три ультразвуковых расходомера SLS-720F через интерфейс RS-485 по протоколу Modbus RTU. Опрашиваются регистры. . .	Hyper-V: Компьютер должен поддерживать доверенный платформенный модуль 2.0. Maks 31.08.2026 При установке Windows 11 на виртуальную машину Hyper-V 2-го поколения вылезла такая ошибка: Решение: в параметрах виртуальной машины, в разделе "Безопасность" (Security) активировать флаг. . .

Сообщение форума

Отменить изменения