🤹 👩🏿‍🔬 🧢 فقدان الذاكرة: The Dark Descent أو كيفية نسيان إصلاح النسخ واللصق 👨🏽‍✈️ 🧖🏿 🦌

قبل إصدار Amnesia: Rebirth ، أصدرت Fractional Games الكود المصدري للعبة Amnesia الأسطورية: The Dark Descent وتكملة لها ، Amnesia: A Machine For Pigs. لماذا لا تستخدم أداة تحليل ثابتة لترى كيف يتم إخفاء الأخطاء الفادحة في دواخل ألعاب الرعب هذه؟

بالنظر إلى Reddit الأخبار التي تفيد بأن الكود المصدري للألعاب " Amnesia: The Dark Descend " و " Amnesia: A Machine for Pigs " قد تم نشرهما ، لم أتمكن من المرور والتحقق من هذا الرمز باستخدام PVS-Studio ، وفي نفس الوقت اكتب حول هذا المقال. لا سيما بالنظر إلى حقيقة أنه في 20 أكتوبر (وفي وقت نشر هذا المقال ، كان قد تم نشره بالفعل) جزء جديد من هذه السلسلة: " فقدان الذاكرة: ولادة جديدة ".

تم إصدار Amnesia: The Dark Descent في عام 2010 وأصبحت لعبة رعب البقاء على قيد الحياة. بصراحة ، لم أتمكن مطلقًا من اجتيازه ، حتى ولو قليلاً ، لأنني ألعب ألعاب الرعب وفقًا لنفس الخوارزمية: الرهان ، الدخول لمدة خمس دقائق ، الخروج من خلال "alt + f4" في أول لحظة زحف وحذف اللعبة.لكنني أحببت مشاهدة المقطع من هذه اللعبة على موقع يوتيوب.

وفي حال لم يكن شخص ما على دراية بـ PVS-Studio حتى الآن ، فهذا محلل ثابت يبحث عن الأخطاء والأماكن المشبوهة في الكود المصدري للبرامج.

أحب بشكل خاص النظر في الكود المصدري للألعاب ، لذلك إذا كنت تتساءل عن الأخطاء التي ارتكبت فيها ، فيمكنك قراءة مقالاتي السابقة. حسنًا ، أو ألق نظرة على مقالات زملائي حول التحقق من الكود المصدري للألعاب.

بعد التحقق ، اتضح أن الكثير من التعليمات البرمجية في "The Dark Descend" و "A Machine For Pigs" متداخلة ، والتقارير الخاصة بالمشروعين كانت متشابهة للغاية. لذا فإن جميع الأخطاء التي سأذكرها أدناه تقريبًا موجودة في كلا المشروعين.

وكانت أكبر طبقة من الأخطاء في جميع أجهزة التحليل التي تم اكتشافها في هذه المشاريع هي طبقة أخطاء "النسخ واللصق". ومن هنا عنوان المقال. السبب الرئيسي لهذه الأخطاء هو " تأثير السطر الأخير ".

حسنًا ، دعنا نبدأ العمل.

أخطاء النسخ واللصق

كان هناك الكثير من الأماكن المشبوهة المشابهة للنسخ غير اللائق. ربما تكون بعض الحالات ناجمة بطريقة ما عن المنطق الداخلي للعبة نفسها. ولكن إذا كانوا يحرجونني أنا والمحلل ، فإن الأمر يستحق على الأقل تقديم تعليق عليهم. بعد كل شيء ، يمكن للمطورين الآخرين أن يكونوا بطيئين كما أنا.

Snippet 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();
}

صورة حتى لا نتجسس بالخطأ على الجواب:

هل يمكن أن تجد الخطأ؟ نعم ، العائد الأخير هو مقارنة باستخدام aObjectDataA على كلا الجانبين. لاحظ أن جميع التعبيرات الموجودة في الكود الأصلي مكتوبة في سطر ، وهنا قمت بوضع الواصلة بحيث يتناسب كل شيء مع عرض السطر تمامًا. تخيل ما سيبحث عنه هذا العيب في نهاية يوم العمل. وسيجدها المحلل مباشرة بعد تجميع التحليل الإضافي وتشغيله.

V501 توجد تعبيرات فرعية متطابقة 'aObjectDataA.mpObject-> GetVertexBuffer ()' على يسار ويمين عامل التشغيل '<'. WorldLoaderHplMap.cpp 1123

نتيجة لذلك ، سيتم العثور على خطأ مشابه تقريبًا في لحظة كتابة الرمز ، بدلاً من الاختباء في أعماق الكود من عدة مراحل QA ، مما يجعل البحث أكثر صعوبة عدة مرات.

مذكرة من الزميل أندري كاربوف. نعم ، هذا "خطأ آخر سطر" تقليدي. ومع ذلك ، يعد هذا أيضًا نمط خطأ كلاسيكي عند مقارنة كائنين. راجع مقالة " الشر يعيش في وظائف مقارنة ".

الجزء 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 تم العثور على جزئين متشابهين من التعليمات البرمجية. ربما يكون هذا خطأ إملائي ويجب استخدام متغير "mvSearcherIDs" بدلاً من "mvAttackerIDs". 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 . لكن التحذير لا يزال يصدر مع ذكر mvAttackerIDs .

على الأرجح، تم نسخ كتلة التعليمات البرمجية "الباحثين" موقعة من كتلة "المهاجمون"، mvAttackerIDs تم استبدال mvSearcherIDs ، ولكن آخر كتلة لم يتغير. نتيجة لذلك ، تستخدم رسالة الخطأ عنصرًا من المصفوفة الخطأ.

لا يؤثر هذا الخطأ على منطق اللعبة ، ولكن بهذه الطريقة يمكنك وضع خنزير خطير على شخص سيضطر إلى تصحيح أخطاء هذا المكان وإضاعة الوقت في العمل باستخدام عنصر mvSearcherIDs الخاطئ .

الجزء 4.

أشار المحلل إلى المكان المشبوه التالي بثلاثة تحذيرات:

V547 تعبير "pEntity == 0" خطأ دائمًا. LuxScriptHandler.cpp 2444
V649 هناك عبارتان "if" بتعبيرات شرطية متطابقة. تحتوي عبارة "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;

  ....
}

تم إصدار تحذير V547 لعملية التحقق الثانية pEntity == NULL . بالنسبة للمحلل ، سيكون هذا التحقق دائمًا خاطئًا ، لأنه إذا كان هذا الشرط صحيحًا ، فسيحدث الخروج من الوظيفة أعلى بسبب الفحص المماثل السابق. تم إصدار

التحذير التالي ، V649 على وجه التحديد لأن لدينا فحصان متطابقان. عادة ، قد لا تكون مثل هذه الحالة خطأ. فجأة ، ينفذ جزء من الكود منطقًا واحدًا ، وفي جزء آخر من الكود ، وفقًا لنفس الفحص ، يجب تنفيذ شيء آخر. لكن في هذه الحالة ، يتكون نص الشيك الأول من الإرجاع، لذا فإن الفحص الثاني ، إذا تبين أن الشرط صحيح ، لن يصل حتى إلى هذه النقطة. من خلال تتبع هذا المنطق ، يقلل المحلل من عدد الرسائل الخاطئة للرموز المشبوهة ويخرجها فقط لمنطق غريب جدًا.

الخطأ المشار إليه في التحذير الأخير مشابه جدًا للمثال السابق. على الأرجح ، تم تكرار جميع عمليات التحقق من الفحص الأول إذا (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 . في إعادة التعيين ، يتم مسح هذه الكتل الثلاثة ، باستثناء المتغيرين الأخيرين من الكتلة الثالثة ، 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;
  ....
}

تم تخصيص القيمة 0 لمتغير mfTimePos . كما في المثال السابق ، لنلق نظرة على إعلان هذا الحقل:

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

يمكنك أن ترى أن كتلة الإعلان هذه تتطابق أيضًا مع ترتيب التخصيص في مقتطف رمز الخطأ ، كما في المثال السابق. هنا ، في التخصيص ، بدلاً من متغير mfLength ، تحصل القيمة على mfTimePos . ولكن هنا لا يمكن تفسير الخطأ بنسخ الكتلة و "تأثير السطر الأخير". قد يكون الأمر أن mfLength لا يحتاج إلى تعيين قيمة جديدة ، لكن هذا الموقع لا يزال مريبًا.

الجزء 6.

أصدر المحلل مجموعة كاملة من التحذيرات لجزء الكود التالي من "فقدان الذاكرة: آلة للخنازير". سأقدم فقط جزءًا من الكود الذي تم إصدار أخطاء من نفس النوع له:

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" كمتغير من النوع المنطقي. 672
V768 يُستخدم ثابت التعداد "eLuxEnemyMoveState_Walking" كمتغير من النوع المنطقي. 680
V768 يُستخدم ثابت التعداد "eLuxEnemyMoveState_Jogging" كمتغير من النوع المنطقي. 688

تكررت سلسلة 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

قد يكون هذا المثال محيرًا. لماذا آخر هو بادئة نفس الأبعد إذا ؟ هل هو الآخر للحالة الخارجية؟ بالاضافة الى ذلك الحين، كنت بحاجة لوضع الأقواس، وإلا آخر يشير إلى السابقة مباشرة إذا .

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 ، الذي يتم إجراء المقارنة به في أقل من ifs ، لا يمكن أن يكون له قيمة أقل من الصفر ، لأنه يتلقى قيمة من طريقة الطول :

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

بعد ذلك ، على الأرجح ، يتمثل جوهر هذه الفحوصات أولاً في التحقق مما إذا كان العنصر mfMoveSpeed أكبر من صفر ، ثم نتحقق من قيمته بالنسبة إلى fForwardSpeed . علاوة على ذلك ، فإن الشرطين الأخيرين يتوافقان مع بعضهما البعض في صياغتهما.

في هذه الحالة ، سيعمل الكود الأصلي على النحو المنشود! لكنها ستجعل بالتأكيد الشخص الذي يأتي لتحريرها / إعادة تشكيلها للغز.

اعتقدت أنني لن أصادف أبدًا رمزًا يشبه هذا. بدافع الاهتمام ، بحثت في قاعدة بياناتنا الخاصة بالأخطاء الموجودة في المشاريع مفتوحة المصدر والموضحة في المقالات. وتم العثور على أمثلة لهذا الخطأ في مشاريع أخرى - يمكنك النظر إليها بنفسك .

ورجاء لا تكتب هكذا حتى لو كنت نفسك واضحا في هذه الحالة. أو الأقواس المتعرجة أو المسافة البادئة الصحيحة ، أو الأفضل ، كلاهما. لا تنغمس في معاناة أولئك الذين يأتون لفهم شفرتك ونفسك في المستقبل ؛)

جزء 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 عن مثل هذه الأماكن المشبوهة والقذرة في رمز هذه اللعبة ، فكم عدد الأخطاء الواضحة من هذا النوع التي يجب توطينها وتصحيحها أثناء عملية التطوير.

استخدم محلل ثابت بانتظام!

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فيرجى استخدام رابط الترجمة: Victoria Khanieva. فقدان الذاكرة: الهبوط المظلم أو كيفية نسيان إصلاح لصق النسخ .

فقدان الذاكرة: The Dark Descent أو كيفية نسيان إصلاح النسخ واللصق

أخطاء النسخ واللصق

أخطاء أخرى

خاتمة

More articles: