Накануне выхода игры «Amnesia: Rebirth» вендор «Fractional Games» открыл исходный код легендарной «Amnesia: The Dark Descent» и ее продолжения «Amnesia: A Machine For Pigs». Почему бы не использовать инструмент статического анализа, чтобы увидеть, какие ужасные ошибки скрыты внутри этих культовых игр ужасов?

Увидев новость на Reddit о том, что исходный код игр Amnesia: The Dark Descent и Amnesia: A Machine for Pigs вышел, я не мог пройти мимо и не проверить этот код с помощью PVS- Studio, а заодно написать об этом статью. Тем более, что новая часть этой игровой серии — Амнезия: Возрождение — выходит 20 октября (а на момент публикации статьи игра уже вышла).

Amnesia: The Dark Descent вышла в 2010 году и стала культовой игрой в жанре survival horror. Честно говоря, я так и не смог в нее поиграть, даже немного. Причина в том, что в хорроры я играю по одному алгоритму: установить, запустить на пять минут, выйти по «alt+f4» в первый же жуткий момент и удалить игру. Но мне нравилось смотреть, как эта игра проходит через видео на YouTube.

Для тех, кто еще не знаком с PVS-Studio, это статический анализатор, который ищет ошибки и подозрительные места в исходном коде программ.

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

После проверки выяснилось, что между «Тёмным спуском» и «Машиной для свиней» большое количество кода пересекается, а отчёты по этим двум проектам были очень похожи. Так что почти все ошибки, которые я приведу далее, имеют место в обоих проектах.

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

Давайте перейдем к делу.

Ошибки копирования-вставки

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

Фрагмент 1.

Начнем с примера, где вся функция состоит из сравнения результатов метода и значений полей двух объектов aObjectDataA и aObjectDataB. Я приведу всю функцию для ясности. Попробуйте сами посмотреть, где была допущена ошибка в функции:

static bool SortStaticSubMeshesForBodies(const ....& aObjectDataA,
                                         const ....& aObjectDataB)
{
  //Is shadow caster check
  if(   aObjectDataA.mpObject->GetRenderFlagBit(....)
     != aObjectDataB.mpObject->GetRenderFlagBit(....))
  {
    return  aObjectDataA.mpObject->GetRenderFlagBit(....)
          < aObjectDataB.mpObject->GetRenderFlagBit(....);
  }
  //Material check
  if( aObjectDataA.mpPhysicsMaterial != aObjectDataB.mpPhysicsMaterial)
  {
    return aObjectDataA.mpPhysicsMaterial < aObjectDataB.mpPhysicsMaterial;
  }
  //Char collider or not
  if( aObjectDataA.mbCharCollider  != aObjectDataB.mbCharCollider)
  {
    return aObjectDataA.mbCharCollider < aObjectDataB.mbCharCollider;
  }
  return  aObjectDataA.mpObject->GetVertexBuffer()
        < aObjectDataA.mpObject->GetVertexBuffer();
}

Вот вам картинка, чтобы случайно не подсмотреть ответ:

Вы нашли ошибку? Итак, в последнем return есть сравнение с использованием aObjectDataA с обеих сторон. Обратите внимание, что все выражения в исходном коде были написаны в строке. Здесь я разбил линии так, чтобы все точно умещалось по ширине линии. Только представьте, как тяжело будет искать такой изъян в конце рабочего дня. Тогда как анализатор найдет его сразу после сборки проекта и запуска инкрементального анализа.

V501 Слева и справа от оператора одинаковые подвыражения aObjectDataA.mpObject-›GetVertexBuffer(). WorldLoaderHplMap.cpp 1123

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

Заметка моего коллеги Андрея Карпова. Да, это классическая ошибка эффекта последней строки. Более того, это тоже классический паттерн ошибки, связанный со сравнением двух объектов. См. статью «Зло в функциях сравнения».

Фрагмент 2.

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

Вот скриншот кода для ясности.

Вот как выглядит предупреждение:

V501 Слева и справа от оператора || одинаковые подвыражения lType == eLuxJournalState_OpenNote. LuxJournal.cpp 2262

Анализатор обнаружил ошибку в проверке значения переменной lType. Равенство с одним и тем же элементом перечислителя eLuxJournalState_OpenNote проверяется дважды.

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

if(!(   lType == eLuxJournalState_OpenNote
     || lType == eLuxJournalState_OpenDiary
     || lType == eLuxJournalState_OpenNote
     || lType == eLuxJournalState_OpenNarratedDiary))
  return false;

В таком виде заметить ошибку становится намного проще даже без анализатора.

В любом случае возникает вопрос — не приводит ли такая ошибочная проверка к логическому искажению программы? Ведь может потребоваться проверка какого-то другого значения lType, но проверка пропущена из-за ошибки копирования-вставки. Итак, давайте взглянем на само перечисление:

enum eLuxJournalState
{
  eLuxJournalState_Main,
  eLuxJournalState_Notes,
  eLuxJournalState_Diaries,
  eLuxJournalState_QuestLog,
  eLuxJournalState_OpenNote,
  eLuxJournalState_OpenDiary,
  eLuxJournalState_OpenNarratedDiary,
  eLuxJournalState_LastEnum,
};

Всего три значения со словом «Открыть» в названии. Все три присутствуют в чеке. Скорее всего, здесь нет логического искажения, но вряд ли мы можем знать наверняка. Итак, анализатор либо нашел логическую ошибку, которую разработчик игры мог исправить, либо нашел «некрасивый» написанный фрагмент, который стоило бы переписать для большей элегантности.

Фрагмент 3.

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

V778 Найдено два одинаковых фрагмента кода. Возможно, это опечатка и вместо mvAttackerIDs следует использовать переменную mvSearcherIDs. LuxSavedGameTypes.cpp 615

void cLuxMusicHandler_SaveData::ToMusicHandler(....)
{
  ....
  // Enemies
  //Attackers
  for(size_t i=0; i<mvAttackerIDs.Size(); ++i)
  {
    iLuxEntity *pEntity = apMap
                         ->GetEntityByID(mvAttackerIDs[i]);
    if(....)
    {
      ....
    }
    else
    {
      Warning("....", mvAttackerIDs[i]);
    }
  }
  //Searchers
  for(size_t i=0; i<mvSearcherIDs.Size(); ++i)
  {
    iLuxEntity *pEntity = apMap->GetEntityByID(mvSearcherIDs[i]);
    if(....)
    {
      ....
    }
    else
    {
      Warning("....", mvAttackerIDs[i]);
    }
  }
}

В первом цикле обрабатывается указатель pEntity (полученный через mvAttackerIDs). Если условие не выполняется, выдается отладочное сообщение для тех же mvAttackerID. Однако в следующем цикле, построенном так же, как и в предыдущем разделе кода, pEntity получается с использованием mvSearcherID. При этом предупреждение по-прежнему выдается с упоминанием mvAttackerIDs.

Скорее всего, блок кода с примечанием «Искатели» был скопирован из блока «Атакующие», mvAttackerIDs заменены на mvSearcherIDs, но else блок не менялся. В результате в сообщении об ошибке используется элемент неправильного массива.

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

Фрагмент 4.

Анализатор указывал на очередной подозрительный фрагмент аж тремя предупреждениями:

  • V547 Выражение pEntity == 0 всегда ложно. LuxScriptHandler.cpp 2444
  • V649 Есть два оператора if с одинаковыми условными выражениями. Первый оператор if содержит возврат функции. Это означает, что второе утверждение если бессмысленно. Проверить строки: 2433, 2444. LuxScriptHandler.cpp 2444
  • V1051 Рассмотрите возможность проверки на опечатки. Возможно, здесь следует проверить pTargetEntity. LuxScriptHandler.cpp 2444

Взгляните на код:

void __stdcall cLuxScriptHandler::PlaceEntityAtEntity(....)
{
  cLuxMap *pMap = gpBase->mpMapHandler->GetCurrentMap();
  iLuxEntity *pEntity = GetEntity(....);
  if(pEntity == NULL) return;
  if(pEntity->GetBodyNum() == 0)
  {
    ....
  }
  iPhysicsBody *pBody = GetBodyInEntity(....);
  if(pBody == NULL) return;
  iLuxEntity *pTargetEntity = GetEntity(....);
  if(pEntity == NULL) return;  // <=
  iPhysicsBody *pTargetBody = GetBodyInEntity(....);
  if(pTargetBody == NULL) return;
  ....
}

Для второй проверки pEntity == NULL выдано предупреждение V547. Для анализатора эта проверка всегда будет false, так как если бы это условие было true, функция завершилась бы раньше из-за предыдущей аналогичной проверки.

Следующее предупреждение (V649) было выдано как раз за то, что у нас две одинаковые проверки. Обычно этот случай не может быть ошибкой. Кто знает, может быть одна часть кода реализует ту же логику, а другая часть кода должна делать что-то другое на основе той же проверки. Но в этом случае тело первой проверки состоит из return, так что до второй проверки не дойдет, если условие true. Отслеживая эту логику, анализатор уменьшает количество ложных сообщений для подозрительного кода и выдает их только для очень странной логики.

Ошибка, обозначенная последним предупреждением, по своему характеру очень похожа на предыдущий пример. Скорее всего, все проверки дублировались из первой проверки if(pEntity == NULL), а затем проверяемый объект заменялся на нужный. В случае с объектами pBody и pTargetBody замена была сделана, но объект pTargetEntity был забыт. В результате этот объект не проверяется.

Если немного покопаться в коде рассматриваемого нами примера, то окажется, что такая ошибка не повлияет на производительность программы. Указатель pTargetBody получает значение из функции GetBodyInEntity:

iPhysicsBody *pTargetBody = GetBodyInEntity(pTargetEntity,
                                            asTargetBodyName);

Первый переданный здесь аргумент — это непроверенный указатель, который больше нигде не используется. К счастью, внутри этой функции есть проверка первого аргумента на NULL:

iPhysicsBody* ....::GetBodyInEntity(iLuxEntity* apEntity, ....)
{
  if(apEntity == NULL){
    return NULL;
  }
  ....
}

В результате этот код в итоге работает корректно, хотя и содержит ошибку.

Фрагмент 5.

Еще одно подозрительное место с копипастом!

В этом методе поля объекта класса cLuxPlayer обнуляются.

void cLuxPlayer::Reset()
{
  ....
  mfRoll=0;
  mfRollGoal=0;
  mfRollSpeedMul=0; //<-
  mfRollMaxSpeed=0; //<-
  mfLeanRoll=0;
  mfLeanRollGoal=0;
  mfLeanRollSpeedMul=0;
  mfLeanRollMaxSpeed=0;
  mvCamAnimPos =0;
  mvCamAnimPosGoal=0;
  mfRollSpeedMul=0; //<-
  mfRollMaxSpeed=0; //<-
  ....
}

Но по какой-то причине две переменные mfRollSpeedMul и mfRollMaxSpeed дважды обнуляются:

  • V519 Переменной mfRollSpeedMul два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 298, 308. LuxPlayer.cpp 308
  • V519 Переменной mfRollMaxSpeed ​​дважды подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 299, 309. LuxPlayer.cpp 309

Посмотрим на сам класс и на его поля:

class cLuxPlayer : ....
{
  ....
private:
  ....
  float mfRoll;
  float mfRollGoal;
  float mfRollSpeedMul;
  float mfRollMaxSpeed;
  float mfLeanRoll;
  float mfLeanRollGoal;
  float mfLeanRollSpeedMul;
  float mfLeanRollMaxSpeed;
  cVector3f mvCamAnimPos;
  cVector3f mvCamAnimPosGoal;
  float mfCamAnimPosSpeedMul;
  float mfCamAnimPosMaxSpeed;
  ....
}

Интересно, что есть три похожих блока переменных с похожими именами: mfRoll, mfLeanRoll и mvCamAnimPos. При Reset эти три блока обнуляются, за исключением двух последних переменных из третьего блока, mfCamAnimPosSpeedMul и mfCamAnimPosMaxSpeed. Просто на месте этих двух переменных встречаются дублирующиеся присваивания. Скорее всего, все эти присваивания были скопированы из первого блока присваивания, а затем имена переменных заменены на нужные.

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

Фрагмент 5.5.

Код очень похож на предыдущий. Сразу приведу фрагмент кода и предупреждение от анализатора на него.

V519 Переменной mfTimePos два раза подряд присваиваются значения. Возможно, это ошибка. Контрольные строки: 49, 53. AnimationState.cpp 53

cAnimationState::cAnimationState(....)
{
  ....
  mfTimePos = 0;
  mfWeight = 1;
  mfSpeed = 1.0f;
  mfBaseSpeed = 1.0f;
  mfTimePos = 0;
  mfPrevTimePos=0;
  ....
}

Переменной mfTimePos дважды было присвоено значение 0. Как и в предыдущем примере, займемся объявлением этого поля:

class cAnimationState
{
  ....
private:
  ....
  //Properties of the animation
  float mfLength;
  float mfWeight;
  float mfSpeed;
  float mfTimePos;
  float mfPrevTimePos;
  ....
}

Вы можете заметить, что этот блок объявлений также соответствует порядку присваивания в ошибочном фрагменте кода, как и в предыдущем примере. Здесь в задании mfTimePos получает значение вместо переменной mfLength. За исключением этого случая, ошибка не может быть объяснена копированием блока и «эффектом последней строки». mfLength, возможно, не нужно присваивать новое значение, но этот фрагмент кода все еще вызывает сомнения.

Фрагмент 6.

Эта часть кода из «Amnesia: A Machine For Pigs» заставила анализатор выдать кучу предупреждений. Приведу лишь часть кода, вызвавшего ошибки такого же рода:

void cLuxEnemyMover::UpdateMoveAnimation(float afTimeStep)
{
  ....
  if(prevMoveState != mMoveState)
  {
    ....
    //Backward
    if(mMoveState == eLuxEnemyMoveState_Backward)
    {
      ....
    }
    ....
    //Walking
    else if(mMoveState == eLuxEnemyMoveState_Walking)
    {
      bool bSync =    prevMoveState == eLuxEnemyMoveState_Running
                   || eLuxEnemyMoveState_Jogging
                    ? true : false;
      ....
    }
    ....
  }
}

Где здесь ошибка?

Вот предупреждения анализатора:

  • V768 Константа перечисления eLuxEnemyMoveState_Jogging используется как переменная логического типа. LuxEnemyMover.cpp 672
  • V768 Константа перечисления eLuxEnemyMoveState_Walking используется как переменная логического типа. LuxEnemyMover.cpp 680
  • V768 Константа перечисления eLuxEnemyMoveState_Jogging используется как переменная логического типа. LuxEnemyMover.cpp 688

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

Рассмотрим строку, на которую указывает анализатор:

bool bSync =    prevMoveState == eLuxEnemyMoveState_Running
             || eLuxEnemyMoveState_Jogging
              ? true : false;

Неудивительно, что в такое выражение, изначально написанное в строке, закралась ошибка. И я уверен, что вы уже это заметили. Элемент перечисления eLuxEnemyMoveState_Jogging ни с чем не сравнивается, но проверяется его значение. Скорее всего имелось в виду выражение prevMoveState == eLuxEnemyMoveState_Jogging.

Такая ошибка может показаться вполне безобидной. Но в другой статье про проверку Bullet Engine, среди коммитов проекта, я нашел фикс ошибки такого же рода, которая приводила к тому, что силы применялись к объектам не с той стороны. Что касается данного случая, то эта ошибка была допущена несколько раз. Ну и заметьте, тернарное условие совершенно бессмысленно, так как к булевым результатам логических операторов оно будет применяться в последнюю очередь.

Фрагмент 7.

Наконец, последняя пара примеров ошибок копирования-вставки. На этот раз снова в условном выражении. Анализатор выдал предупреждение на этот участок кода:

void iParticleEmitter::SetSubDivUV(const cVector2l &avSubDiv)
{
  //Check so that there is any subdivision
  // and that no sub divison axis is
  //equal or below zero
  if( (avSubDiv.x > 1 || avSubDiv.x > 1) && (avSubDiv.x >0 && avSubDiv.y >0))
  {
    ....
  }
  ....
}

Думаю, что в таком отдельном от всего кода фрагменте довольно легко заметить несуразное место. Тем не менее, ошибку удалось скрыть от разработчиков этой игры.

Анализатор выдал следующее сообщение:

V501 Слева и справа от оператора || находятся одинаковые подвыражения: avSubDiv.x › 1 || avSubDiv.x › 1 ParticleEmitter.cpp 199

Вторая скобка в условии указывает, что оба поля x и y проверены. Но в первой скобке почему-то пропущен этот пункт и проверяется только поле x. К тому же, судя по комментарию к обзору, оба поля должны были быть проверены. Так что здесь сработал не «эффект последней строки», а скорее «эффект первой строки», так как в первой скобке автор забыл заменить доступ к полю x на доступ к y поле.

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

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

if(   (avSubDiv.x > 1 || avSubDiv.x > 1)
   && (avSubDiv.x > 0 && avSubDiv.y > 0))

Фрагмент 7.5.

Абсолютно аналогичная ошибка была обнаружена в другом месте:

static bool EdgeTriEqual(const cTriEdge &edge1, const cTriEdge &edge2)
{
  if(edge1.tri1 == edge2.tri1 && edge1.tri2 == edge2.tri2)
    return true;
  if(edge1.tri1 == edge1.tri1 && edge1.tri2 == edge2.tri1)
    return true;
  return false;
}

У вас была возможность увидеть, где он прятался? Мы не зря уже разобрались с таким количеством примеров :)

Анализатор выдал предупреждение:

V501 Слева и справа от оператора == одинаковые подвыражения: edge1.tri1 == edge1.tri1 Math.cpp 2914

Разберем этот фрагмент одну часть за другой. Очевидно, первая проверка проверяет равенство полей edge1.tri1 и edge2.tri2, а заодно и равенство edge1.tri2 и edge2.tri2:

edge1.tri1 -> edge2.tri1
edge1.tri2 -> edge2.tri2

Во второй проверке, судя по правильной части проверки «edge1.tri2 == edge2.tri1», равенство этих полей нужно было проверять крест-накрест:

Но вместо проверки edge1.tri1 == edge2.tri2 была бессмысленная проверка edge1.tri1 == edge1.tri1. Кстати, все это есть в функции, я ничего не удалял. Еще такая ошибка залезла в код.

Другие ошибки

Фрагмент 1.

Вот следующий фрагмент кода с исходными отступами.

void iCharacterBody::CheckMoveCollision(....)
{
  ....
  /////////////////////////////////////
  //Forward velocity reflection
  //Make sure that new velocity points in the right direction
  //and that it is not too large!
  if(mfMoveSpeed[eCharDir_Forward] != 0)
  {
    vForwardVel = ....;
    float fForwardSpeed = vForwardVel.Length();
    if(mfMoveSpeed[eCharDir_Forward] > 0)
      if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
        mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
    else
      if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed)
        mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
  }
  ....
}

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

Этот пример может сбить с толку. Почему else имеет тот же отступ, что и внешний, на уровне if? Подразумевается ли, что else соответствует самому внешнему условию? Что ж, тогда нужно правильно расставить фигурные скобки, иначе else ссылается на правое переднее if.

if(mfMoveSpeed[eCharDir_Forward] > 0)
{
  if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed)
    mfMoveSpeed[eCharDir_Forward] = fForwardSpeed;
}
else if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed) 
{
  mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed;
}

Или это не так? При написании этой статьи я несколько раз менял свое мнение о том, какой вариант последовательности действий для этого кода наиболее вероятен.

Если копнуть немного глубже в этот код, то окажется, что переменная fForwardSpeed, которая сравнивается в нижнем if, не может иметь значение меньше нуля, так как он получает значение из метода Length:

inline T Length() const
{
  return sqrt( x * x + y * y +  z * z);
}

Тогда, скорее всего, смысл этих проверок в том, что мы сначала проверяем, больше ли элемент mfMoveSpeed нуля, а затем проверяем его значение относительно fForwardSpeed. Более того, последние два оператора if соответствуют друг другу по формулировкам.

В этом случае исходный код будет работать так, как задумано! Но это определенно заставит задуматься того, кто придет редактировать/рефакторить его.

Я думал, что никогда не сталкивался с таким кодом. Ради интереса посмотрел нашу коллекцию ошибок, найденных в open-source проектах и ​​описанных в статьях. Примеры этой ошибки встречались и в других проектах — вы можете посмотреть их сами.

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

Фрагмент 2.

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

Взгляните на код:

bool cBinaryBuffer::DecompressAndAdd(char *apSrcData, size_t alSize)
{
  ....
  ///////////////////////////
  // Init decompression
  int ret = inflateInit(&zipStream);
  if (ret != Z_OK) return false;
  ///////////////////////////
  // Decompress, chunk by chunk 
  do
  {
    //Set current output chunk
    zipStream.avail_out = lMaxChunkSize;
    ....
    //Decompress as much as possible to current chunk
    int ret = inflate(&zipStream, Z_NO_FLUSH);
    if(ret != Z_OK && ret != Z_STREAM_END)
    {
      inflateEnd(&zipStream);
      return false;
    }
    ....
  }
  while (zipStream.avail_out == 0 && ret != Z_STREAM_END);
  ....
  return true;
}

V711 Опасно создавать внутри цикла локальную переменную с тем же именем, что и у переменной, управляющей этим циклом. BinaryBuffer.cpp 371

Итак, у нас есть переменная ret, которая управляет выходом из цикла do-while. Но внутри этого цикла вместо присвоения нового значения этой внешней переменной объявляется новая переменная с именем ret. В результате он переопределяет внешнюю переменную ret, и переменная, проверяемая в условии цикла, никогда не изменится.

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

Вывод

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

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

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

Регулярно используйте статический анализатор!