記憶喪失：ダークディセントまたはコピー＆ペーストの修正を忘れる方法

Amnesia：Rebirthのリリースに先立ち、Fractional Gamesは、伝説的なAmnesia：The Dark Descentとその続編であるAmnesia：A Machine ForPigsのソースコードをリリースしました。静的分析ツールを使用して、これらのカルトホラーゲームの内部にひどい間違いがどのように隠されているかを確認してみませんか？



Redditで、ゲーム「Amnesia：The DarkDescend」と「Amnesia：A Machine for Pigs」のソースコードが公開されたというニュースを見て、PVS-Studioを使用してこのコードを通り過ぎて確認することができず、同時にこの記事について。特に、10月20日（そしてこの記事の発行時点ではすでに公開されていた）に、このシリーズの新しい部分である「記憶喪失：再生」という事実を考慮します。



Amnesia：The Dark Descentは2010年にリリースされ、カルトサバイバルホラーゲームになりました。正直なところ、私は同じアルゴリズムに従ってホラーゲームをプレイしているので、少しでもそれを通過することができませんでした。賭け、5分間入力し、最初のクリープの瞬間に「alt + f4」で終了し、ゲームを削除します。しかし、私はYouTubeでこのゲームのパッセージを見るのが好きでした。



そして、誰かがまだPVS-Studioに精通していない場合に備えて、これはプログラムのソースコード内のエラーや疑わしい場所を探す静的アナライザーです。

私は特にゲームのソースコードを調べるのが好きなので、どのような間違いがあったのか疑問に思っている場合は、以前の記事を読むことができます。または、ゲームのソースコードの確認に関する同僚の記事を見てください。



確認したところ、「TheDarkDescend」と「AMachineFor Pigs」のコードの多くが重複しており、2つのプロジェクトのレポートは非​​常に似ていることがわかりました。したがって、以下で引用するエラーのほとんどすべてが両方のプロジェクトに含まれています。



そして、これらのプロジェクトで検出されたすべてのアナライザーのエラーの最大の層は、「コピー＆ペースト」エラーの層でした。したがって、記事のタイトル。これらのエラーの主な理由は、「最後の行の影響」です。



さて、ビジネスに取り掛かりましょう。

コピー＆ペーストエラー

不注意なコピーに似た不審な場所がたくさんありました。場合によっては、ゲーム自体の内部ロジックが原因である可能性があります。しかし、彼らが私と分析者の両方を当惑させたのであれば、少なくとも彼らについてコメントする価値がありました。結局のところ、他の開発者は私と同じように機知に富んでいる可能性があります。



スニペット1.



関数全体が2つのオブジェクト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();
}

誤って答えをスパイしないように、写真：





エラーを見つけられますか？はい、最後の戻り値は、両側でaObjectDataAを使用した比較です。元のコードのすべての式が1行で記述されていることに注意してください。ここでは、すべてが行幅に正確に収まるようにハイフネーションを作成しました。そのような欠陥が一日の終わりに何を探すか想像してみてください。そして、アナライザーは、増分分析を組み立てて実行した直後にそれを見つけます。



V501 '<'演算子の左側と右側に同一のサブ式 'aObjectDataA.mpObject-> GetVertexBuffer（）'があります。 WorldLoaderHplMap.cpp 1123



その結果、このようなエラーは、いくつかのQAステージからコードの深部に隠れるのではなく、コードを記述したほぼ瞬間に検出され、検索が何倍も困難になります。

同僚のAndreyKarpovによるメモ。はい、これは古典的な「最終行エラー」です。ただし、これは2つのオブジェクトを比較するときの典型的なエラーパターンでもあります。記事「比較機能における悪の生活」を参照してください。

フラグメント2。



警告の原因となったコードを見てみましょう。





わかりやすくするために、スクリーンショット付きのコードを示します。



警告自体は次のようになります。



V501同じ部分式「LTYPE == eLuxJournalState_OpenNote」の左にと「||」の右側にあります。オペレーター。 LuxJournal.cpp 2262



アナライザーは、lType変数の値のチェック中にエラーがあることを検出しました。同等性は、eLuxJournalState_OpenNote列挙子の同じメンバーで2回チェックされます。



まず、読みやすさを向上させるために、このような条件を「表形式」で記述してほしい。ミニブック「プログラミング、リファクタリング、その他すべての最大の問題」の第N13章を参照してください。

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つだけです。そして、3つすべてがチェックに含まれています。おそらく、ここには論理の歪みはありませんが、それでも確実に言うことは不可能です。そのため、アナライザーは、ゲーム開発者が修正できる論理エラーを見つけるか、より明確にするために書き直すべき「醜い」書かれた場所を見つけました。



フラグメント3。



次のケースは、一般に、コピーと貼り付けのエラーの最も明白な例です。



V7782つの類似したコードフラグメントが見つかりました。おそらく、これはタイプミスであり、「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、条件が満たされない場合は、デバッグ用のメッセージが同じに対して発行さmvAttackerIDs。ただし、コードの前のセクションとまったく同じスタイルの次のループでは、pEntityはmvSearcherIDsを使用して取得されます。ただし、警告はmvAttackerIDの言及とともに発行されます。



ほとんどの場合、「サーチャー」に署名したコードブロックは、「攻撃者」ブロックからコピーされた、mvAttackerIDsを置換した後mvSearcherIDsが、他のブロックは変更されませんでした。その結果、エラーメッセージは間違った配列の要素を使用します。



このエラーはゲームのロジックには影響しませんが、この方法で、この場所をデバッグし、間違ったmvSearcherIDs要素で作業する時間を無駄にする必要がある人に深刻な豚を置くことができます。





フラグメント4。



アナライザーは、次の疑わしい場所を3つの警告とともに示しました。

• V547式 'pEntity == 0'は常にfalseです。LuxScriptHandler.cpp 2444
• V649同一の条件式を持つ2つの「if」ステートメントがあります。最初の「if」ステートメントには関数returnが含まれています。これは、2番目の「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;

  ....
}

2番目のチェックpEntity == NULLに対してV547 警告が発行されました。アナライザーの場合、このチェックは常にfalseになります。これは、この条件がtrueの場合、前の同様のチェックのために関数がより高く終了するためです。 次の警告であるV649は、2つの同一のチェックがあるために発行されました。通常、このような場合はエラーではない可能性があります。突然、コードの一部が1つのロジックを実装し、コードの別の部分で、同じチェックに従って、別の何かを実装する必要があります。ただし、この場合、最初のチェックの本文は返品で構成されます



、したがって、条件が真であることが判明した場合、2番目のチェックはポイントに到達することさえありません。このロジックを追跡することにより、アナライザーは疑わしいコードの誤ったメッセージの数を減らし、非常に奇妙なロジックに対してのみそれらを出力します。



最後の警告で示されたエラーは、本質的に前の例と非常によく似ています。ほとんどの場合、（pEntity == NULL）の場合、最初のチェックからすべてのチェックが複製され、チェックされたオブジェクトが必要なオブジェクトに置き換えられました。以下の場合にpBodyとpTargetBodyオブジェクト、交換がなされたが、pTargetEntityのオブジェクトをし忘れました。その結果、このオブジェクトはチェックされません。



私たちが検討している例では、コードをもう少し深く掘り下げてみると、そのようなエラーは一般にプログラムのコースに影響を与えないことがわかります。pTargetBodyポインターは、GetBodyInEntity関数からその値を取得します。

iPhysicsBody *pTargetBody = GetBodyInEntity(pTargetEntity,
                                            asTargetBodyName);

ここでの最初の引数は、以前にチェックされていないポインタであり、他の場所では使用されていません。そして幸いなことに、この関数の中にはNULLの最初の引数のチェックがあります：

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

その結果、このコードにはエラーが含まれていますが、最終的には正しく機能します。



フラグメント5。



そして、コピー＆ペーストでもう1つの疑わしい場所！





このメソッドは、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; //<-
  ....
}

しかし、何らかの理由で、2つの変数mfRollSpeedMulとmfRollMaxSpeedが2回無効になります。

• V519'mfRollSpeedMul '変数には、2回連続して値が割り当てられます。おそらくこれは間違いです。チェックライン：298、308。LuxPlayer.cpp 308
• V519'mfRollMaxSpeed '変数には、2回連続して値が割り当てられます。おそらくこれは間違いです。チェックライン：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;
  ....
}

興味深いことに、関連する名前を持つ変数の3つの類似したブロックがあります：mfRoll、mfLeanRoll、およびmvCamAnimPos。でリセット、これらの三つのブロックは、第3のブロックからの最後の2つの変数を除いて、クリアされmfCamAnimPosSpeedMulとmfCamAnimPosMaxSpeed。そして、重複した割り当てが見つかるのは、まさにこれら2つの変数の代わりです。ほとんどの場合、これらの割り当てはすべて最初の割り当てブロックからコピーされ、次に変数名が必要な名前に置き換えられました。



2つの欠落している変数をゼロに設定するべきではなかった可能性がありますが、反対の可能性も非常に高くなります。そして、再割り当ては明らかにこのコードの維持に役立ちません。ご覧のとおり、同じアクションの長いフットクロスでは、そのようなエラーに気付かない場合があり、アナライザーがここで役立ちます。



フラグメント5.5。



この例は前の例と非常によく似ています。コードスニペットとそれに対するアナライザーの警告を提供します。



V519'mfTimePos '変数には、2回連続して値が割り当てられます。おそらくこれは間違いです。チェックライン：49、53。AnimationState.cpp 53

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

mfTimePos 変数には値0が2回割り当てられています。前の例と同様に、このフィールドの宣言を見てみましょう。

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

前の例のように、この宣言ブロックもエラーコードスニペットの割り当て順序と一致していることがわかります。ここで、割り当てでは、mfLength変数の代わりに、値はmfTimePosを取得します。しかし、ここでは、ブロックと「最後の行の効果」をコピーしてもエラーを説明できません。これは、のかもしれmfLengthは、新しい値を割り当てる必要がありませんが、この場所はまだ疑わしいです。



フラグメント6.



アナライザーは、「Amnesia：A MachineForPigs」の次のコードフラグメントに対して大量の警告を発行しました。同じ種類のエラーが発行されたコードの一部のみを示します。

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チェーンが元のコードで繰り返され、その後、これらの警告は、ifの各本体に対して発行されました。



パーサーが指す線について考えてみます。

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

そのような表現に誤りが忍び込んだのも当然のことであり、それもオリジナルの行に書かれています。そして、あなたはおそらくすでにそれに気づいていました。eLuxEnemyMoveState_Jogging列挙のメンバーは何とも比較されず、その値がチェックされます。ほとんどの場合、式 'prevMoveState == eLuxEnemyMoveState_Jogging'が暗示されていました。



そのような間違いは完全に無害に見えるかもしれません。しかし、他の記事で、Bullet Engineのチェックについて、プロジェクトへのコミットの中で、バグ修正を見つけました同じ種類のもので、反対側からオブジェクトに力が加えられます。そしてこの場合、この間違いは何度かありました。さて、私はまた、三元条件は完全に無意味であることに注意します。なぜなら、それは最後に、論理演算子のブール結果に対して満たされるからです。



フラグメント7。



最後に、コピーと貼り付けのエラーの最後の2つの例。今回も条件付きステートメントで。アナライザーは、次のコードに対して警告を発行しました。

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



条件の2番目の括弧は、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.cpp2914



これを分析しましょう順番にフラグメントします。明らかに、最初のチェックはフィールドedge1.tri1とedge2.tri2の同等性をチェックし、同時にedge1.tri2とedge2.tri2の同等性をチェックします。

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

そして、2番目のチェックでは、チェック '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.cpp1591



この例は混乱を招く可能性があります。なぜ他の場合は外側と同じようにインデントされますか？それは外的条件のための他のものですか？それでは、あなたはそれ以外の場合は、括弧を配置する必要があり、他直前のを指している場合。

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

か否か？この記事を書く過程で、私はこのコードの考えられたアクションのシーケンスのどのバリアントが最も可能性が高いかについて何度か意見を変えました。



このコードをもう少し深く掘り下げてみると、より低いifで比較が実行されるfForwardSpeed変数は、Lengthメソッドから値を受け取るため、ゼロ未満の値を持つことはできません。

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

次に、おそらく、これらのチェックの本質は、最初に要素mfMoveSpeedがゼロより大きいかどうかをチェックし、次にfForwardSpeedと比較してその値をチェックすることです。さらに、最後の2つのifは、その文言で互いに対応しています。



この場合、元のコードは意図したとおりに機能します。しかし、それは間違いなくそれを編集/リファクタリングするようになる人を困惑させるでしょう。



このようなコードに出くわすことはないと思いました。興味深いことに、私はオープンソースプロジェクトで見つかり、記事で説明されているエラーのデータベースを調べました。そして、このエラーの例は他のプロジェクトで見つかりました-あなたはそれらを自分で見ることができます。



そして、この場合、あなた自身が明確であっても、そのように書かないでください。または、中括弧または正しいインデント、あるいはその両方。あなたのコードを理解するようになった人々、そして将来彼ら自身を苦しめることに突入しないでください;）



フラグメント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



したがって、do-whileループからの終了を制御する変数retがあります。ただし、このループ内では、この外部変数に新しい値を割り当てる代わりに、retという名前の新しい変数が宣言されます。その結果、外部変数retをオーバーライドし、ループ条件でチェックされる変数が変更されることはありません。



不幸な状況の組み合わせでは、そのようなサイクルは無限になる可能性があります。ほとんどの場合、この場合、このコードは内部変数retの値をチェックする内部条件を保存します そして、関数を終了します。

結論

多くの場合、開発者は静的分析を定期的にではなく、長い休憩をとって使用します。または、アナライザーを介してプロジェクトを1回実行することもできます。このアプローチの結果として、アナライザーは多くの場合、深刻なものを検出しないか、検討している例のようなものを検出します。これは、おそらくゲームの機能に特に影響を与えませんでした。アナライザーは特に有用ではないようです。まあ、彼はそのような場所を見つけましたが、それでも機能します。



実際には、エラーはマスクされていませんが、明らかにプログラムのバグにつながっている同様の場所は、長時間のデバッグ、テストの実行、およびテスト部門によってすでに修正されています。その結果、プロジェクトをチェックするとき、アナライザーは、まったく現れていない問題のみを表示します。そのような問題の中には、プログラムの動作に実際に影響を与える深刻な瞬間もあることがありますが、プログラムがそのパスをたどる可能性は低く、したがってこのエラーは開発者にはわかりませんでした。



したがって、静的分析の有用性を評価することは、通常の使用後にのみ非常に重要です。PVS-Studioを1回実行しただけで、このゲームのコードにこのような疑わしい不正確な場所が見つかった場合、開発プロセス中にこの種の明らかなエラーをローカライズして修正する必要がありました。



静的アナライザーを定期的に使用してください！

この記事を英語を話す聴衆と共有したい場合は、翻訳リンクを使用してください：VictoriaKhanieva。記憶喪失：ダークディセントまたはコピーペーストの修正を忘れる方法。