Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

Обзор плана Source SDK

Anna | 24.06.2014 | нет комментариев
SourceSource SDK — комплект утилит для создания модификаций на движке Source, разработанный корпорацией Valve. Начальные коды плана были скачены и проверены ещё в конце 2013 года. На новогодних праздниках я планировал написать статью о итогах проверок. Но лень поборола созидание, и я приступил к написанию статьи только когда возвратился на работу. Однако, я думаю, вряд ли за данный период что-то поспело измениться в начальных кодах. Предлагаю вашему вниманию ознакомиться с подозрительными местами, которые я нашёл с поддержкой анализатора кода PVS-Studio.

Что такое Source SDK

Позаимствую изложение плана из Wikipedia:

Source SDK (англ. Software Development Kit — «комплект разработчика приложений») — комплект утилит для создания модификаций на движке Source, даром распространяемый Valve через сеть Steam каждому игрокам, купившим всякую Source-игру от Valve. Данный комплект разрешает редактировать карты на 2-х версиях движка — 15-ой и обновлённой 7-ой (ветхая версия движка, применяемая в Half-Life 2, не применяется из-за совместимости с новой версией).

Сайт плана: http://source.valvesoftware.com/sourcesdk.php

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

Подозрительные выражения

Деление переменных на самих себя

static void DrawPyroVignette(....)
{
  ....
  Vector2D vMaxSize(
   ( float )nScreenWidth / ( float )nScreenWidth /
     NUM_PYRO_SEGMENTS * 2.0f,
   ( float )nScreenHeight / ( float )nScreenHeight /
     NUM_PYRO_SEGMENTS * 2.0f );
  ....
}

PVS-Studio выдаёт предупреждение V501 на дальнейший файл: viewpostprocess.cpp 1888

Обратите внимание, вот на это:

  • ( float )nScreenWidth / ( float )nScreenWidth
  • ( float )nScreenHeight / ( float )nScreenHeight

Это дюже подозрительные выражения. Я затрудняюсь сказать, что тут должно быть написано, но скорее каждого, что-то иное.

Двойственный вызов функции IsJoystickPOVCode()

void TextEntry::OnKeyCodePressed(KeyCode code)
{
  ....
  if ( IsMouseCode(code) || IsNovintButtonCode(code) ||
       IsJoystickCode(code) || IsJoystickButtonCode(code) ||
       IsJoystickPOVCode(code) || IsJoystickPOVCode(code) ||
       IsJoystickAxisCode(code) )
  ....
}

PVS-Studio выдаёт предупреждение V501 на дальнейший файл: textentry.cpp 1639

Два раза вызывается функция ‘IsJoystickPOVCode(code)’. 2-й вызов избыточен либо следовало вызвать иную функцию.

Неизменно ложное условие

unsigned  numbounce = 100;
int ParseCommandLine( int argc, char **argv, bool *onlydetail )
{
  ....
  numbounce = atoi (argv[i]);
  if ( numbounce < 0 )
  {
    Warning(
      "Error: expected non-negative value after '-bounce'\n");
    return 1;
  }
  ....
}

PVS-Studio выдаёт предупреждение V547 на дальнейший файл: vrad.cpp 2412.

Условие «numbounce < 0» никогда не выполнится. Беззнаковая переменная не может быть поменьше нуля.

Необычное сопоставление строк

void CMultiplayRules::DeathNotice( .... )
{
  ....
  else if ( strncmp( killer_weapon_name, "NPC_", 8 ) == 0 )
  ....
}

PVS-Studio выдаёт предупреждение V666 на дальнейший файл: multiplay_gamerules.cpp 860.

Как я понимаю, хотели проверить, что наименование оружия начинается с символов «NPC_». Но тогда данный код содержит опечатку. Вероятно, верный вариант такой:

else if ( strncmp( killer_weapon_name, "NPC_", 4 ) == 0 )

Ошибки при работе с массивами

Неправильное определение размера массива

#define RTL_NUMBER_OF_V1(A) (sizeof(A)/sizeof((A)[0]))
#define _ARRAYSIZE(A)   RTL_NUMBER_OF_V1(A)

int GetAllNeighbors( const CCoreDispInfo *pDisp,
                     int iNeighbors[512] )
{
  ....
  if ( nNeighbors < _ARRAYSIZE( iNeighbors ) )
    iNeighbors[nNeighbors  ] = pCorner->m_Neighbors[i];
  ....
}

PVS-Studio выдаёт предупреждение V511 на дальнейший файл: disp_vrad.cpp 60

Формальный довод «int iNeighbors[512]» это не массив. Это легко указатель. Число ’512′ подсказывает программисту, что, скорее каждого, указатель ссылается на массив из 512 элементов. Но не больше того. Выражение ‘sizeof(iNeighbors)’ нелегально. Оно возвращает не размер массива, а размер указателя. То есть выражение ‘sizeof(iNeighbors)’ будет равно ‘sizeof(void *).

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

template <typename T, size_t N>
char (&ArraySizeHelper(T (&array)[N]))[N];
#define arraysize(array) (sizeof(ArraySizeHelper(array)))

При попытке вычисления размера указателя, возникнет оплошность на этапе компиляции. Такой макрос применяется в плане Chromium. Подробнее про эту магическую конструкцию дозволено прочитать в статье “PVS-Studio vs Chromium“.

Неправильное определение длины строки

typedef struct message_s
{
  ....
  char    *text;
  ....
} message_t;

int CMessageCharsPanel::AddText(....)
{
  ....
  msg->text = new char[ Q_strlen( data )   1 ];
  Assert( msg->text );
  Q_strncpy( msg->text, data, sizeof( msg->text ) );
  ....
}

PVS-Studio выдаёт предупреждение V579 на дальнейший файл: vgui_messagechars.cpp 240

Выражение «sizeof(msg->text)» вычисляет размер указателя, а совсем не длину строки. Скорее каждого, тут следовало написать: Q_strcpy( msg->text, data);

Работа с уничтоженным массивом

static Activity DetermineExpressionMoveActivity(
  CChoreoEvent *event, CAI_BaseNPC *pNPC )
{
  ....
  const char *pszAct = Q_strstr( sParam2, " " );
  if ( pszAct )
  {
    char szActName[256];
    Q_strncpy( szActName, sParam2, sizeof(szActName) );
    szActName[ (pszAct-sParam2) ] = '';
    pszAct = szActName;
  }
  ....
}

PVS-Studio выдаёт предупреждение V507 на дальнейший файл: baseflex.cpp 1326

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

Выход за рубеж массива

#define MAX_WEAPON_SLOTS    6  // hud item selection slots

void CHudWeaponSelection::Paint()
{
  ....
  int xModifiers[] = { 0, 1, 0, -1 };
  int yModifiers[] = { -1, 0, 1, 0 };

  for ( int i = 0; i < MAX_WEAPON_SLOTS;   i )
  {
    ....
    xPos  = ( m_flMediumBoxWide   5 ) * xModifiers[ i ];
    yPos  = ( m_flMediumBoxTall   5 ) * yModifiers[ i ];
  ....
}

PVS-Studio выдаёт предупреждение V557 на дальнейший файл: hud_weaponselection.cpp 632, 633.

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

Небезопасное применение оператора new.

Проверки, не имеющие смысла

void EmitDispLMAlphaAndNeighbors()
{
  ....
  CCoreDispInfo *pDisp = new CCoreDispInfo;
  if ( !pDisp )
  {
    g_CoreDispInfos.Purge();
    return;
  }
  ....
}

PVS-Studio выдаёт предупреждение V668 на дальнейший файл: disp_vbsp.cpp 532.

Если не удаётся сделать объект типа ‘CCoreDispInfo’, то должна быть вызвана функция g_CoreDispInfos.Purge(). Но эта функция вызвана не будет. Еслипроизойдет оплошность выделения памяти, то возникнет исключение std::bad_alloc. Данный код является устаревшим и должен быть модифицирован с учетом изменений в поведении оператора ‘new’.

Другие места, где проверяется, что вернул оператор ‘new’ будут приведены в конце статьи в приложении.

Оператор new в деструкторе

CNewParticleEffect::~CNewParticleEffect(void)
{
  ....
  KeyValues *msg = new KeyValues( "ParticleSystem_Destroy" );
  ....
}

PVS-Studio выдаёт предупреждение V509 на дальнейший файл: particles_new.cpp 92.

Небезопасно применять в деструкторе конструкции, которые могут привести к происхождению исключения. Именно такой конструкцией и является оператор ‘new’. В случае ошибки выделения памяти, он генерирует исключение.

Поясню в чем угроза. Если в программе появляется исключение, начинается свертывание стека, в ходе которого объекты разрушаются путем вызова деструкторов. Если деструктор объекта, разрушаемого при свертывании стека, бросает еще одно исключение, и это исключение покидает деструктор, библиотека C неотлагательно аварийно завершает программу, вызывая функцию terminate().

Опечатки

Оплошность во вложенном цикле

void DrawTeslaSegs(....)
{
  int i;
  ....
  for ( i = 0; i < segments; i   )
  {
    ....
    for ( int j = 0; i < iBranches; j   )
    {
      curSeg.m_flWidth *= 0.5;
    }
    ....
  }
  ....
}

PVS-Studio выдаёт предупреждение V534 на дальнейший файл: beamdraw.cpp 592.

Обратите внимание на 2-й цикл:

for ( int j = 0; i < iBranches; j   )

В условии заключения вложенного цикла применяется переменная ‘i’, относящаяся к внешнему циклу. У меня есть мощное сомнение, что мы имеем дело с опечаткой.

Неправильная инициализация

inline void SetX( float val );
inline void SetY( float val );
inline void SetZ( float val );
inline void SetW( float val );

inline void Init( float ix=0, float iy=0,
                  float iz=0, float iw = 0 ) 
{
  SetX( ix );
  SetY( iy );
  SetZ( iz );
  SetZ( iw );
}

PVS-Studio выдаёт предупреждение V525 на дальнейший файл: networkvar.h 455.

Мне кажется, функция должна была быть написана так:

{
  SetX( ix );
  SetY( iy );
  SetZ( iz );
  SetW( iw );
}

Обратите внимание на конечный вызов функции.

Последствие Copy-Paste

class ALIGN16 FourVectors
{
public:
  fltx4 x, y, z;
  ....
};

FourVectors BackgroundColor;

void RayTracingEnvironment::RenderScene(....)
{
  ....
  intens.x=OrSIMD(AndSIMD(BackgroundColor.x,no_hit_mask),
                  AndNotSIMD(no_hit_mask,intens.x));
  intens.y=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask),
                  AndNotSIMD(no_hit_mask,intens.y));
  intens.z=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask),
                  AndNotSIMD(no_hit_mask,intens.z));

  ....
}

PVS-Studio выдаёт предупреждение V537 на дальнейший файл: trace2.cpp 189.

Скорее каждого, данный код писался с поддержкой Copy-Paste. Глядите, в первой строке применяются члены класса с именем ‘x’. Во 2-й, с именем ‘y’. А в третьей есть как ‘z’, так и ‘y’. Скорее каждого, код должен быть таким:

intens.z=OrSIMD(AndSIMD(BackgroundColor.z,no_hit_mask),
                AndNotSIMD(no_hit_mask,intens.z));

Присваивания различных значений одной переменной

void GetFPSColor( int nFps, unsigned char ucColor[3] )
{
  ....
  int nFPSThreshold1 = 20;
  int nFPSThreshold2 = 15;

  if (IsPC() &&
      g_pMaterialSystemHardwareConfig->GetDXSupportLevel() >= 95)
  {
    nFPSThreshold1 = 60;
    nFPSThreshold1 = 50;
  }
  ....
}

PVS-Studio выдаёт предупреждение V519 на дальнейший файл: vgui_fpspanel.cpp 192.

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

nFPSThreshold1 = 60;
nFPSThreshold2 = 50;

Дрянной конструктор

CAI_ShotRegulator::CAI_ShotRegulator() :
  m_nMinBurstShots(1), m_nMaxBurstShots(1)
{
  m_flMinRestInterval = 0.0f;
  m_flMinRestInterval = 0.0f;
  m_flMinBurstInterval = 0.0f;
  m_flMaxBurstInterval = 0.0f;
  m_flNextShotTime = -1;
  m_nBurstShotsRemaining = 1;
  m_bInRestInterval = false;
  m_bDisabled = false;
}

PVS-Studio выдаёт предупреждение V519 на дальнейший файл: ai_utils.cpp 49.

Опять опечатка, из-за которой:

  1. Переменной m_flMinRestInterval два раза присваивается нулевое значение.
  2. Переменная m_flMaxRestInterval остаётся неинициализированной.

Схожие беды есть в конструкторах классов CEnvTonemapController, CBasePlayerAnimState. Но описывать схожие примеры тоскливо, следственно соответствующие я помещаю в приложение (см. конец статьи).

Неопределённое поведение

Трудные выражения

int m_nNewSequenceParity;
int m_nResetEventsParity;

void C_BaseAnimating::ResetSequenceInfo( void )
{
  ....
  m_nNewSequenceParity = 
    (   m_nNewSequenceParity ) & EF_PARITY_MASK;
  m_nResetEventsParity =
    (   m_nResetEventsParity ) & EF_PARITY_MASK;
  ....
}

PVS-Studio выдаёт предупреждение V567 на дальнейший файл: c_baseanimating.cpp 5301, 5302.

Отчего тут появляется неопределённое поведение и отчего невозможно предсказать значение переменной ‘m_nResetEventsParity’ отлично описано в документации. В изложении есть пример дюже схожего кода.

Сдвиги

inline void SetStyleType( int w, int h, int type )
{
  Assert( type < NUM_EDGE_STYLES );
  Assert( type >= 0 );
  // Clear old value
  m_nPanelBits[ w ][ h ] &= ( ~0x03 << 2 );
  // Insert new value
  m_nPanelBits[ w ][ h ] |= ( type << 2 );
}

PVS-Studio выдаёт предупреждение V610 на дальнейший файл: c_func_breakablesurf.cpp 157.

Сдвиг негативных чисел приводит к неопределённому поведению. В этом коде негативным числом является ‘~0×03′. Подробнее вопрос сдвига негативных чисел я теснее рассматривал в статье “Не зная брода, не лезь в воду. Часть третья“.

Отсутствует воображаемый деструктор

class CFlashlightEffect
{
  ....
  ~CFlashlightEffect();
  ....
};

class CHeadlightEffect : public CFlashlightEffect { .... };

CFlashlightEffect *m_pFlashlight;

C_BasePlayer::~C_BasePlayer()
{
  ....
  delete m_pFlashlight;
}

PVS-Studio выдаёт предупреждение V599 на дальнейший файл: c_baseplayer.cpp 454.

Существует класс CFlashlightEffect. Он имеет не воображаемый деструктор. Впрочем, от этого класса наследуются класс CHeadlightEffect. Вытекающие из этого итоги, думаю, внятны.

Подозрительная арифметика

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

1-й подозрительный фрагмент

void TE_BloodStream(....)
{
  ....
  int      speedCopy = amount;
  ....
  speedCopy -= 0.00001; // so last few will drip
  ....
}

PVS-Studio выдаёт предупреждение V674 на дальнейший файл: c_te_bloodstream.cpp 141.

Дюже сомнительно вычитать из переменной типа ‘int’ значение 0.00001.

2-й подозрительный фрагмент

#define  ON_EPSILON    0.1      
void CPrediction::SetIdealPitch (....)
{
  int    step;
  ....
  step = floor_height[j] - floor_height[j-1];
  if (step > -ON_EPSILON && step < ON_EPSILON)
    continue;
  ....
}

PVS-Studio выдаёт п

Источник: programmingmaster.ru

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB