Celestia: as aventuras de insetos no espaço

Celestia é um simulador espacial tridimensional. A simulação espacial nos permite explorar nosso universo em três dimensões. O Celestia está disponível no Windows, Linux e macOS. O projeto é muito pequeno e, usando o PVS-Studio, um número muito pequeno de defeitos é detectado. No entanto, realmente queremos prestar atenção nele, já que este é um projeto educacional popular que é útil para melhorar. A propósito, o programa é usado em filmes, séries e programas populares para representar o espaço. O que também aumenta os requisitos de qualidade do código.

1. Introdução

No site oficial do projeto Celestia , você pode encontrar uma descrição detalhada. O código fonte do projeto está hospedado no GitHub . Excluindo bibliotecas e testes, o analisador verificou 166 arquivos .cpp. O projeto é pequeno, mas os defeitos encontrados são bastante interessantes.

Para analisar o código, usamos o analisador de código estático PVS-Studio . O Celestia e o PVS-Studio são multiplataforma. Para análise, a plataforma Windows foi selecionada. O projeto foi fácil de construir, gerando dependências usando o Vcpkg , o gerenciador de bibliotecas da Microsoft. Segundo as avaliações, é inferior aos recursos do Conan , mas este programa também foi 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 erros 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 opção correta provavelmente é esta:

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

Um estudo interessante sobre esse tópico: "O mal vive em 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 argumentos para a função memset . Em vez de preencher a estrutura com zeros, é indicado 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 mais alto do que em comparação 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 uma classe herdada da classe std :: exception por meio do modificador privado (especificado por padrão). Essa herança é perigosa porque, no caso de herança não pública, ao tentar capturar a exceção std :: exception , ela será ignorada. Como resultado, os manipuladores de exceção não se comportam como 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 um lugar da expressão condicional, eles esqueceram de desreferenciar os ponteiros. O resultado foi uma comparação do ponteiro, não o valor nele. E isso 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; } .... } 

Quando você sai da função, a memória é liberada pelo ponteiro glShader , mas não é apagada pelo ponteiro vertexShader .

O mesmo lugar é mais baixo no código:

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

Verifique novamente o nome do arquivo. Isso não é um erro, mas devido ao fato de a variável inputFilename já estar marcada no início da função, a verificação pode ser removida abaixo, o que tornará 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; } .... } 

Na instrução switch , os valores da enumeração são confusos. Por esse motivo , as enumerações de diferentes tipos são comparadas em um único local: 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 seguidas. Há um erro ou duas condições podem ser combinadas em uma e, assim, simplificar o código.

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 a zero. Se o operador não pôde alocar memória, de acordo com o padrão da linguagem C ++, uma exceção std :: bad_alloc () é lançada . Portanto, verificar o ponteiro de igualdade para zero não faz sentido.

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 é recomendado, mas é realmente melhor usar uma 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 em programas de treinamento. Na Internet, existem milhares de complementos com diferentes objetos espaciais. Celestia foi usada em The Day After Tomorrow e na série de documentários Through the Wormhole com Morgan Freeman .

Estamos satisfeitos por testar muitos projetos de código aberto, não apenas popularizando a metodologia de análise de código estático, mas também contribuindo para o desenvolvimento do mundo dos projetos abertos. A propósito, você também pode usar o analisador PVS-Studio não apenas para testar o seu próprio, mas também projetos 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!

Se você deseja compartilhar este artigo com um público que fala inglês, use o link da tradução: Svyatoslav Razmyslov. Celestia: As aventuras dos insetos no espaço .