سيليستيا: مغامرات البق في الفضاء

الصورة 1

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

مقدمة


على الموقع الرسمي لمشروع Celestia ، يمكنك العثور على وصف تفصيلي له. يتم استضافة التعليمات البرمجية المصدر للمشروع على جيثب . باستثناء المكتبات والاختبارات ، فحص المحلل ملفات 166 .cpp. المشروع صغير ، لكن العيوب الموجودة مثيرة للاهتمام.

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

نتائج التحليل


تحذير 1

V501 هناك تعبيرات فرعية مماثلة إلى اليسار وإلى يمين عامل التشغيل "<": b.nAttributes <b.nAttributes cmodfix.cpp 378

bool operator<(const Mesh::VertexDescription& a, const Mesh::VertexDescription& b) { if (a.stride < b.stride) return true; if (b.stride < a.stride) return false; if (a.nAttributes < b.nAttributes) // <= return true; if (b.nAttributes < b.nAttributes) // <= return false; for (uint32_t i = 0; i < a.nAttributes; i++) { if (a.attributes[i] < b.attributes[i]) return true; else if (b.attributes[i] < a.attributes[i]) return false; } return false; } 

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

قام المبرمج بنسخ التعبير الشرطي ولم يقم بتحريره بالكامل. الخيار الصحيح هو الأرجح:

 if (a.nAttributes < b.nAttributes) return true; if (b.nAttributes < a.nAttributes) return false; 

دراسة مثيرة للاهتمام حول هذا الموضوع: " يعيش الشر في وظائف المقارنة ."

تحذير 2

V575 تقوم وظيفة "memset" بمعالجة عناصر "0". تفقد الحجة الثالثة. winmain.cpp 2235

 static void BuildScriptsMenu(HMENU menuBar, const fs::path& scriptsDir) { .... MENUITEMINFO info; memset(&info, sizeof(info), 0); info.cbSize = sizeof(info); info.fMask = MIIM_SUBMENU; .... } 

اختلط مؤلف التعليمات البرمجية الوسيطتين الثانية والثالثة بوظيفة memset . بدلاً من ملء البنية بالأصفار ، تتم الإشارة إلى ملء 0 بايت من الذاكرة.

تحذير 3

V595 تم استخدام مؤشر "الأماكن" قبل أن يتم التحقق منه ضد nullptr. خطوط الفحص: 48 ، 50. wintourguide.cpp 48

 BOOL APIENTRY TourGuideProc(....) { .... const DestinationList* destinations = guide->appCore->getDestinations(); Destination* dest = (*destinations)[0]; guide->selectedDest = dest; if (hwnd != NULL && destinations != NULL) { .... } .... } 

يتم إلغاء تحديد مؤشر الوجهات سطرين أعلى من مقارنة NULL . قد يؤدي هذا الرمز إلى حدوث خطأ.

تحذير 4

يجب أن تكون فئات V702 مستمدة دائمًا من std :: استثناء (وما شابه ) كـ "عام" (لم يتم تحديد أي كلمة رئيسية ، لذلك يقوم المحول البرمجي بتعيينها إلى "خاص"). 21

 class filesystem_error : std::system_error { public: filesystem_error(std::error_code ec, const char* msg) : std::system_error(ec, msg) { } }; // filesystem_error 

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

تحذير 5

V713 تم استخدام المؤشر في التعبير المنطقي قبل أن يتم التحقق منه ضد nullptr في نفس التعبير المنطقي. winmain.cpp 3031

 static char* skipUntilQuote(char* s) { while (*s != '"' && s != '\0') s++; return s; } 

في مكان واحد من التعبير الشرطي ، نسوا أن dereference المؤشر s . كانت النتيجة مقارنة المؤشر ، وليس القيمة الموجودة عليه. وهذا لا معنى له في هذا الموقف.

تحذير 6

V773 تم إنهاء الوظيفة دون تحرير مؤشر "vertexShader". تسرب الذاكرة ممكن. modelviewwidget.cpp 1517

 GLShaderProgram* ModelViewWidget::createShader(const ShaderKey& shaderKey) { .... auto* glShader = new GLShaderProgram(); auto* vertexShader = new GLVertexShader(); if (!vertexShader->compile(vertexShaderSource.toStdString())) { qWarning("Vertex shader error: %s", vertexShader->log().c_str()); std::cerr << vertexShaderSource.toStdString() << std::endl; delete glShader; return nullptr; } .... } 

عند الخروج من الوظيفة ، يتم تحرير الذاكرة بواسطة مؤشر glShader ، لكن لا يتم مسحها بواسطة مؤشر vertexShader .

نفس المكان أقل في الرمز:

  • V773 تم إنهاء الوظيفة دون تحرير مؤشر 'fragmentShader'. تسرب الذاكرة ممكن. modelviewwidget.cpp 1526

تحذير 7

تعبير V547 '! InputFilename.empty ()' صحيح دائمًا. makexindex.cpp 128

 int main(int argc, char* argv[]) { if (!parseCommandLine(argc, argv) || inputFilename.empty()) { Usage(); return 1; } istream* inputFile = &cin; if (!inputFilename.empty()) { inputFile = new ifstream(inputFilename, ios::in); if (!inputFile->good()) { cerr << "Error opening input file " << inputFilename << '\n'; return 1; } } .... } 

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

تحذير 8

V556 تتم مقارنة قيم أنواع التعداد المختلفة: switch (ENUM_TYPE_A) {case ENUM_TYPE_B: ...}. render.cpp 7457

 enum LabelAlignment { AlignCenter, AlignLeft, AlignRight }; enum LabelVerticalAlignment { VerticalAlignCenter, VerticalAlignBottom, VerticalAlignTop, }; struct Annotation { .... LabelVerticalAlignment valign : 3; .... }; void Renderer::renderAnnotations(....) { .... switch (annotations[i].valign) { case AlignCenter: vOffset = -font[fs]->getHeight() / 2; break; case VerticalAlignTop: vOffset = -font[fs]->getHeight(); break; case VerticalAlignBottom: vOffset = 0; break; } .... } 

في بيان التبديل ، يتم خلط قيم التعداد. لهذا السبب ، تتم مقارنة تعدادات الأنواع المختلفة في مكان واحد: LabelVerticalAlignment و AlignCenter .

تحذير 9

V581 التعبيرات الشرطية لعبارات "if" الموجودة بجانب بعضها البعض متطابقة. خطوط الفحص: 2844 ، 2850. shadermanager.cpp 2850

 GLVertexShader* ShaderManager::buildParticleVertexShader(const ShaderProperties& props) { .... if (props.texUsage & ShaderProperties::PointSprite) { source << "uniform float pointScale;\n"; source << "attribute float pointSize;\n"; } if (props.texUsage & ShaderProperties::PointSprite) { source << DeclareVarying("pointFade", Shader_Float); } .... } 

اكتشف المحلل تعبيرين شرطيين متطابقين على التوالي. هناك خطأ أو يمكن ضم شرطين في واحد ، وبالتالي تبسيط التعليمات البرمجية.

تحذير 10

V668 لا يوجد أي معنى في اختبار مؤشر 'dp' مقابل قيمة خالية ، حيث تم تخصيص الذاكرة باستخدام عامل التشغيل "الجديد". سيتم إنشاء الاستثناء في حالة خطأ تخصيص الذاكرة. windatepicker.cpp 625

 static LRESULT DatePickerCreate(HWND hwnd, CREATESTRUCT& cs) { DatePicker* dp = new DatePicker(hwnd, cs); if (dp == NULL) return -1; .... } 

تتم مقارنة قيمة المؤشر التي أرجعها المشغل الجديد إلى الصفر. إذا تعذر على المشغل تخصيص الذاكرة ، فسيتم إلقاء استثناء std :: bad_alloc () وفقًا لمعايير لغة C ++. وبالتالي ، التحقق من المؤشر إلى المساواة إلى الصفر غير منطقي.

ثلاثة اختبارات أخرى مماثلة:

  • V668 لا يوجد أي معنى في اختبار مؤشر "الأوضاع" مقابل قيمة خالية ، حيث تم تخصيص الذاكرة باستخدام عامل التشغيل "الجديد". سيتم إنشاء الاستثناء في حالة خطأ تخصيص الذاكرة. winmain.cpp 2967
  • V668 لا يوجد أي معنى في اختبار مؤشر "dropTarget" مقابل قيمة خالية ، حيث تم تخصيص الذاكرة باستخدام عامل التشغيل "الجديد". سيتم إنشاء الاستثناء في حالة خطأ تخصيص الذاكرة. winmain.cpp 3272
  • V668 لا يوجد أي معنى في اختبار مؤشر "appCore" مقابل قيمة خالية ، حيث تم تخصيص الذاكرة باستخدام عامل التشغيل "الجديد". سيتم إنشاء الاستثناء في حالة خطأ تخصيص الذاكرة. winmain.cpp 3352

تحذير 11

V624 الثابت 3.14159265 قيد الاستخدام. يمكن أن تكون القيمة الناتجة غير دقيقة. حاول استخدام ثابت M_PI من <math.h>. 3dstocmod.cpp 62

 int main(int argc, char* argv[]) { .... Model* newModel = GenerateModelNormals(*model, float(smoothAngle * 3.14159265 / 180.0), weldVertices, weldTolerance); .... } 

يوصى باستخدام أداة التشخيص ، ولكن من الأفضل حقًا استخدام ثابت جاهز لرقم Pi من المكتبة القياسية.

استنتاج


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

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

استخدم محللات الكود الثابت ، وجعل مشاريعك أكثر موثوقية وأفضل!



إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بالترجمة: Svyatoslav Razmyslov. سيليستيا: مغامرات البق في الفضاء .

Source: https://habr.com/ru/post/ar469743/


All Articles