Аномалии в X-Ray Engine

X-Ray Engine — игровой движок, используемый в игре S.T.A.L.K.E.R. серия игр. Его код был обнародован 16 сентября 2014 года, и с тех пор фанаты STALKER продолжают его разработку. Большой размер проекта и огромное количество багов в играх дает нам прекрасную возможность показать, на что способен PVS-Studio.

Введение

Рентген был создан украинской компанией GSC GameWorld для игры S.T.A.L.K.E.R.: Тень Чернобыля. Этот движок имеет рендерер с поддержкой DirectX 8.1/9.0c/10/10.1/11, физический и звуковой движки, мультиплеер и систему искусственного интеллекта — A-Life. Позже компания собиралась создать версию 2.0 для своей новой игры, но разработка была прекращена, а исходный код выложен в открытый доступ.

Этот проект легко собирается со всеми его зависимостями в Visual Studio 2015. Для анализа мы использовали исходный код движка 1.6v из репозитория на GitHub и статический анализ кода PVS-Studio 6.05, который можно скачать с этого "ссылка на сайт".

Копировать вставить

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

MxMatrix& MxQuadric::homogeneous(MxMatrix& H) const
{
  ....
  unsigned int i, j;
  for(i=0; i<A.dim(); i++)  for(j=0; j<A.dim(); i++)
    H(i,j) = A(i,j);
  ....
}

Предупреждение PVS-Studio:V533 Вероятно, внутри оператора for увеличивается не та переменная. Подумайте о пересмотре i. mxqmetric.cpp 76

Анализатор обнаружил, что во вложенном цикле for переменная i увеличивается, но проверяется другая переменная -j, что приводит к бесконечному петля. Скорее всего, программист просто забыл его изменить.

void CBaseMonster::settings_read(CInifile const * ini,
                                 LPCSTR section, 
                                 SMonsterSettings &data)
{
  ....
  if (ini->line_exist(ppi_section,"color_base"))
    sscanf(ini->r_string(ppi_section,"color_base"), "%f,%f,%f", 
           &data.m_attack_effector.ppi.color_base.r, 
           &data.m_attack_effector.ppi.color_base.g, 
           &data.m_attack_effector.ppi.color_base.b);        
  if (ini->line_exist(ppi_section,"color_base"))
    sscanf(ini->r_string(ppi_section,"color_gray"), "%f,%f,%f", 
           &data.m_attack_effector.ppi.color_gray.r, 
           &data.m_attack_effector.ppi.color_gray.g, 
           &data.m_attack_effector.ppi.color_gray.b);
  if (ini->line_exist(ppi_section,"color_base"))
    sscanf(ini->r_string(ppi_section,"color_add"), "%f,%f,%f", 
           &data.m_attack_effector.ppi.color_add.r,  
           &data.m_attack_effector.ppi.color_add.g,    
           &data.m_attack_effector.ppi.color_add.b);
  ....
}

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

• V581 Условные выражения операторов если, расположенных рядом друг с другом, идентичны. Проверьте строки: 445, 447. base_monster_startup.cpp 447
• V581 Условные выражения операторов если, расположенных рядом друг с другом, идентичны. Проверьте строки: 447, 449. base_monster_startup.cpp 449

В этом фрагменте мы видим несколько условных выражений подряд. Очевидно, нам нужно заменить color_base на color_gray и color_add в соответствии с кодом в ветке if.

/* process a single statement */
static void ProcessStatement(char *buff, int len)
{
  ....
  if (strncmp(buff,"\\pauthr\\",8) == 0)
  {
    ProcessPlayerAuth(buff, len);
  } else if (strncmp(buff,"\\getpidr\\",9) == 0)
  {
    ProcessGetPid(buff, len);
  } else if (strncmp(buff,"\\getpidr\\",9) == 0)
  {
    ProcessGetPid(buff, len);
  } else if (strncmp(buff,"\\getpdr\\",8) == 0)
  {
    ProcessGetData(buff, len);
  } else if (strncmp(buff,"\\setpdr\\",8) == 0)
  {
    ProcessSetData(buff, len);
  }  
}

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

Как и в предыдущем примере, здесь используются два похожих условия (strncmp(buff”,\\getpidr\\”,9) == 0). Трудно сказать точно, ошибка это или просто недостижимый код, но пересмотреть точно стоит. Возможно, у нас должны быть блоки с getpidr/setpidr по аналогии с getpdr/setpdr.

class RGBAMipMappedCubeMap
{
  ....
  size_t height() const
  {
    return cubeFaces[0].height();
  }
  size_t width() const
  {
    return cubeFaces[0].height();
  }
  ....
};

Предупреждение PVS-Studio:V524 Странно, что тело функции ширина полностью эквивалентно телу функции высота. тпиксель.ч 1090

Методы height() и width() имеют одно и то же тело. Учитывая, что здесь мы оцениваем грани куба, возможно, ошибки нет. Но лучше переписать метод width() следующим образом:

size_t width() const
{
  return cubeFaces[0].width();
}

Неправильное использование С++

C++ — прекрасный язык, предоставляющий программисту массу возможностей… выстрелить себе в ногу множеством самых жестоких способов. Неопределенное поведение, утечки памяти и, конечно же, опечатки. И об этом пойдет речь в этом разделе.

template <class T>
struct _matrix33
{
public:
  typedef _matrix33<T>Self;
  typedef Self& SelfRef;
  ....
  IC SelfRef sMTxV(Tvector& R, float s1, const Tvector& V1) const
  {
    R.x = s1*(m[0][0] * V1.x + m[1][0] * V1.y + m[2][0] * V1.z);
    R.y = s1*(m[0][1] * V1.x + m[1][1] * V1.y + m[2][1] * V1.z);
    R.z = s1*(m[0][2] * V1.x + m[1][2] * V1.y + m[2][2] * V1.z);
  }
  ....
}

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

В конце метода нет return *this. Согласно стандарту, это приведет к неопределенному поведению. Поскольку возвращаемое значение является ссылкой, это, вероятно, приведет к сбою программы при попытке доступа к возвращаемому значению.

ETOOLS_API int __stdcall ogg_enc(....)
{
  ....
  FILE *in, *out    = NULL;
  ....
  input_format    *format;
  ....
  in = fopen(in_fn, "rb");
  if(in == NULL)  return 0;
  format = open_audio_file(in, &enc_opts);
  if(!format){
    fclose(in);
    return 0;
  };
  out = fopen(out_fn, "wb");
  if(out == NULL){
    fclose(out);
    return 0;
  }    
  ....
}

Предупреждение PVS-Studio:V575 Нулевой указатель передается в функцию fclose. Проверьте первый аргумент. ogg_enc.cpp 47

Довольно интересный пример. Анализатор обнаружил, что аргументом в fclose является nullptr, что делает вызов функции бессмысленным. Предположительно, поток in должен был быть закрыт.

void NVI_Image::ABGR8_To_ARGB8()
{
  // swaps RGB for all pixels
  assert(IsDataValid());
  assert(GetBytesPerPixel() == 4);
  UINT hxw = GetNumPixels();
  for (UINT i = 0; i < hxw; i++)
  {
    DWORD col;
    GetPixel_ARGB8(&col, i);
    DWORD a = (col >> 24) && 0x000000FF;
    DWORD b = (col >> 16) && 0x000000FF;
    DWORD g = (col >> 8)  && 0x000000FF;
    DWORD r = (col >> 0)  && 0x000000FF;
    col = (a << 24) | (r << 16) | (g << 8) | b;
    SetPixel_ARGB8(i, col);
  }
}

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

• V560 Часть условного выражения всегда истинна: ​​0x000000FF. nvi_image.cpp 170
• V560 Часть условного выражения всегда истинна: ​​0x000000FF. nvi_image.cpp 171
• V560 Часть условного выражения всегда истинна: ​​0x000000FF. nvi_image.cpp 172
• V560 Часть условного выражения всегда истинна: ​​0x000000FF. nvi_image.cpp 173

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

Правильный вариант:

DWORD a = (col >> 24) & 0x000000FF;
DWORD b = (col >> 16) & 0x000000FF;
DWORD g = (col >> 8)  & 0x000000FF;
DWORD r = (col >> 0)  & 0x000000FF;

Еще один пример странного кода:

VertexCache::VertexCache()
{
  VertexCache(16);
}

Предупреждение PVS-Studio:V603 Объект создан, но не используется. Если вы хотите вызвать конструктор, следует использовать ‘this-›VertexCache::VertexCache(….)’. vertexcache.cpp 6

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

BOOL CActor::net_Spawn(CSE_Abstract* DC)
{
  ....
  m_States.empty();
  ....
}

Предупреждение PVS-Studio:V530 Необходимо использовать возвращаемое значение функции пусто. act_network.cpp 657

Анализатор предупреждает, что значение, возвращаемое функцией, не используется. Похоже, что программист перепутал методы empty() и clear(): метод empty() не очищает массив, а проверяет, он пустой или нет.

Такие ошибки довольно обычны в различных проектах. Дело в том, что название empty() не очень очевидно: некоторые рассматривают его как действие — удаление. Чтобы избежать такой двусмысленности, рекомендуется добавить has, или is в начало метода: будет труднее перепутать isEmpty () с помощью clear().

Аналогичное предупреждение:

V530 Необходимо использовать возвращаемое значение функции «unique». uidragdroplistex.cpp 780

size_t xrDebug::BuildStackTrace(EXCEPTION_POINTERS* exPtrs,
                                char *buffer,
                                size_t capacity,
                                size_t lineCapacity)
{
  memset(buffer, capacity*lineCapacity, 0);
  ....
}

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

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

Правильное использование memset:

memset(buffer, 0, capacity*lineCapacity);

Следующая ошибка связана с неправильно сформированным логическим выражением.

void configs_dumper::dumper_thread(void* my_ptr)
{
  ....
  DWORD wait_result = WaitForSingleObject(
             this_ptr->m_make_start_event, INFINITE);
  while ( wait_result != WAIT_ABANDONED) ||
         (wait_result != WAIT_FAILED))
  ....
}

Предупреждение PVS-Studio:Выражение V547 всегда истинно. Вероятно, здесь следует использовать оператор &&. configs_dumper.cpp 262

Выражение ‘x != a || x != b’ всегда верно. Скорее всего, && должен был быть здесь вместо || оператор.

Подробнее на тему ошибок в логических выражениях можно прочитать в статье Логические выражения в C/C++. Ошибки профессионалов».

void SBoneProtections::reload(const shared_str& bone_sect, 
                              IKinematics* kinematics)
{
  ....
  CInifile::Sect &protections = pSettings->r_section(bone_sect);
  for (CInifile::SectCIt i=protections.Data.begin();
       protections.Data.end() != i; ++i) 
  {
    string256 buffer;
    BoneProtection BP;
    ....
    BP.BonePassBullet = (BOOL) (
                atoi( _GetItem(i->second.c_str(), 2, buffer) )>0.5f);
    ....
  }
}

Предупреждение PVS-Studio:V674 Литерал ‘0.5f’ типа ‘float’ сравнивается со значением типа ‘int’. Boneprotections.cpp 54

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

class IGameObject :
  public virtual IFactoryObject,
  public virtual ISpatial,
  public virtual ISheduled,
  public virtual IRenderable,
  public virtual ICollidable
{
public:
  ....
  virtual u16 ID() const = 0;
  ....
}
BOOL CBulletManager::test_callback(
  const collide::ray_defs& rd,
  IGameObject* object,
  LPVOID params)
{
  bullet_test_callback_data* pData = 
             (bullet_test_callback_data*)params;
  SBullet* bullet = pData->pBullet;
  if( (object->ID() == bullet->parent_id) && 
      (bullet->fly_dist<parent_ignore_distance) &&
      (!bullet->flags.ricochet_was)) return FALSE;
  BOOL bRes = TRUE;
  if (object){
    ....
  }
    
  return bRes;
}

Предупреждение PVS-Studio:V595 Указатель ‘object’ был использован до того, как он был проверен на соответствие nullptr. Контрольные строки: 42, 47. level_bullet_manager_firetrace.cpp 42

Проверка указателя object на соответствие nullptr происходит после разыменования object-›ID().В случае, когда object имеет значение nullptr, программа аварийно завершает работу.

#ifdef _EDITOR
BOOL WINAPI DllEntryPoint(....)
#else
BOOL WINAPI DllMain(....)
#endif
{
  switch (ul_reason_for_call)
  {
  ....
  case DLL_THREAD_ATTACH:
    if (!strstr(GetCommandLine(), "-editor"))
      CoInitializeEx(NULL, COINIT_MULTITHREADED);
    timeBeginPeriod(1);
    break;
  ....
  }
  return TRUE;
}

Предупреждение PVS-Studio:V718 Функцию CoInitializeEx нельзя вызывать из функции DllMain. xrcore.cpp 205

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

Ошибки приоритета

int sgetI1( unsigned char **bp )
{
  int i;
  if ( flen == FLEN_ERROR ) return 0;
  i = **bp;
  if ( i > 127 ) i -= 256;
  flen += 1;
  *bp++;
  return i;
}

Предупреждение PVS-Studio:V532 Рассмотрите возможность проверки оператора шаблона ‘*pointer++’. Вероятно имелось в виду: ‘(*pointer)++’. lwio.c 316

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

*(bp++);

Так у нас будет сдвиг не содержимого по bpадресу, а самого указателя, что в данном контексте бессмысленно. Далее в коде есть фрагменты типа *bp += N, натолкнувшие меня на мысль, что это ошибка.

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

Похожие предупреждения:

• V532 Рассмотрите возможность проверки оператора шаблона ‘*pointer++’. Вероятно имелось в виду: ‘(*pointer)++’. lwio.c 354
• V532 Рассмотрите возможность проверки оператора шаблона ‘*pointer++’. Вероятно имелось в виду: ‘(*pointer)++’. лвоб.c 80

void CHitMemoryManager::load    (IReader &packet)
{
  ....
  if (!spawn_callback || !spawn_callback->m_object_callback)
    if(!g_dedicated_server)
      Level().client_spawn_manager().add(
          delayed_object.m_object_id,m_object->ID(),callback);
#ifdef DEBUG
  else {
    if (spawn_callback && spawn_callback->m_object_callback) {
      VERIFY(spawn_callback->m_object_callback == callback);
    }
  }
#endif // DEBUG
}

Предупреждение PVS-Studio:V563 Возможно, эта ветвь else должна применяться к предыдущему оператору if. hit_memory_manager.cpp 368

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

Так что рекомендация проста — ставить фигурные скобки в более-менее сложные ветки.

void HUD_SOUND_ITEM::PlaySound(HUD_SOUND_ITEM&     hud_snd,
                                const Fvector&     position,
                                const IGameObject* parent,
                                bool               b_hud_mode,
                                bool               looped,
                                u8                 index)
{
  ....
  hud_snd.m_activeSnd->snd.set_volume(
    hud_snd.m_activeSnd->volume * b_hud_mode?psHUDSoundVolume:1.0f);
}

Предупреждение PVS-Studio:V502 Возможно, оператор ‘?:’ работает не так, как ожидалось. Оператор ?: имеет более низкий приоритет, чем оператор *. hudsound.cpp 108

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

(hud_snd.m_activeSnd->volume * b_hud_mode)?psHUDSoundVolume:1.0f

Судя по всему, правильный код должен быть следующим:

hud_snd.m_activeSnd->volume * (b_hud_mode?psHUDSoundVolume:1.0f)

Выражения, содержащие тернарный оператор, несколько ветвей if-else или операции AND/OR — это те случаи, когда лучше ставить дополнительные скобки.

Похожие предупреждения:

• V502 Возможно, оператор ‘?:’ работает не так, как ожидалось. Оператор «?:» имеет более низкий приоритет, чем оператор «+». uihudstateswnd.cpp 487
• V502 Возможно, оператор ‘?:’ работает не так, как ожидалось. Оператор «?:» имеет более низкий приоритет, чем оператор «+». uicellcustomitems.cpp 106

Ненужные сравнения

void CDestroyablePhysicsObject::OnChangeVisual()
{
  if (m_pPhysicsShell){
    if(m_pPhysicsShell)m_pPhysicsShell->Deactivate();
    ....
  }
  ....
}

Предупреждение PVS-Studio:V571 Периодическая проверка. Условие if (m_pPhysicsShell) уже проверено в строке 32. destroyablephysicsobject.cpp 33

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

void CSE_ALifeItemPDA::STATE_Read(NET_Packet &tNetPacket,
                                  u16 size)
{
  ....
  if (m_wVersion > 89)
  if ( (m_wVersion > 89)&&(m_wVersion < 98)  )
  {
    ....
  }else{
    ....
  }
}

Предупреждение PVS-Studio:V571 Периодическая проверка. Условие m_wVersion › 89 уже проверено в строке 987. xrserver_objects_alife_items.cpp 989

Этот код очень странный. В этом фрагменте мы видим, что программист либо забыл выражение после if (m_wVersion › 89), либо целую серию else-if. Этот метод требует более тщательного рассмотрения.

void ELogCallback(void *context, LPCSTR txt)
{
  ....
  bool bDlg = ('#'==txt[0])||((0!=txt[1])&&('#'==txt[1]));
  if (bDlg){
    int mt = ('!'==txt[0])||((0!=txt[1])&&('!'==txt[1]))?1:0;
    ....
  }
}

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

• V590 Попробуйте проверить выражение ‘(0 != txt[1]) && (‘#’ == txt[1])’. Выражение является избыточным или содержит опечатку. elog.cpp 29
• V590 Попробуйте проверить выражение ‘(0 != txt[1]) && (‘!’ == txt[1])’. Выражение является избыточным или содержит опечатку. elog.cpp 31

Проверка (0 != txt[1]) является избыточной в выражениях инициализации переменных bDlg и mt.Если мы его опустим, выражение будет намного легче читать.

bool bDlg = ('#'==txt[0])||('#'==txt[1]);
int mt = ('!'==txt[0])||('!'==txt[1])?1:0;

Ошибки в типах данных

float CRenderTarget::im_noise_time;
CRenderTarget::CRenderTarget()
{
  ....
  param_blur           = 0.f;
  param_gray           = 0.f;
  param_noise          = 0.f;
  param_duality_h      = 0.f;
  param_duality_v      = 0.f;
  param_noise_fps      = 25.f;
  param_noise_scale    = 1.f;
  im_noise_time        = 1/100;
  im_noise_shift_w     = 0;
  im_noise_shift_h     = 0;
  ....
}

Предупреждение PVS-Studio:V636 Выражение 1 / 100 было неявно приведено из типа int в тип float. Рассмотрите возможность использования явного приведения типа, чтобы избежать потери дробной части. Пример: двойной A = (двойной)(X)/Y;. gl_rendertarget.cpp 245

Значение выражения 1/100 равно 0, так как это операция целочисленного деления. Чтобы получить значение 0,01f, нам нужно использовать реальный литерал и переписать выражение: 1/100,0f. Хотя, все же есть шанс, что такое поведение должно было быть здесь, и ошибки нет.

CSpaceRestriction::merge(....) const
{
  ....
  LPSTR S = xr_alloc<char>(acc_length);
    
  for ( ; I != E; ++I)
    temp = strconcat(sizeof(S),S,*temp,",",*(*I)->name());
  ....
}

Предупреждение PVS-Studio:V579 Функция strconcat получает в качестве аргументов указатель и его размер. Возможно, это ошибка. Проверьте первый аргумент. space_restriction.cpp 201

Функция strconcat получает размер буфера в качестве первого параметра. Sбуфер объявляется как LPSTR, т. е. как указатель на строку. sizeof(S) будет равен размеру указателя в байтах, а именно sizeof(char *),а не количеству символов в строке.Для оценки длины мы должны использовать strlen(S).

class XRCDB_API MODEL
{
  ....
  u32 status; // 0=ready, 1=init, 2=building
  ....
}
void MODEL::build (Fvector* V, int Vcnt, TRI* T, int Tcnt, 
                   build_callback* bc, void* bcp)
{
  ....
  BTHREAD_params P = { this, V, Vcnt, T, Tcnt, bc, bcp };
  thread_spawn(build_thread,"CDB-construction",0,&P);
  while (S_INIT == status) Sleep(5);
  ....
}

Предупреждение PVS-Studio:V712 Имейте в виду, что компилятор может удалить этот цикл или сделать его бесконечным. Чтобы избежать этого, используйте изменчивые переменные или примитивы синхронизации. xrcdb.cpp 100

Компилятор может удалить проверку S_INIT == status в качестве меры оптимизации, поскольку переменная status не изменяется в цикле. Чтобы избежать такого поведения, мы должны использовать изменчивые переменные или типы синхронизации данных между потоками.

Похожие предупреждения:

• V712 Имейте в виду, что компилятор может удалить этот цикл или сделать его бесконечным. Чтобы избежать этого, используйте изменчивые переменные или примитивы синхронизации. levelcompilerloggerwindow.cpp 23
• V712 Имейте в виду, что компилятор может удалить этот цикл или сделать его бесконечным. Чтобы избежать этого, используйте изменчивые переменные или примитивы синхронизации. levelcompilerloggerwindow.cpp 232

void CAI_Rat::UpdateCL()
{
  ....
  if (!Useful()) {
    inherited::UpdateCL        ();
    Exec_Look                  (Device.fTimeDelta);
    CMonsterSquad *squad = monster_squad().get_squad(this);
    if (squad && ((squad->GetLeader() != this &&
                  !squad->GetLeader()->g_Alive()) ||
                 squad->get_index(this) == u32(-1)))
      squad->SetLeader(this);
    ....
  }
  ....
}

Предупреждение PVS-Studio:V547 Выражение ‘squad-›get_index(this) == u32(- 1)’ всегда ложно. Диапазон значений типа unsigned char: [0, 255]. ai_rat.cpp 480

Чтобы понять, почему это выражение всегда ложно, давайте оценим значения отдельных операндов. u32(-1) равен 0xFFFFFFFF или 4294967295. Тип, возвращаемый методом squad-›get_index(….),являетсяu8,таким образом, его максимальное значение равно 0xFF или 255, что строго меньше u32(-1). Следовательно, результат такого сравнения всегда будет ложным. Этот код можно легко исправить, если мы изменим тип данных на u8:

squad->get_index(this) == u8(-1)

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

namespace ALife
{
  typedef u64 _TIME_ID;
}
ALife::_TIME_ID CScriptActionCondition::m_tLifeTime;
IC bool CScriptEntityAction::CheckIfTimeOver()
{
  return((m_tActionCondition.m_tLifeTime >= 0) &&
         ((m_tActionCondition.m_tStartTime +
           m_tActionCondition.m_tLifeTime) < Device.dwTimeGlobal));
}

Предупреждение PVS-Studio:V547 Выражение ‘m_tActionCondition.m_tLifeTime ›= 0’ всегда истинно. Значение беззнакового типа всегда ›= 0. script_entity_action_inline.h 115

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

То самое предупреждение:

V547 Выражение ‘m_tActionCondition.m_tLifeTime ‹ 0’ всегда ложно. Значение типа без знака никогда не равно ‹ 0. script_entity_action_inline.h 143

ObjectFactory::ServerObjectBaseClass *
CObjectItemScript::server_object    (LPCSTR section) const
{
  ObjectFactory::ServerObjectBaseClass *object = nullptr;
  try {
    object = m_server_creator(section);
  }
  catch(std::exception e) {
    Msg("Exception [%s] raised while creating server object from "
        "section [%s]", e.what(),section);
    return        (0);
  }
  ....
}

Предупреждение PVS-Studio:Нарезка типа V746. Исключение должно быть перехвачено по ссылке, а не по значению. object_item_script.cpp 39

Функция std::exception::what() является виртуальной и может быть переопределена в унаследованных классах. В этом примере исключение перехватывается по значению, поэтому экземпляр класса будет скопирован, а вся информация о полиморфном типе будет утеряна. Доступ к what() в этом случае не имеет смысла. Исключение должно быть поймано по ссылке:

catch(const std::exception& e) {

Разное

void compute_cover_value (....)
{
  ....
  float    value    [8];
  ....
  if (value[0] < .999f) {
    value[0] = value[0];
  }    
  ....
}

Предупреждение PVS-Studio:V570 Переменная value[0] присваивается самой себе. компилятор_cover.cpp 260

Переменная value[0] присваивается самой себе. Непонятно, почему так должно быть. Возможно, ему следует присвоить другое значение.

void CActor::g_SetSprintAnimation(u32 mstate_rl,
                                  MotionID &head,
                                  MotionID &torso,
                                  MotionID &legs)
{
  SActorSprintState& sprint = m_anims->m_sprint;
    
  bool jump = (mstate_rl&mcFall)     ||
              (mstate_rl&mcLanding)  ||
              (mstate_rl&mcLanding)  ||
              (mstate_rl&mcLanding2) ||
              (mstate_rl&mcJump);
  ....
}

Предупреждение PVS-Studio:V501 Слева и справа от оператора || одинаковые подвыражения ‘(mstate_rl & mcLanding)’. акторанимация.cpp 290

Скорее всего, у нас есть лишняя проверка mstate_rl & mcLanding, но довольно часто такие предупреждения указывают на ошибку в логике и значения enum, которые не были учтены.

Похожие предупреждения:

• V501 Имеются идентичные подвыражения «HudItemData()» слева и справа от оператора «&&». huditem.cpp 338
• V501 Слева и справа от оператора «||» есть идентичные подвыражения list_idx == e_outfit. uimptradewnd_misc.cpp 392
• V501 Имеются одинаковые подвыражения ‘(D3DFMT_UNKNOWN == fTarget)’ слева и справа от оператора ‘||’. hw.cpp 312

RELATION_REGISTRY::RELATION_MAP_SPOTS::RELATION_MAP_SPOTS()
{
  ....
  spot_names[ALife::eRelationTypeWorstEnemy] = "enemy_location";
  spot_names[ALife::eRelationTypeWorstEnemy] = "enemy_location";
  ....
}

Предупреждение PVS-Studio:V519 Переменной дважды подряд присваиваются значения. Возможно, это ошибка. Проверьте строки: 57, 58. ratio_registry.cpp 58

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

void safe_verify(....)
{
  ....
  printf("FATAL ERROR (%s): failed to verify data\n");
  ....
}

Предупреждение PVS-Studio:V576 Неверный формат. При вызове функции printf ожидается другое количество фактических аргументов. Ожидается: 2. Присутствует: 1. entry_point.cpp 41

В функцию printpf передано недостаточное количество аргументов: формат ‘%s’ показывает, что нужно передать указатель на строку. Такая ситуация может привести к ошибке доступа к памяти и завершению программы.

Вывод

Анализ X-Ray Engine выявил большое количество как избыточного, так и подозрительного кода, а также ошибочных и опасных моментов. Стоит отметить, что статический анализатор отлично помогает в выявлении ошибок на ранних стадиях разработки, что значительно упрощает жизнь программисту и дает больше времени на создание новых версий ваших приложений.

Павел Беликов