Celestia：虫子的太空历险记

Celestia是一个三维空间模拟器。 对空间的模拟可以从三个维度探索我们的宇宙。 Celestia在Windows，Linux和macOS上可用。 该项目很小，PVS-Studio几乎没有发现任何缺陷。 尽管有这个事实，我们还是要注意它，因为它是一个受欢迎的教育项目，并且对改进它很有用。 顺便说一下，这个程序被用于流行的电影，系列和节目中以显示空间。 反过来，这一事实对代码质量提出了要求。

引言

Celestia项目的官方网站提供了其详细说明。 源代码可在GitHub上获得 。 分析器检查了166个.cpp文件，不包括库和测试。 该项目很小，但是发现的缺陷值得注意。

为了进行源代码分析，我们使用了PVS-Studio静态代码分析器。 Celestia和PVS-Studio都是跨平台的。 我们在Windows平台上分析了该项目。 通过使用Vcpkg -Microsoft库管理器获取依赖关系来构建项目很简单。 根据评论，它不如柯南的能力，但该程序使用起来也很方便。

分析结果

警告1

V501 '<'运算符的左侧和右侧有相同的子表达式：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; } 

复制代码时容易出错。 我们在每条评论中都会对此进行介绍。 显然，在这种情况下，只有静态代码分析才能提供帮助。

程序员复制了条件表达式，但没有完全对其进行编辑。 正确的版本很可能如下所示：

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

关于这个主题的有趣研究：“ 比较函数中的邪恶 ”。

警告2

V575'memset '函数处理'0'元素。 检查第三个论点。 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; .... } 

代码作者混合了memset函数的第二个和第三个参数。 而不是用零填充结构，而是说要填充0字节的内存。

警告3

V595在针对nullptr进行验证之前，已使用“目标”指针。 检查行：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) { .... } .... } 

与NULL进行比较之前， 目标指针已被解除引用两行。 这样的代码可能会导致错误。

警告4

V702类应始终从std :: exception（类似）作为“ public”派生（未指定关键字，因此编译器默认将其设置为“ 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 

分析器已通过private修饰符（默认设置）检测到从std :: exception类继承的类。 这样的继承很危险，因为std :: exception异常不会由于非公共继承而被捕获。 结果，异常处理程序的行为不符合预期。

警告5

V713在针对同一逻辑表达式中的nullptr进行验证之前，在逻辑表达式中使用了指针“ s”。 winmain.cpp 3031

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

在条件表达式的一个片段中，程序员忘记了取消引用s指针。 原来是指针的比较，而不是其值。 在这种情况下，这没有任何意义。

警告6

V773在不释放'vertexShader'指针的情况下退出了该函数。 可能发生内存泄漏。 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; } .... } 

内存由glShader指针释放，但在退出函数时不会被vertexShader指针清除。

以下是类似的片段：

• V773在不释放'fragmentShader'指针的情况下退出了该函数。 可能发生内存泄漏。 modelviewwidget.cpp 1526

警告7

V547表达式'！InputFilename.empty（）'始终为true。 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; } } .... } 

重复检查文件名是否存在。 这不是错误，但是由于在函数开始时已经检查了inputFilename变量，因此可以删除下面的检查，从而使代码更紧凑。

警告8

V556比较不同枚举类型的值：开关（ENUM_TYPE_A）{情况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; } .... } 

枚举值在switch运算符中混合在一起。 因此，在一个片段中比较了不同类型的枚举： LabelVerticalAlignment和AlignCenter 。

警告9

V581彼此并排放置的'if'语句的条件表达式相同。 检查线：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); } .... } 

分析器已连续检测到两个相同的条件表达式。 要么出错，要么可以将两个条件组合为一个条件，从而使代码更简单。

警告10

V668测试'dp'指针是否为null没有意义，因为内存是使用'new'运算符分配的。 如果内存分配错误，将生成异常。 windatepicker.cpp 625

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

将new运算符返回的指针值与null进行比较。 如果操作员无法分配内存，则根据C ++标准，将引发异常std :: bad_alloc（） 。 那么对null的检查是没有意义的。

另外三项类似的检查：

• V668测试“ modes”指针是否为null没有意义，因为使用“ new”运算符分配了内存。 如果内存分配错误，将生成异常。 winmain.cpp 2967
• V668没有针对空测试'dropTarget'指针的意义，因为使用'new'运算符分配了内存。 如果内存分配错误，将生成异常。 winmain.cpp 3272
• V668测试“ appCore”指针是否为null没有意义，因为使用“ new”运算符分配了内存。 如果内存分配错误，将生成异常。 winmain.cpp 3352

警告11

V624正在使用常数3.14159265。 结果值可能不正确。 考虑使用<math.h>中的M_PI常量。 3dstocmod.cpp 62

 int main(int argc, char* argv[]) { .... Model* newModel = GenerateModelNormals(*model, float(smoothAngle * 3.14159265 / 180.0), weldVertices, weldTolerance); .... } 

诊断是可选的，但在这种情况下，最好使用标准库中Pi编号的现成常量。

结论

最近，该项目已由发烧友开发，但仍然很受欢迎，并且在培训计划中仍然有需求。 Internet上有成千上万个具有不同空间对象的插件。 在电影《 明天之后 》和纪录片系列《 与摩根·弗里曼的蠕虫中穿越 》中使用了天体。

我们很高兴，通过使用开源代码检查各种项目，我们不仅促进了静态代码分析方法，而且为开源项目的开发做出了贡献。 顺便说一句，您还可以使用PVS-Studio分析仪不仅可以测试自己的产品，还可以使用爱好者来测试第三方项目。 为此，您可以使用免费许可的选项之一。

使用静态代码分析器，使您的项目更可靠，更好！