VVVVVV ؟؟ VVVVVV !!! :)

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

مقدمة

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

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

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

في الوقت الحالي ، أنا C ++ - المطور في فريق 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 حرفًا. سلسلة التنسيق (الوسيطة الثالثة sprintf ) بعد استبدال محتويات السلسلة oldDirectory به ستبدو كما يلي:

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

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

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

تحذير 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 */ 

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

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

 str[0] != '\0' 

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

تحذير 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); .... //  tmap    . } 

تحذير 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; // <= .... }; 

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

ومن المثير للاهتمام ، اكتشف 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; 

تكون المتغيرات الإجمالية والوفيات القاسية من النوع الصحيح ، وبالتالي فإن تعيين النتيجة إلى وظيفة 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 يستخدم بشكل غير آمن فور التحقق من وجوده . للتأكد من صحة المحلل ، ألقِ نظرة على تعريف الدالة Element () ، حيث تقوم قيمة الإرجاع بتهيئة مؤشر pElem :

 /** @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; } 

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

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

تحذير 10

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

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

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

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

استنتاج

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

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

إذا كنت ترغب في مشاركة هذا المقال مع جمهور ناطق باللغة الإنجليزية ، فيرجى استخدام الرابط الخاص بالترجمة: George Gribkov. VVVVVV ؟؟ VVVVVV !!! :) .