Celestia: les aventures des insectes dans l'espace

Image 1

Celestia est un simulateur spatial en trois dimensions. La simulation de l'espace permet d'explorer notre univers en trois dimensions. Celestia est disponible sur Windows, Linux et macOS. Le projet est très petit et PVS-Studio y a détecté peu de défauts. Malgré ce fait, nous aimerions y faire attention, car c'est un projet éducatif populaire et il sera plutôt utile de l'améliorer en quelque sorte. Soit dit en passant, ce programme est utilisé dans des films, séries et programmes populaires pour montrer l'espace. Ce fait, à son tour, augmente la qualité du code.

Présentation


Le site officiel du projet Celestia fournit sa description détaillée. Le code source est disponible sur GitHub . L'analyseur a vérifié 166 fichiers .cpp, à l'exclusion des bibliothèques et des tests. Le projet est petit, mais les défauts trouvés sont à noter.

Pour effectuer l'analyse du code source, nous avons utilisé l'analyseur de code statique PVS-Studio . Celestia et PVS-Studio sont multi-plateformes. Nous avons analysé le projet sur la plateforme Windows. Il était simple de construire le projet en obtenant des dépendances à l'aide de Vcpkg - gestionnaire de bibliothèques Microsoft. Selon les critiques, il est inférieur aux capacités de Conan , mais ce programme était également très 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 se tromper 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. La version correcte est très probablement la suivante:

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

Une recherche intéressante sur ce sujet: " Le mal 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 de la fonction memset . Au lieu de remplir la structure avec des zéros, il dit 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 destination est déréférencé de deux lignes avant d'être comparé à NULL . Un tel code peut 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é la classe héritée de la classe std :: exception via le modificateur privé (défini par défaut). Un tel héritage est dangereux car l' exception std :: exception ne sera pas interceptée en raison d'un héritage non public. 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; } 

Dans une partie de l'expression conditionnelle, le programmeur a oublié de déréférencer le pointeur s . Il s'est avéré être 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; } .... } 

La mémoire est libérée par le pointeur glShader mais n'est pas effacée par le pointeur vertexShader lors de la sortie de la fonction.

Un fragment similaire ci-dessous:
  • 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érification répétée de la présence du nom de fichier. Ce n'est pas un bogue, mais étant donné que la variable inputFilename est déjà vérifiée au début de la fonction, la vérification ci-dessous peut être supprimée, ce qui rend 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; } .... } 

Les valeurs d'énumération sont mélangées dans l'opérateur de commutation. Pour cette raison, les énumérations de différents types sont comparées dans un seul fragment: 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. Soit une erreur a été commise, soit deux conditions peuvent être combinées en une seule, ce qui simplifie 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 retourné par le nouvel opérateur est comparée à null. Si l'opérateur n'a pas pu allouer de mémoire, alors selon la norme C ++, une exception std :: bad_alloc () est levée. Ensuite, la vérification de null est inutile.

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); .... } 

Le diagnostic est facultatif mais dans ce cas, il est préférable d'utiliser la 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. Il existe des milliers d'extensions avec différents objets spatiaux sur Internet. Celestia a été utilisée dans le film " The Day After Tomorrow " et la série documentaire " Through the Wormhole with Morgan Freeman ".

Nous sommes heureux qu'en vérifiant divers projets avec du code open source, nous ne faisons pas seulement la promotion de la méthodologie d'analyse de code statique, mais contribuons également au développement de 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!

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


All Articles