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

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

مقدمة

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

للقيام بتحليل الكود المصدري ، استخدمنا محلل الكود الثابت 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; } 

في جزء واحد من التعبير الشرطي ، نسي المبرمج إلغاء تحديد مؤشر 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; .... } 

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

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

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

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