Celestia ist ein dreidimensionaler Raumsimulator. Die Simulation des Raums ermöglicht die Erforschung unseres Universums in drei Dimensionen. Celestia ist unter Windows, Linux und MacOS verfügbar. Das Projekt ist sehr klein und PVS-Studio hat nur wenige Mängel festgestellt. Trotz dieser Tatsache möchten wir darauf achten, da es sich um ein beliebtes Bildungsprojekt handelt und es ziemlich nützlich sein wird, es irgendwie zu verbessern. Dieses Programm wird übrigens in populären Filmen, Serien und Programmen zur Darstellung von Raum verwendet. Diese Tatsache stellt wiederum Anforderungen an die Codequalität.
Einführung
Die offizielle Website des
Celestia- Projekts enthält eine detaillierte Beschreibung. Der Quellcode ist auf
GitHub verfügbar. Der Analysator überprüfte 166 CPP-Dateien mit Ausnahme von Bibliotheken und Tests. Das Projekt ist klein, aber festgestellte Mängel sind bemerkenswert.
Für die Quellcode-Analyse haben wir den statischen Code-Analysator
PVS-Studio verwendet. Sowohl Celestia als auch PVS-Studio sind plattformübergreifend. Wir haben das Projekt auf der Windows-Plattform analysiert. Es war einfach, das Projekt zu erstellen, indem Abhängigkeiten mit
Vcpkg - Microsoft Library Manager
abgerufen wurden . Laut Bewertungen ist es
Conans Kapazitäten unterlegen, aber dieses Programm war auch recht bequem zu bedienen.
Analyseergebnisse
Warnung 1V501 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)
Wie einfach es ist, beim Kopieren von Code einen 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 Version ist höchstwahrscheinlich wie folgt:
if (a.nAttributes < b.nAttributes) return true; if (b.nAttributes < a.nAttributes) return false;
Eine interessante Recherche zu diesem Thema: "
Das Böse in den Vergleichsfunktionen ".
Warnung 2V575 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 Code-Autor hat das zweite und dritte Argument der
Memset- Funktion
verwechselt . Anstatt die Struktur mit Nullen zu füllen, heißt es, 0 Bytes Speicher zu füllen.
Warnung 3V595 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 dereferenziert, bevor er mit
NULL verglichen wird. Ein solcher Code kann möglicherweise zu einem Fehler führen.
Warnung 4V702- 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) { } };
Der Analysator hat die von der Klasse
std :: exception geerbte Klasse über den
privaten Modifikator (standardmäßig festgelegt) erkannt. Eine solche Vererbung ist gefährlich, da die
Ausnahme std :: exception aufgrund einer nicht öffentlichen Vererbung nicht abgefangen wird. Infolgedessen verhalten sich Ausnahmebehandlungsroutinen nicht wie beabsichtigt.
Warnung 5V713 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; }
In einem Teil des bedingten Ausdrucks hat der Programmierer vergessen, den Zeiger
s zu dereferenzieren. Es stellte sich heraus, dass es sich um einen Vergleich des Zeigers handelte, nicht um seinen Wert. Und das macht in dieser Situation keinen Sinn.
Warnung 6V773 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; } .... }
Der Speicher wird vom
glShader- Zeiger
freigegeben, aber beim Verlassen der Funktion nicht vom
vertexShader- Zeiger gelöscht.
Ein ähnliches Fragment unten:
- V773 Die Funktion wurde beendet, ohne den Zeiger 'fragmentShader' freizugeben. Ein Speicherverlust ist möglich. modelviewwidget.cpp 1526
Warnung 7V547 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; } } .... }
Wiederholte Überprüfung des Vorhandenseins des Dateinamens. Es ist kein Fehler, aber aufgrund der Tatsache, dass die Variable
inputFilename bereits zu Beginn der Funktion überprüft wurde, kann die folgende Überprüfung entfernt werden, wodurch der Code kompakter wird.
Warnung 8V556 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; } .... }
Aufzählungswerte werden im Switch-Operator verwechselt. Aus diesem Grund werden Aufzählungen verschiedener Typen in einem Fragment verglichen:
LabelVerticalAlignment und
AlignCenter .
Warnung 9V581 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 hat zwei identische bedingte Ausdrücke hintereinander erkannt. Es wurde entweder ein Fehler gemacht oder zwei Bedingungen können zu einer kombiniert werden, wodurch der Code einfacher wird.
Warnung 10V668 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 ++ - Standard eine Ausnahme
std :: bad_alloc () ausgelöst. Dann ist die Prüfung auf Null sinnlos.
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 11V624 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 ist optional, aber in diesem Fall ist es besser, die 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 den Trainingsprogrammen gefragt. Es gibt Tausende von Addons mit unterschiedlichen Weltraumobjekten im Internet. 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 die Überprüfung
verschiedener Projekte mit Open Source-Code nicht nur die statische Code-Analysemethode fördern, sondern auch zur Entwicklung von Open Source-Projekten beitragen. Übrigens können Sie mit dem PVS-Studio-Analysegerät nicht nur Ihre eigenen, sondern auch Projekte von Drittanbietern als Enthusiasten testen. Zu diesem Zweck können Sie eine der
Optionen der kostenlosen Lizenzierung verwenden.
Verwenden Sie statische Code-Analysatoren, um Ihre Projekte zuverlässiger und besser zu machen!