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 1V501 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)
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 2V575 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 3V595 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 4Les 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) { } };
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 5V713 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 6V773 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 7V547 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 8V556 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 9V581 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 10V668 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 11V624 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!