VVVVVV ؟؟ VVVVVV !!! :)

إذا كنت تقرأ هذا النص ، فأنت تعتقد أن هناك شيئًا ما خطأ في العنوان الرئيسي أو أنك رأيت اسم لعبة كمبيوتر مألوفة. VVVVVV هي لعبة منهاج مستقل سرقت قلوب العديد من اللاعبين بسبب بساطتها الخارجية اللطيفة وبدون تعقيد داخلي لطيف. قبل بضعة أيام ، بلغ عمر VVVVVV 10 سنوات ، واحتفل مؤلف اللعبة - Terry Cavanagh - بهذا العيد بنشر الكود المصدر. ما الأشياء المذهلة التي تختبئ بها؟ اقرأ الإجابة في هذا المقال.

مقدمة

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

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

على أي حال ، دعنا نعود إلى الكود المصدري ، المنشور على شرف ذكرى اللعبة .

في الوقت الحالي ، أنا مطور برنامج PVS-Studio ، وهو محلل أكواد ثابت لـ C و C ++ و C # و Java. بالإضافة إلى التطوير المباشر ، فإننا نشارك أيضًا في الترويج لمنتجاتنا. بالنسبة لنا ، واحدة من أفضل الطرق للقيام بذلك هي كتابة مقالات حول التحقق من المشاريع مفتوحة المصدر. يحصل القراء على مقالات جذابة حول موضوعات البرمجة ، ونحصل على فرصة لإظهار قدرات PVS-Studio. لذلك عندما سمعت عن فتح شفرة مصدر VVVVVV ، لم أستطع تجاوزها.

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

نظرة عامة على تحذيرات المحلل

تحذير 1

V512 سيؤدي استدعاء وظيفة 'sprintf' إلى تجاوز سعة 'searchSearch' العازلة. FileSystemUtils.cpp 307

#define MAX_PATH 260 .... void PLATFORM_migrateSaveData(char *output) { char oldLocation[MAX_PATH]; char newLocation[MAX_PATH]; char oldDirectory[MAX_PATH]; char fileSearch[MAX_PATH]; .... /* Same place, different layout. */ strcpy(oldDirectory, output); sprintf(fileSearch, "%s\\*.vvvvvv", oldDirectory); .... } 

كما ترون ، فإن سلاسل البحث عن الملفات و oldDirectory هي بنفس الحجم: 260 حرفًا. بعد كتابة محتويات سلسلة oldDirectory في سلسلة التنسيق (الوسيطة sprintf الثالثة) ، ستبدو كما يلي:

 <i>contents_oldDirectory\*.vvvvvv</i> 

هذا الخط أطول من 9 أحرف من القيمة الأصلية لـ oldDirectory . هذا هو تسلسل الأحرف الذي سيتم كتابته في fileSearch . ماذا يحدث إذا كان طول السلسلة oldDirectory أكثر من 251؟ ستكون السلسلة الناتجة أطول مما يمكن أن يحتويه fileSearch ، مما يؤدي إلى انتهاك حدود الصفيف. ما البيانات الموجودة في ذاكرة الوصول العشوائي يمكن أن تتضرر والنتيجة التي ستؤدي إليها هي مسألة بلاغية :)

تحذير 2

V519 يتم تعيين قيم "الخلفية" مرتين على التوالي. ربما هذا خطأ. خطوط التحقق: 1367 ، 1373. Map.cpp 1373

 void mapclass::loadlevel(....) { .... case 4: //The Warpzone tmap = warplevel.loadlevel(rx, ry, game, obj); fillcontent(tmap); roomname = warplevel.roomname; tileset = 1; background = 3; // <= dwgfx.rcol = warplevel.rcol; dwgfx.backgrounddrawn = false; warpx = warplevel.warpx; warpy = warplevel.warpy; background = 5; // <= if (warpy) background = 4; if (warpx) background = 3; if (warpx && warpy) background = 5; break; .... } 

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

 void Game::loadquick(....) { .... else if (pKey == "frames") { frames = atoi(pText); frames = 0; } .... } 

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

تحذير 3

تم إنشاء كائن V808 "pKey" من النوع "basic_string" ولكن لم يتم استخدامه. editor.cpp 1866

 void editorclass::load(std::string &_path) { .... std::string pKey(pElem->Value()); .... if (pKey == "edEntities") { int i = 0; for (TiXmlElement *edEntityEl = pElem->FirstChildElement(); edEntityEl; edEntityEl = edEntityEl->NextSiblingElement()) { std::string pKey(edEntityEl->Value()); // <= //const char* pText = edEntityEl->GetText() ; if (edEntityEl->GetText() != NULL) { edentity[i].scriptname = std::string(edEntityEl->GetText()); } edEntityEl->QueryIntAttribute("x", &edentity[i].x); edEntityEl->QueryIntAttribute("y", &edentity[i].y); edEntityEl->QueryIntAttribute("t", &edentity[i].t); edEntityEl->QueryIntAttribute("p1", &edentity[i].p1); edEntityEl->QueryIntAttribute("p2", &edentity[i].p2); edEntityEl->QueryIntAttribute("p3", &edentity[i].p3); edEntityEl->QueryIntAttribute("p4", &edentity[i].p4); edEntityEl->QueryIntAttribute("p5", &edentity[i].p5); edEntityEl->QueryIntAttribute("p6", &edentity[i].p6); i++; } EditorData::GetInstance().numedentities = i; } .... } 

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

وبالتالي ، إذا أشرت إلى قيمة سلسلة pKey خارج حلقة for ، فستحصل على القيمة المساوية لـ pElem-> Value () ، ولكن عند القيام بالشيء نفسه داخل الحلقة ، ستحصل على القيمة المساوية لـ edEntityEl -> القيمة () . الأسماء المتداخلة هي خطأ فادح إلى حد ما ، والذي قد يكون من الصعب جدًا العثور عليه بمراجعة الكود.

تحذير 4

V805 انخفاض الأداء. من غير الفعال تحديد سلسلة فارغة باستخدام بناء 'strlen (str)> 0'. الطريقة الأكثر فعالية هي التحقق من: str [0]! = '\ 0'. physfs.c 1604

 static char *prefDir = NULL; .... const char *PHYSFS_getPrefDir(const char *org, const char *app) { .... assert(strlen(prefDir) > 0); ... return prefDir; } /* PHYSFS_getPrefDir */ 

وجد المحلل جزءًا من التحسين الدقيق المحتمل. يستخدم الدالة strlen للتحقق مما إذا كانت السلسلة فارغة. هذه الوظيفة تعبر جميع عناصر السلسلة وتتحقق من كل منها للحصول على فاصل لاغٍ ('\ 0'). إذا حصلنا على سلسلة طويلة ، فستتم مقارنة كل حرف مع فاصل فارغ.

لكننا نحتاج فقط إلى التحقق من أن السلسلة فارغة! كل ما عليك فعله هو معرفة ما إذا كانت سلسلة الأحرف الأولى فارغة. لذلك ، لتحسين هذا التحقق داخل التأكيد ، يستحق الكتابة:

 str[0] != '\0' 

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

تحذير 5

لإظهار جوهر خطأ آخر ، لا بد لي من ذكر شظيتين من التعليمات البرمجية هنا: إعلان فئة entclass ومنشئه . لنبدأ بالإعلان:

 class entclass { public: entclass(); void clear(); bool outside(); public: //Fundamentals bool active, invis; int type, size, tile, rule; int state, statedelay; int behave, animate; float para; int life, colour; //Position and velocity int oldxp, oldyp; float ax, ay, vx, vy; int cx, cy, w, h; float newxp, newyp; bool isplatform; int x1, y1, x2, y2; //Collision Rules int onentity; bool harmful; int onwall, onxwall, onywall; //Platforming specific bool jumping; bool gravity; int onground, onroof; int jumpframe; //Animation int framedelay, drawframe, walkingframe, dir, actionframe; int yp; int xp; }; 

يبدو مُنشئ الفصل هذا كما يلي:

 entclass::entclass() { clear(); } void entclass::clear() { // Set all values to a default, // required for creating a new entity active = false; invis = false; type = 0; size = 0; tile = 0; rule = 0; state = 0; statedelay = 0; life = 0; colour = 0; para = 0; behave = 0; animate = 0; xp = 0; yp = 0; ax = 0; ay = 0; vx = 0; vy = 0; w = 16; h = 16; cx = 0; cy = 0; newxp = 0; newyp = 0; x1 = 0; y1 = 0; x2 = 320; y2 = 240; jumping = false; gravity = false; onground = 0; onroof = 0; jumpframe = 0; onentity = 0; harmful = false; onwall = 0; onxwall = 0; onywall = 0; isplatform = false; framedelay = 0; drawframe = 0; walkingframe = 0; dir = 0; actionframe = 0; } 

العديد من المجالات ، ألا تقول؟ لا عجب في ذلك ، فقد أصدرت PVS-Studio تحذيراً من حدوث خلل ، وهي مختبئة هنا:

V730 من الممكن عدم تهيئة جميع أعضاء الفصل داخل المنشئ. النظر في التفتيش: oldxp ، oldyp. Ent.cpp 3

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

تحذير 6

انظر إلى هذا الكود:

 void mapclass::loadlevel(....) { .... std::vector<std::string> tmap; .... tmap = otherlevel.loadlevel(rx, ry, game, obj); fillcontent(tmap); .... // The tmap vector gets changed again many times. } 

تحذير PVS-Studio: V688 يمتلك المتغير المحلي "tmap" نفس اسم أحد أعضاء الفصل ، مما قد يؤدي إلى حدوث تشويش. Map.cpp 1192

في الواقع ، بالبحث داخل فئة mapclass ، يمكنك العثور على نفس المتجه بنفس الاسم هناك:

 class mapclass { public: .... std::vector <int> roomdeaths; std::vector <int> roomdeathsfinal; std::vector <int> areamap; std::vector <int> contents; std::vector <int> explored; std::vector <int> vmult; std::vector <std::string> tmap; // <= .... }; 

لسوء الحظ ، فإن إعلان متجه الاسم نفسه داخل الوظيفة يجعل المتجه المعلن في الفصل غير مرئي. كما تبين أن ناقل tmap يتم تغييره فقط داخل وظيفة loadlevel . الموجه المعلن في الفصل يبقى كما هو!

ومن المثير للاهتمام ، أن PVS-Studio قد عثر على 20 من هذه الأجزاء من الشفرات! بالنسبة للجزء الأكبر ، فهي تتعلق بمتغيرات مؤقتة تم إعلانها "للراحة" كأعضاء في الفصل. كتب مؤلف اللعبة (ومطورها الوحيد) عن نفسه أنه اعتاد أن يكون لديه هذه العادة السيئة. يمكنك أن تقرأ عنها في المنشور - يتم إعطاء الرابط في بداية المقال.

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

تحذير 7

V601 نوع العدد الصحيح يتم ضمنيًا إلى نوع char. Game.cpp 4997

 void Game::loadquick(....) { .... else if (pKey == "totalflips") { totalflips = atoi(pText); } else if (pKey == "hardestroom") { hardestroom = atoi(pText); // <= } else if (pKey == "hardestroomdeaths") { hardestroomdeaths = atoi(pText); } .... } 

لفهم ما يجري ، دعنا نلقي نظرة على تعريفات المتغيرات من الجزء المحدد من الكود:

 //Some stats: int totalflips; std::string hardestroom; int hardestroomdeaths; 

تعد متغيرات totalflips و hardestroomdeaths عددًا صحيحًا ، لذلك من الطبيعي تمامًا تعيين نتيجة دالة atoi لهم . ولكن ماذا يحدث إذا قمت بتعيين قيمة عدد صحيح إلى std :: string ؟ اتضح أن هذه المهمة صحيحة من منظور اللغة. نتيجة لذلك ، سيتم كتابة قيمة غير واضحة في متغير hardestroom !

تحذير 8

V1004 تم استخدام مؤشر 'pElem' بطريقة غير آمنة بعد التحقق من عدم وجود nullptr. خطوط التحقق: 1739 ، 1744. editor.cpp 1744

 void editorclass::load(std::string &_path) { .... TiXmlHandle hDoc(&doc); TiXmlElement *pElem; TiXmlHandle hRoot(0); version = 0; { pElem = hDoc.FirstChildElement().Element(); // should always have a valid root // but handle gracefully if it does if (!pElem) { printf("No valid root! Corrupt level file?\n"); } pElem->QueryIntAttribute("version", &version); // <= // save this for later hRoot = TiXmlHandle(pElem); } .... } 

يحذر المحلل من أن مؤشر pElem يستخدم بطريقة غير آمنة مباشرة بعد التحقق من وجود nullptr . للتأكد من أن المحلل على صواب ، دعنا نتحقق من تعريف الدالة Element () التي تُرجع القيمة التي تقوم بدورها بتهيئة pElem poiter:

 /** @deprecated use ToElement. Return the handle as a TiXmlElement. This may return null. */ TiXmlElement *Element() const { return ToElement(); } 

كما نرى من التعليق ، قد ترجع هذه الوظيفة خالية .

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

تحذير 9

أثار جزء التعليمات البرمجية أربعة تحذيرات محلل PVS-Studio:

• V560 جزء من التعبير الشرطي صحيح دائمًا: x> = 0. editor.cpp 1137
• V560 جزء من التعبير الشرطي صحيح دائمًا: y> = 0. editor.cpp 1137
• V560 جزء من التعبير الشرطي صحيح دائمًا: x <40. editor.cpp 1137
• V560 جزء من التعبير الشرطي صحيح دائمًا: y <30. editor.cpp 1137

 int editorclass::at( int x, int y ) { if(x<0) return at(0,y); if(y<0) return at(x,0); if(x>=40) return at(39,y); if(y>=30) return at(x,29); if(x>=0 && y>=0 && x<40 && y<30) { return contents[x+(levx*40)+vmult[y+(levy*30)]]; } return 0; } 

جميع التحذيرات تتعلق بآخر عبارة if . المشكلة هي أن الشيكات الأربعة ، التي يتم إجراؤها فيها ، ستعود دائمًا إلى حقيقة لن أقول إنه خطأ خطير ، لكنه مضحك للغاية. قرر المؤلف أن يأخذ هذه الوظيفة على محمل الجد وفقط في حالة فحص كل متغير مرة أخرى :)

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

تحذير 10

في مقالته عن ذكرى اللعبة ، لاحظ تيري على نحو مثير للسخرية أن أحد العناصر التي تتحكم في منطق اللعبة هو التحول الكبير من وظيفة Game :: updatestate () ، المسؤولة عن عدد كبير من حالات اللعبة المختلفة في نفس الوقت . وكان من المتوقع أن أجد التحذير التالي:

V2008 التعقيد السيكلومي: 548. فكر في إعادة بيع وظيفة "Game :: updatestate". Game.cpp 612

نعم ، لقد فهمت ذلك بشكل صحيح: أعطت PVS-Studio الوظيفة تصنيف التعقيد التالي - 548. خمسمائة وثمانية وأربعون !!! هذه هي الطريقة التي يبدو بها "الرمز الأنيق". وهذا على الرغم من حقيقة أنه ، باستثناء بيان التبديل ، لا يوجد شيء آخر تقريباً في الوظيفة. في المحول نفسه ، عدت أكثر من 300 تعبير حالة.

كما تعلمون ، في شركتنا لدينا منافسة صغيرة لأطول مقال. أرغب في جلب رمز الوظيفة بالكامل (3450 سطرًا) إلى هنا ، لكن مثل هذا الفوز سيكون غير عادل ، لذلك سأقتصر فقط على رابط التبديل العملاق. أنصحك باتباع الرابط ورؤية طوله بنفسك! بالنسبة إلى ذلك ، بالإضافة إلى Game :: updatestate () ، عثر PVS-Studio أيضًا على 44 وظيفة ذات تعقيد سيكلومي مضخم ، 10 منها لديها عدد تعقيد أكثر من 200.

استنتاج

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

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