Celestia es un simulador espacial tridimensional. La simulación espacial nos permite explorar nuestro universo en tres dimensiones. Celestia está disponible en Windows, Linux y macOS. El proyecto es muy pequeño y en él, utilizando PVS-Studio, se detecta un número muy pequeño de defectos. Sin embargo, realmente queremos prestarle atención, ya que este es un proyecto educativo popular que es útil para mejorar. Por cierto, el programa se usa en películas, series y programas populares para representar el espacio. Lo que también aumenta los requisitos de calidad del código.
Introduccion
En el sitio web oficial del proyecto
Celestia puede encontrar una descripción detallada del mismo. El código fuente del proyecto está alojado en
GitHub . Excluyendo bibliotecas y pruebas, el analizador verificó 166 archivos .cpp. El proyecto es pequeño, pero los defectos encontrados son bastante interesantes.
Para analizar el código, utilizamos el analizador de código estático
PVS-Studio . Tanto Celestia como PVS-Studio son multiplataforma. Para el análisis, se seleccionó la plataforma Windows. El proyecto fue fácil de construir al extraer dependencias usando
Vcpkg , el administrador de la biblioteca de Microsoft. Según las revisiones, es inferior a
Conan , pero este programa también fue conveniente de usar.
Resultados de analisis
Advertencia 1V501 Hay
subexpresiones idénticas a la izquierda y a la derecha del operador '<': 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)
Qué fácil es cometer errores al copiar código. Escribimos sobre esto en cada revisión. Aparentemente, solo el análisis de código estático puede ayudar en esta situación.
El programador copió la expresión condicional y no la editó por completo. La opción correcta es muy probablemente esta:
if (a.nAttributes < b.nAttributes) return true; if (b.nAttributes < a.nAttributes) return false;
Un estudio interesante sobre este tema: "El
mal vive en funciones de comparación ".
Advertencia 2V575 La función 'memset' procesa elementos '0'. Inspeccione el tercer argumento. 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; .... }
El autor del código mezcló el segundo y el tercer argumento con la función
memset . En lugar de llenar la estructura con ceros, se indica que llene 0 bytes de memoria.
Advertencia 3V595 El puntero de 'destinos' se utilizó antes de verificarlo con nullptr. Líneas de verificación: 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) { .... } .... }
El puntero de
destinos se desreferencia dos líneas más arriba que en comparación con
NULL . Tal código podría conducir a un error.
Advertencia 4Las clases
V702 siempre deben derivarse de std :: exception (y similares) como 'público' (no se especificó ninguna palabra clave, por lo que el compilador lo predetermina a 'privado'). fs.h 21
class filesystem_error : std::system_error { public: filesystem_error(std::error_code ec, const char* msg) : std::system_error(ec, msg) { } };
El analizador detectó una clase heredada de la clase
std :: exception a través del modificador
privado (especificado por defecto). Esta herencia es peligrosa porque en caso de herencia no pública, cuando se intenta detectar la
excepción std :: exception , se omitirá. Como resultado, los manejadores de excepciones no se comportan según lo previsto.
Advertencia 5V713 El puntero 's' se utilizó en la expresión lógica antes de que se verificara contra nullptr en la misma expresión lógica. winmain.cpp 3031
static char* skipUntilQuote(char* s) { while (*s != '"' && s != '\0') s++; return s; }
En un lugar de la expresión condicional, olvidaron desreferenciar los punteros. El resultado fue una comparación del puntero, no el valor en él. Y esto no tiene sentido en esta situación.
Advertencia 6V773 Se salió de la función sin soltar el puntero 'vertexShader'. Una pérdida de memoria es posible. 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; } .... }
Cuando sale de la función, el puntero
glShader libera la
memoria , pero el puntero
vertexShader no la
borra .
El mismo lugar es más bajo en el código:
- V773 Se salió de la función sin soltar el puntero 'fragmentShader'. Una pérdida de memoria es posible. modelviewwidget.cpp 1526
Advertencia 7La expresión
V547 '! InputFilename.empty ()' siempre es verdadera. 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; } } .... }
Vuelva a verificar el nombre del archivo. Esto no es un error, pero debido al hecho de que la variable
inputFilename ya
está marcada al comienzo de la función, la verificación se puede eliminar a continuación, lo que hará que el código sea más compacto.
Advertencia 8V556 Se
comparan los valores de diferentes tipos de enumeración: 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; } .... }
En la
declaración de cambio , los valores de enumeración son confusos. Debido a esto, las enumeraciones de diferentes tipos se comparan en un solo lugar:
LabelVerticalAlignment y
AlignCenter .
Advertencia 9V581 Las expresiones condicionales de las declaraciones 'if' ubicadas una al lado de la otra son idénticas. Líneas de verificación: 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); } .... }
El analizador detectó dos expresiones condicionales idénticas en una fila. Hay un error o se pueden combinar dos condiciones en una sola, y así simplificar el código.
Advertencia 10V668 No tiene sentido probar el puntero 'dp' contra nulo, ya que la memoria se asignó utilizando el operador 'nuevo'. La excepción se generará en caso de error de asignación de memoria. windatepicker.cpp 625
static LRESULT DatePickerCreate(HWND hwnd, CREATESTRUCT& cs) { DatePicker* dp = new DatePicker(hwnd, cs); if (dp == NULL) return -1; .... }
El valor del puntero devuelto por el
nuevo operador se compara con cero. Si el operador no pudo asignar memoria, de acuerdo con el estándar del lenguaje C ++, se
produce una excepción
std :: bad_alloc () . Por lo tanto, verificar el puntero a igualdad a cero no tiene sentido.
Tres controles similares más:
- V668 No tiene sentido probar el puntero de 'modos' contra nulo, ya que la memoria se asignó utilizando el operador 'nuevo'. La excepción se generará en caso de error de asignación de memoria. winmain.cpp 2967
- V668 No tiene sentido probar el puntero 'dropTarget' contra nulo, ya que la memoria se asignó utilizando el operador 'nuevo'. La excepción se generará en caso de error de asignación de memoria. winmain.cpp 3272
- V668 No tiene sentido probar el puntero 'appCore' contra nulo, ya que la memoria se asignó utilizando el operador 'nuevo'. La excepción se generará en caso de error de asignación de memoria. winmain.cpp 3352
Advertencia 11V624 Se está utilizando la constante 3.14159265. El valor resultante podría ser inexacto. Considere usar 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); .... }
Se recomienda el diagnóstico, pero es realmente mejor usar una constante lista para el número Pi de la biblioteca estándar.
Conclusión
Recientemente, el proyecto ha sido desarrollado por entusiastas, pero sigue siendo popular y muy solicitado en los programas de capacitación. En Internet hay miles de complementos con diferentes objetos espaciales. Celestia se usó en
The Day After Tomorrow y la serie documental
Through the Wormhole con Morgan Freeman .
Nos complace que al probar
muchos proyectos de código abierto, no solo popularizamos la metodología de análisis de código estático, sino que también contribuimos al desarrollo del mundo de los proyectos abiertos. Por cierto, también puede usar el analizador PVS-Studio no solo para probar los suyos, sino también proyectos de terceros como entusiastas. Para hacer esto, puede usar una de las
opciones de licencia gratuita.
Use analizadores de código estático, haga que sus proyectos sean más confiables y mejores.
Si desea compartir este artículo con una audiencia de habla inglesa, utilice el enlace a la traducción: Svyatoslav Razmyslov.
Celestia: Bugs 'Adventures in Space .