Celestia: las aventuras de Bugs en el espacio

Celestia es un simulador espacial tridimensional. La simulación del espacio permite explorar nuestro universo en tres dimensiones. Celestia está disponible en Windows, Linux y macOS. El proyecto es muy pequeño y PVS-Studio detectó algunos defectos en él. A pesar de este hecho, nos gustaría prestarle atención, ya que es un proyecto educativo popular y será bastante útil para mejorarlo de alguna manera. Por cierto, este programa se utiliza en películas, series y programas populares para mostrar el espacio. Este hecho, a su vez, aumenta los requisitos para la calidad del código.

Introduccion

El sitio web oficial del proyecto Celestia proporciona su descripción detallada. El código fuente está disponible en GitHub . El analizador verificó 166 archivos .cpp, excluyendo bibliotecas y pruebas. El proyecto es pequeño, pero los defectos encontrados son notables.

Para hacer el análisis del código fuente utilizamos el analizador de código estático PVS-Studio . Tanto Celestia como PVS-Studio son multiplataforma. Analizamos el proyecto en la plataforma Windows. Fue simple construir el proyecto obteniendo dependencias usando Vcpkg - administrador de la biblioteca de Microsoft. Según las revisiones, es inferior a las capacidades de Conan , pero este programa también fue bastante conveniente de usar.

Resultados de analisis

Advertencia 1

V501 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) // <= 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; } 

Qué fácil es cometer un error al copiar código. Escribimos sobre ello 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 versión correcta es más probable de la siguiente manera:

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

Una investigación interesante sobre este tema: " El mal dentro de las funciones de comparación ".

Advertencia 2

V575 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 de la función memset . En lugar de llenar la estructura con ceros, dice llenar 0 bytes de memoria.

Advertencia 3

V595 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 destino se desreferencia dos líneas antes de compararlo con NULL . Tal código puede conducir potencialmente a un error.

Advertencia 4

Las 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) { } }; // filesystem_error 

El analizador ha detectado la clase heredada de la clase std :: exception a través del modificador privado (configurado de forma predeterminada). Dicha herencia es peligrosa porque la excepción std :: exception no se detectará debido a una herencia no pública. Como resultado, los manejadores de excepciones no se comportan según lo previsto.

Advertencia 5

V713 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 una parte de la expresión condicional, el programador olvidó desreferenciar el puntero s . Resultó ser una comparación del puntero, no su valor. Y no tiene sentido en esta situación.

Advertencia 6

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

El puntero glShader libera la memoria, pero el puntero vertexShader no la borra al salir de la función.

Un fragmento similar a continuación:

• V773 Se salió de la función sin soltar el puntero 'fragmentShader'. Una pérdida de memoria es posible. modelviewwidget.cpp 1526

Advertencia 7

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

Comprobación repetida de la presencia del nombre del archivo. 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 a continuación puede eliminarse, haciendo que el código sea más compacto.

Advertencia 8

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

Los valores de enumeración se mezclan en el operador del interruptor. Debido a esto, las enumeraciones de diferentes tipos se comparan en un fragmento: LabelVerticalAlignment y AlignCenter .

Advertencia 9

V581 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 ha detectado dos expresiones condicionales idénticas en una fila. Se ha cometido un error o se pueden combinar dos condiciones en una sola, y así simplificar el código.

Advertencia 10

V668 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 nulo. Si el operador no pudo asignar memoria, de acuerdo con el estándar C ++, se produce una excepción std :: bad_alloc () . Entonces la comprobación de nulo 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 11

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

El diagnóstico es opcional, pero en este caso es mejor usar la 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 demandado en los programas de capacitación. Hay miles de complementos con diferentes objetos espaciales en Internet. Celestia fue utilizada en la película " El día después de mañana " y en la serie documental " A través del agujero de gusano con Morgan Freeman ".

Nos complace que al verificar varios proyectos con código fuente abierto, no solo promocionamos la metodología de análisis de código estático, sino que también contribuimos al desarrollo de proyectos de código abierto. 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.