Celestia: les aventures de bugs dans l'espace

Celestia est un simulateur spatial en trois dimensions. La simulation spatiale nous permet d'explorer notre univers en trois dimensions. Celestia est disponible sur Windows, Linux et macOS. Le projet est très petit et en lui, en utilisant PVS-Studio, un très petit nombre de défauts sont détectés. Cependant, nous voulons vraiment faire attention à lui, car il s'agit d'un projet éducatif populaire qui est utile pour s'améliorer. Soit dit en passant, le programme est utilisé dans des films, séries et programmes populaires pour représenter l'espace. Ce qui soulève également les exigences de qualité du code.

Présentation

Sur le site officiel du projet Celestia , vous pouvez en trouver une description détaillée. Le code source du projet est hébergé sur GitHub . Excluant les bibliothèques et les tests, l'analyseur a vérifié 166 fichiers .cpp. Le projet est petit, mais les défauts constatés sont assez intéressants.

Pour analyser le code, nous avons utilisé l'analyseur de code statique PVS-Studio . Celestia et PVS-Studio sont multi-plateformes. Pour l'analyse, la plate-forme Windows a été sélectionnée. Le projet a été facile à construire en récupérant les dépendances à l'aide de Vcpkg , le gestionnaire de bibliothèque Microsoft. Selon les critiques, il est inférieur à Conan , mais ce programme était également pratique à utiliser.

Résultats d'analyse

Avertissement 1

V501 Il existe des sous-expressions identiques à gauche et à droite de l'opérateur '<': 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; } 

Comme il est facile de faire des erreurs lors de la copie de code. Nous écrivons à ce sujet dans chaque avis. Apparemment, seule l'analyse de code statique peut aider dans cette situation.

Le programmeur a copié l'expression conditionnelle et ne l'a pas entièrement modifiée. L'option correcte est très probablement la suivante:

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

Une étude intéressante sur ce sujet: «Le mal vit dans les fonctions de comparaison ».

Avertissement 2

V575 La fonction 'memset' traite les éléments '0'. Inspectez le troisième 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; .... } 

L'auteur du code a mélangé les deuxième et troisième arguments à la fonction memset . Au lieu de remplir la structure avec des zéros, il est indiqué de remplir 0 octet de mémoire.

Avertissement 3

V595 Le pointeur «destinations» a été utilisé avant d'être vérifié par rapport à nullptr. Vérifier les lignes: 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) { .... } .... } 

Le pointeur de destinations est déréférencé deux lignes plus haut que comparé à NULL . Un tel code pourrait potentiellement conduire à une erreur.

Avertissement 4

Les classes V702 doivent toujours être dérivées de std :: exception (et similaires) en tant que «public» (aucun mot-clé n'a été spécifié, donc le compilateur le définit par défaut sur «privé»). 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 

L'analyseur a détecté une classe héritée de la classe std :: exception via le modificateur privé (spécifié par défaut). Cet héritage est dangereux car en cas d'héritage non public, lors de la tentative d'interception de l' exception std :: exception , il sera ignoré. Par conséquent, les gestionnaires d'exceptions ne se comportent pas comme prévu.

Avertissement 5

V713 Le pointeur a été utilisé dans l'expression logique avant d'être vérifié par rapport à nullptr dans la même expression logique. winmain.cpp 3031

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

À un endroit de l'expression conditionnelle, ils ont oublié de déréférencer le pointeur s . Le résultat a été une comparaison du pointeur, pas de sa valeur. Et cela n'a pas de sens dans cette situation.

Avertissement 6

V773 La fonction a été fermée sans relâcher le pointeur 'vertexShader'. Une fuite de mémoire est possible. 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; } .... } 

Lorsque vous quittez la fonction, la mémoire est libérée par le pointeur glShader , mais elle n'est pas effacée par le pointeur vertexShader .

Le même endroit est plus bas dans le code:

• V773 La fonction a été quittée sans libérer le pointeur 'fragmentShader'. Une fuite de mémoire est possible. modelviewwidget.cpp 1526

Avertissement 7

V547 L' expression '! InputFilename.empty ()' est toujours vraie. 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; } } .... } 

Vérifiez à nouveau le nom du fichier. Ce n'est pas une erreur, mais étant donné que la variable inputFilename est déjà vérifiée au début de la fonction, la vérification peut être supprimée ci-dessous, ce qui rendra le code plus compact.

Avertissement 8

V556 Les valeurs des différents types d'énumération sont comparées: 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; } .... } 

Dans l' instruction switch , les valeurs d'énumération sont confuses. Pour cette raison, les énumérations de différents types sont comparées en un seul endroit: LabelVerticalAlignment et AlignCenter .

Avertissement 9

V581 Les expressions conditionnelles des instructions «if» situées côte à côte sont identiques. Vérifiez les lignes: 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); } .... } 

L'analyseur a détecté deux expressions conditionnelles identiques d'affilée. Il y a soit une erreur, soit deux conditions peuvent être combinées en une seule et simplifier ainsi le code.

Avertissement 10

V668 Il est inutile de tester le pointeur 'dp' contre null, car la mémoire a été allouée à l'aide de l'opérateur 'new'. L'exception sera générée en cas d'erreur d'allocation de mémoire. windatepicker.cpp 625

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

La valeur du pointeur renvoyé par le nouvel opérateur est comparée à zéro. Si l'opérateur n'a pas pu allouer de mémoire, alors selon la norme du langage C ++, une exception std :: bad_alloc () est levée . Ainsi, vérifier le pointeur de l'égalité à zéro n'a pas de sens.

Trois autres contrôles similaires:

• V668 Il est inutile de tester le pointeur «modes» par rapport à null, car la mémoire a été allouée à l'aide de l'opérateur «nouveau». L'exception sera générée en cas d'erreur d'allocation de mémoire. winmain.cpp 2967
• V668 Il est inutile de tester le pointeur 'dropTarget' contre null, car la mémoire a été allouée à l'aide de l'opérateur 'new'. L'exception sera générée en cas d'erreur d'allocation de mémoire. winmain.cpp 3272
• V668 Il est inutile de tester le pointeur «appCore» par rapport à null, car la mémoire a été allouée à l'aide de l'opérateur «nouveau». L'exception sera générée en cas d'erreur d'allocation de mémoire. winmain.cpp 3352

Avertissement 11

V624 La constante 3,14159265 est utilisée. La valeur résultante peut être inexacte. Pensez à utiliser la constante M_PI de <math.h>. 3dstocmod.cpp 62

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

Les diagnostics sont recommandés, mais il est vraiment préférable d'utiliser une constante prête à l'emploi pour le nombre Pi de la bibliothèque standard.

Conclusion

Récemment, le projet a été développé par des passionnés, mais il est toujours populaire et en demande dans les programmes de formation. Sur Internet, il existe des milliers de modules complémentaires avec différents objets spatiaux. Celestia a été utilisé dans The Day After Tomorrow et la série documentaire Through the Wormhole avec Morgan Freeman .

Nous sommes heureux qu'en vérifiant de nombreux projets open source, nous non seulement vulgarisons la méthodologie de l'analyse de code statique, mais contribuons également au développement du monde des projets open source. Soit dit en passant, vous pouvez également utiliser l'analyseur PVS-Studio non seulement pour tester le vôtre, mais également des projets tiers en tant que passionné. Pour ce faire, vous pouvez utiliser l'une des options de licence gratuite.

Utilisez des analyseurs de code statique, rendez vos projets plus fiables et meilleurs!

Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Svyatoslav Razmyslov. Celestia: Bugs 'Adventures in Space .