Celestia: As aventuras dos insetos no espaço

Celestia é um simulador espacial tridimensional. A simulação do espaço permite explorar nosso universo em três dimensões. O Celestia está disponível no Windows, Linux e macOS. O projeto é muito pequeno e o PVS-Studio detectou poucos defeitos. Apesar disso, gostaríamos de prestar atenção, pois é um projeto educacional popular e será bastante útil para melhorá-lo. A propósito, este programa é usado em filmes, séries e programas populares para mostrar espaço. Esse fato, por sua vez, aumenta os requisitos para a qualidade do código.

1. Introdução

O site oficial do projeto Celestia fornece sua descrição detalhada. O código fonte está disponível no GitHub . O analisador verificou 166 arquivos .cpp, excluindo bibliotecas e testes. O projeto é pequeno, mas os defeitos encontrados são dignos de nota.

Para fazer a análise do código fonte, usamos o analisador de código estático PVS-Studio . O Celestia e o PVS-Studio são multiplataforma. Analisamos o projeto na plataforma Windows. Era simples criar o projeto obtendo dependências usando o Vcpkg - gerenciador de bibliotecas da Microsoft. Segundo as avaliações, é inferior às capacidades de Conan , mas este programa também foi bastante conveniente de usar.

Resultados da análise

Aviso 1

V501 Existem subexpressões idênticas à esquerda e à direita do 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ão fácil é cometer um erro ao copiar código. Escrevemos sobre isso em todas as análises. Aparentemente, apenas a análise de código estático pode ajudar nessa situação.

O programador copiou a expressão condicional e não a editou completamente. A versão correta é mais provável da seguinte maneira:

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

Uma pesquisa interessante sobre esse tópico: " O mal nas funções de comparação ".

Aviso 2

V575 A função 'memset' processa elementos '0'. Inspecione o terceiro 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; .... } 

O autor do código misturou o segundo e o terceiro argumento da função memset . Em vez de preencher a estrutura com zeros, ele diz para preencher 0 bytes de memória.

Aviso 3

V595 O ponteiro de 'destinos' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 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) { .... } .... } 

O ponteiro de destinos é desreferenciado duas linhas antes de ser comparado com NULL . Esse código pode potencialmente levar a um erro.

Aviso 4

As classes V702 sempre devem ser derivadas de std :: exception (e similares) como 'public' (nenhuma palavra-chave foi especificada, então o compilador o padroniza como 'private'). 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 

O analisador detectou a classe herdada da classe std :: exception por meio do modificador privado (definido por padrão). Essa herança é perigosa porque a exceção std :: exception não será capturada devido à herança não pública. Como resultado, os manipuladores de exceção não se comportam conforme o planejado.

Aviso 5

V713 O ponteiro 's' foi utilizado na expressão lógica antes de ser verificado contra nullptr na mesma expressão lógica. winmain.cpp 3031

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

Em uma parte da expressão condicional, o programador esqueceu de desreferenciar o ponteiro s . Acabou sendo uma comparação do ponteiro, não seu valor. E não faz sentido nesta situação.

Aviso 6

V773 A função foi encerrada sem liberar o ponteiro 'vertexShader'. É possível um vazamento de memória. 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; } .... } 

A memória é liberada pelo ponteiro glShader, mas não é apagada pelo ponteiro vertexShader ao sair da função.

Um fragmento semelhante abaixo:

• V773 A função foi encerrada sem liberar o ponteiro 'fragmentShader'. É possível um vazamento de memória. modelviewwidget.cpp 1526

Aviso 7

A expressão V547 '! InputFilename.empty ()' sempre é verdadeira. 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; } } .... } 

Verificação repetida da presença do nome do arquivo. Não é um bug, mas devido ao fato de a variável inputFilename já estar marcada no início da função, a verificação abaixo pode ser removida, tornando o código mais compacto.

Aviso 8

V556 Os valores de diferentes tipos de enumerações são comparados: 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; } .... } 

Os valores de enumeração são misturados no operador do comutador. Por esse motivo , enumerações de tipos diferentes são comparadas em um fragmento: LabelVerticalAlignment e AlignCenter .

Aviso 9

V581 As expressões condicionais das instruções 'if' localizadas uma ao lado da outra são idênticas. Verifique as linhas: 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); } .... } 

O analisador detectou duas expressões condicionais idênticas em uma linha. Um erro foi cometido ou duas condições podem ser combinadas em uma e, assim, tornar o código mais simples.

Aviso 10

V668 Não há sentido em testar o ponteiro 'dp' contra nulo, pois a memória foi alocada usando o operador 'novo'. A exceção será gerada no caso de erro de alocação de memória. windatepicker.cpp 625

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

O valor do ponteiro retornado pelo novo operador é comparado com nulo. Se o operador não conseguiu alocar memória, de acordo com o padrão C ++, uma exceção std :: bad_alloc () é lançada. Em seguida, a verificação de nulo é inútil.

Mais três verificações semelhantes:

• V668 Não há sentido em testar o ponteiro dos 'modos' contra nulo, pois a memória foi alocada usando o operador 'novo'. A exceção será gerada no caso de erro de alocação de memória. winmain.cpp 2967
• V668 Não há sentido em testar o ponteiro 'dropTarget' contra nulo, pois a memória foi alocada usando o operador 'new'. A exceção será gerada no caso de erro de alocação de memória. winmain.cpp 3272
• V668 Não há sentido em testar o ponteiro 'appCore' contra nulo, pois a memória foi alocada usando o operador 'novo'. A exceção será gerada no caso de erro de alocação de memória. winmain.cpp 3352

Aviso 11

V624 A constante 3.14159265 está sendo utilizada. O valor resultante pode ser impreciso. Considere usar a 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); .... } 

O diagnóstico é opcional, mas, neste caso, é melhor usar a constante pronta para o número Pi da biblioteca padrão.

Conclusão

Recentemente, o projeto foi desenvolvido por entusiastas, mas ainda é popular e demanda nos programas de treinamento. Existem milhares de complementos com diferentes objetos espaciais na Internet. Celestia foi usada no filme " O dia depois de amanhã " e na série de documentários " Through the Wormhole with Morgan Freeman ".

Estamos felizes por verificar vários projetos com código-fonte aberto não apenas promovendo a metodologia estática de análise de código, mas também contribuindo para o desenvolvimento de projetos de código-fonte aberto. A propósito, você também pode usar o analisador PVS-Studio não apenas para testar seus próprios projetos, mas também de terceiros como entusiasta. Para fazer isso, você pode usar uma das opções de licenciamento gratuito.

Use analisadores de código estático, torne seus projetos mais confiáveis ​​e melhores!