Celestia: Die Abenteuer der Käfer im Weltraum

Celestia ist ein dreidimensionaler Raumsimulator. Die Weltraumsimulation ermöglicht es uns, unser Universum in drei Dimensionen zu erkunden. Celestia ist unter Windows, Linux und MacOS verfügbar. Das Projekt ist sehr klein und mit PVS-Studio werden nur sehr wenige Fehler erkannt. Wir möchten ihm jedoch wirklich Aufmerksamkeit schenken, da dies ein beliebtes Bildungsprojekt ist, das zur Verbesserung nützlich ist. Das Programm wird übrigens in populären Filmen, Serien und Programmen zur Darstellung des Raumes verwendet. Dies erhöht auch die Qualitätsanforderungen des Codes.

Einführung

Auf der offiziellen Website des Celestia- Projekts finden Sie eine detaillierte Beschreibung. Der Quellcode für das Projekt wird auf GitHub gehostet . Mit Ausnahme von Bibliotheken und Tests überprüfte der Analysator 166 CPP-Dateien. Das Projekt ist klein, aber die gefundenen Mängel sind sehr interessant.

Zur Analyse des Codes haben wir den statischen Code-Analysator PVS-Studio verwendet. Sowohl Celestia als auch PVS-Studio sind plattformübergreifend. Für die Analyse wurde die Windows-Plattform ausgewählt. Das Projekt war einfach zu erstellen, indem Abhängigkeiten mit Vcpkg , dem Microsoft-Bibliotheksmanager, aufgerufen wurden. Laut Bewertungen ist es den Fähigkeiten von Conan unterlegen, aber dieses Programm war auch bequem zu bedienen.

Analyseergebnisse

Warnung 1

V501 Links und rechts vom Operator '<' befinden sich identische Unterausdrücke: 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; } 

Wie einfach es ist, beim Kopieren von Code Fehler zu machen. Wir schreiben darüber in jeder Rezension. Anscheinend kann in dieser Situation nur die statische Code-Analyse helfen.

Der Programmierer hat den bedingten Ausdruck kopiert und nicht vollständig bearbeitet. Die richtige Option ist höchstwahrscheinlich folgende:

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

Eine interessante Studie zu diesem Thema: "Das Böse lebt in Vergleichsfunktionen ."

Warnung 2

V575 Die Funktion 'memset' verarbeitet '0'-Elemente. Untersuchen Sie das dritte Argument. 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; .... } 

Der Autor des Codes hat das zweite und dritte Argument mit der Memset- Funktion verwechselt . Anstatt die Struktur mit Nullen zu füllen, wird angezeigt, 0 Bytes Speicher zu füllen.

Warnung 3

V595 Der Zeiger 'Ziele' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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) { .... } .... } 

Der Zielzeiger wird zwei Zeilen höher dereferenziert als im Vergleich zu NULL . Ein solcher Code kann möglicherweise zu einem Fehler führen.

Warnung 4

V702- Klassen sollten immer von std :: exception (und ähnlichem) als 'public' abgeleitet werden (es wurde kein Schlüsselwort angegeben, daher ist der Compiler standardmäßig 'private'). fs.h 21

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

Der Analysator hat eine Klasse erkannt, die von der Klasse std :: exception über den privaten Modifikator (standardmäßig angegeben) geerbt wurde. Diese Vererbung ist gefährlich, da sie bei nicht öffentlicher Vererbung beim Versuch, die Ausnahme std :: exception abzufangen , übersprungen wird. Infolgedessen verhalten sich Ausnahmebehandlungsroutinen nicht wie beabsichtigt.

Warnung 5

V713 Der Zeiger 's' wurde im logischen Ausdruck verwendet, bevor er im selben logischen Ausdruck gegen nullptr verifiziert wurde. winmain.cpp 3031

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

An einer Stelle des bedingten Ausdrucks vergaßen sie, den Zeiger s zu dereferenzieren. Das Ergebnis war ein Vergleich des Zeigers, nicht des Wertes darauf. Und das macht in dieser Situation keinen Sinn.

Warnung 6

V773 Die Funktion wurde beendet, ohne den Zeiger 'vertexShader' freizugeben. Ein Speicherverlust ist möglich. 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; } .... } 

Wenn Sie die Funktion verlassen, wird der Speicher durch den glShader- Zeiger freigegeben , aber nicht durch den vertexShader- Zeiger gelöscht .

Die gleiche Stelle ist im Code niedriger:

• V773 Die Funktion wurde beendet, ohne den Zeiger 'fragmentShader' freizugeben. Ein Speicherverlust ist möglich. modelviewwidget.cpp 1526

Warnung 7

V547 Ausdruck '! InputFilename.empty ()' ist immer wahr. 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; } } .... } 

Überprüfen Sie den Dateinamen erneut. Dies ist kein Fehler, aber aufgrund der Tatsache, dass die Variable inputFilename bereits zu Beginn der Funktion überprüft wurde , kann die Überprüfung unten entfernt werden, wodurch der Code kompakter wird.

Warnung 8

V556 Die Werte verschiedener Aufzählungstypen werden verglichen: 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; } .... } 

In der switch-Anweisung werden die Aufzählungswerte verwechselt. Aus diesem Grund werden Aufzählungen verschiedener Typen an einer Stelle verglichen: LabelVerticalAlignment und AlignCenter .

Warnung 9

V581 Die bedingten Ausdrücke der nebeneinander angeordneten if-Anweisungen sind identisch. Überprüfen Sie die Zeilen: 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); } .... } 

Der Analysator erkannte zwei identische bedingte Ausdrücke hintereinander. Es liegt entweder ein Fehler vor oder zwei Bedingungen können zu einer kombiniert werden, wodurch der Code vereinfacht wird.

Warnung 10

V668 Es macht keinen Sinn, den 'dp'-Zeiger gegen null zu testen, da der Speicher mit dem' new' -Operator zugewiesen wurde. Die Ausnahme wird bei einem Speicherzuordnungsfehler generiert. windatepicker.cpp 625

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

Der Wert des vom neuen Operator zurückgegebenen Zeigers wird mit Null verglichen. Wenn der Operator keinen Speicher zuordnen konnte, wird gemäß dem C ++ - Sprachstandard eine Ausnahme std :: bad_alloc () ausgelöst . Daher ist es nicht sinnvoll, den Zeiger auf Gleichheit auf Null zu überprüfen.

Drei weitere ähnliche Prüfungen:

• V668 Es macht keinen Sinn, den Zeiger 'Modi' gegen Null zu testen, da der Speicher mit dem Operator 'Neu' zugewiesen wurde. Die Ausnahme wird bei einem Speicherzuordnungsfehler generiert. winmain.cpp 2967
• V668 Es macht keinen Sinn, den 'dropTarget'-Zeiger gegen null zu testen, da der Speicher mit dem' new'-Operator zugewiesen wurde. Die Ausnahme wird bei einem Speicherzuordnungsfehler generiert. winmain.cpp 3272
• V668 Es macht keinen Sinn, den 'appCore'-Zeiger gegen null zu testen, da der Speicher mit dem' new'-Operator zugewiesen wurde. Die Ausnahme wird bei einem Speicherzuordnungsfehler generiert. winmain.cpp 3352

Warnung 11

V624 Die Konstante 3.14159265 wird verwendet. Der resultierende Wert kann ungenau sein. Erwägen Sie die Verwendung der M_PI-Konstante aus <math.h>. 3dstocmod.cpp 62

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

Die Diagnose wird empfohlen, es ist jedoch besser, eine vorgefertigte Konstante für die Pi-Nummer aus der Standardbibliothek zu verwenden.

Fazit

Vor kurzem wurde das Projekt von Enthusiasten entwickelt, ist aber immer noch beliebt und in Schulungsprogrammen gefragt. Im Internet gibt es Tausende von Add-Ons mit unterschiedlichen Weltraumobjekten. Celestia wurde in dem Film " The Day After Tomorrow " und der Dokumentarserie " Through the Wormhole with Morgan Freeman " verwendet.

Wir freuen uns, dass wir durch das Testen vieler Open Source-Projekte nicht nur die Methodik der statischen Code-Analyse populär machen, sondern auch zur Entwicklung der Welt der Open-Projekte beitragen. Übrigens können Sie mit dem PVS-Studio-Analysegerät nicht nur Ihre eigenen, sondern auch Projekte von Drittanbietern als Enthusiasten testen. Dazu können Sie eine der Optionen für die kostenlose Lizenzierung verwenden.

Verwenden Sie statische Code-Analysatoren, um Ihre Projekte zuverlässiger und besser zu machen!

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Svyatoslav Razmyslov. Celestia: Bugs 'Adventures im Weltraum .