Geekly articles weekly

再次进入太空：独角兽恒星如何参观

在他们整个生存期间，人们付出了巨大的努力来研究几乎整个星空。 迄今为止，我们已经检查了成千上万的小行星，彗星，星云和恒星，星系和行星。 要亲自看到所有这些美丽，不必离开屋子并自己买望远镜。 您可以在计算机上安装Stellarium（一个虚拟天象仪），并看着舒适地躺在沙发上的夜空……但是舒适吗？ 为了找到该问题的答案，我们将检查Stellarium中的计算机代码是否有错误。



关于项目的一点点...


根据Wikipedia网站上的描述 ， Stellarium是可用于Linux，Mac OS X，Microsoft Windows，Symbian，Android和iOS以及MeeGo的开源虚拟天象馆。 该程序使用OpenGL和Qt技术实时创建真实的天空。 使用Stellarium，您可以看到用中型甚至大型望远镜看到的东西。 该程序还提供日食和彗星运动的观测。

Stellarium由法国程序员Fabian Chereau创建，他于2001年夏季启动了该项目。 其他杰出的开发商包括Robert Spearman，Johannes Gadzhozik，Matthew Gates，Timothy Reeves，Bogdan Marinov和Johan Meeris，他们负责艺术品。

...关于分析仪


使用PVS-Studio静态代码分析器进行了项目分析。 这是一个用于检测用C，C ++和C＃编写的程序的源代码中的错误和潜在漏洞的工具（Java很快！）。 它可以在Windows，Linux和macOS上运行。 它是为那些需要提高代码质量的人而设计的。

分析非常简单。 首先，我从GitHub 下载了Stellarium项目，然后安装了组装所需的所有软件包。 由于该项目是使用Qt Creator构建的，因此我使用了编译器启动跟踪系统，该系统内置在分析仪的独立版本中。 您可以在那里查看完成的分析报告。

Stellarium的新读者和用户可能会感到奇怪：为什么独角兽出现在文章标题中，它与代码分析有何关系？ 我回答：我是PVS-Studio的开发商之一，而独角兽是我们最喜欢的顽皮吉祥物。 起来了！


我希望感谢本文，使读者能够自己学习一些新东西，而Stellarium开发人员将能够消除一些错误并提高代码质量。

为咖啡配上羊角面包，让自己舒适，因为我们将介绍最有趣的部分-分析结果概述和错误分析！

可疑情况


为了获得更大的阅读乐趣，我建议不要直接看分析仪的警告，而要自己尝试进一步检查错误。

void QZipReaderPrivate::scanFiles() { .... // find EndOfDirectory header int i = 0; int start_of_directory = -1; EndOfDirectory eod; while (start_of_directory == -1) { const int pos = device->size() - int(sizeof(EndOfDirectory)) - i; if (pos < 0 || i > 65535) { qWarning() << "QZip: EndOfDirectory not found"; return; } device->seek(pos); device->read((char *)&eod, sizeof(EndOfDirectory)); if (readUInt(eod.signature) == 0x06054b50) break; ++i; } .... }

PVS-Studio 警告 ： V654循环的条件“ start_of_directory ==-1”始终为true。 qzip.cpp 617

你能找到一个错误吗？ 如果是这样，那就称赞。

错误在于while循环的情况 。 总是如此，因为变量start_of_directory在循环体中不会更改。 循环很可能不会是永恒的，因为它包含return和break ，但是这样的代码看起来很奇怪。

在我看来，他们在代码中忘记了在验证签名的地方进行分配start_of_directory = pos 。 那么break语句可能是多余的。 在这种情况下，可以这样重写代码：

 int i = 0; int start_of_directory = -1; EndOfDirectory eod; while (start_of_directory == -1) { const int pos = device->size() - int(sizeof(EndOfDirectory)) - i; if (pos < 0 || i > 65535) { qWarning() << "QZip: EndOfDirectory not found"; return; } device->seek(pos); device->read((char *)&eod, sizeof(EndOfDirectory)); if (readUInt(eod.signature) == 0x06054b50) start_of_directory = pos; ++i; }

但是，我不确定代码应该是什么样的。 最好项目开发人员自己分析程序的这一部分并进行必要的更改。

另一个奇怪的情况：

 class StelProjectorCylinder : public StelProjector { public: .... protected: .... virtual bool intersectViewportDiscontinuityInternal(const Vec3d& capN, double capD) const { static const SphericalCap cap1(1,0,0); static const SphericalCap cap2(-1,0,0); static const SphericalCap cap3(0,0,-1); SphericalCap cap(capN, capD); return cap.intersects(cap1) && cap.intersects(cap2) && cap.intersects(cap2); } };

PVS-Studio警告： V501 '&&'运算符的左侧和右侧有相同的子表达式'cap.intersects（cap2）'。 StelProjectorClasses.hpp 175

您可能已经猜到了，错误出在函数的最后一行：程序员输入了错误，最后发现无论cap3的值如何，函数都会返回结果。

这种错误非常普遍：几乎在每个经过测试的项目中，我们都遇到了与形式name1和name2之类的名称有关的错别字。 通常，此类错误与复制粘贴有关。

此代码实例是另一个常见错误模式的主要示例，对此我们甚至进行了单独的小型研究。 我的同事安德烈·卡尔波夫（Andrei Karpov）将其称为“ 最后一行效果” 。 如果您不熟悉此材料，建议您在浏览器中打开一个选项卡以供日后阅读，但现在让我们继续。

 void BottomStelBar::updateText(bool updatePos) { .... updatePos = true; .... if (location->text() != newLocation || updatePos) { updatePos = true; .... } .... if (fov->text() != str) { updatePos = true; .... } .... if (fps->text() != str) { updatePos = true; .... } if (updatePos) { .... } }

PVS-Studio警告：

  • V560条件表达式的一部分始终为true：updatePos。 StelGuiItems.cpp 732
  • V547表达式“ updatePos”始终为true。 StelGuiItems.cpp 831
  • V763参数'updatePos'始终在使用前在函数体中重写。 StelGuiItems.cpp 690

在使用updatePos参数之前，其值始终会被覆盖，即 无论传递给它的值如何，该函数都将相同。

看起来很奇怪，不是吗？ 在涉及updatePos参数的所有地方，它都是正确的 。 这意味着条件if（location-> text（）！= NewLocation || updatePos）和if（updatePos）将始终为true。

另一个代码段：

 void LandscapeMgr::onTargetLocationChanged(StelLocation loc) { .... if (pl && flagEnvironmentAutoEnabling) { QSettings* conf = StelApp::getInstance().getSettings(); setFlagAtmosphere(pl->hasAtmosphere() & conf->value("landscape/flag_atmosphere", true).toBool()); setFlagFog(pl->hasAtmosphere() & conf->value("landscape/flag_fog", true).toBool()); setFlagLandscape(true); } .... }

PVS-Studio警告：

  • V792不管左操作数的值如何，都将调用位于运算符'＆'右侧的'toBool'函数。 也许，最好使用“ &&”。 景观经理cpp 782
  • V792不管左操作数的值如何，都将调用位于运算符'＆'右侧的'toBool'函数。 也许，最好使用“ &&”。 景观经理cpp 783

分析器在setFlagAtmosphere和setFlagFog函数的参数中检测到可疑表达式。 确实：在位运算符的两边， 还有 bool类型的值 。 应该使用&&运算符代替＆运算符，现在我将解释原因。

是的，此表达式的结果将始终是正确的。 在使用按位“和”之前，两个操作数都将被提升为int 。 在C ++中，这种转换是明确的 ：将false转换为0，将true转换为1。因此，此表达式的结果将与使用&&运算符相同。

但是有细微差别。 在计算&&运算的结果时，使用了所谓的“惰性计算”。 如果左操作数的值为false ，那么甚至不会计算右值，因为在任何情况下逻辑“和”都将返回false 。 这样做是为了节省计算资源，并允许您编写更复杂的设计。 例如，您可以验证指针是否不为null，如果是，则取消引用该指针以执行其他检查。 示例： if（ptr && ptr-> foo（）） 。

不会使用按位运算符＆来执行这种“惰性计算”：表达式conf-> value（“ ...”，true）.toBool（）每次都会被求值，而与pl-> hasAtmosphere（）的值无关。

在极少数情况下，这是有意为之。 例如，如果计算正确的操作数具有“副作用”，其结果将在以后使用。 这样做也不是一件好事，因为这会使代码的理解复杂化，并使维护复杂化。 另外，未定义计算操作数＆的顺序，因此在某些使用这种“技巧”的情况下，您可以获得不确定的行为。

如果需要保存副作用，请在单独的一行中进行保存，并将结果保存在单独的变量中。 将来将使用此代码的人们将感谢您:)


我们继续下一个主题。

错误的内存处理


让我们从这个片段开始动态内存的主题：

 /************ Basic Edge Operations ****************/ /* __gl_meshMakeEdge creates one edge, * two vertices, and a loop (face). * The loop consists of the two new half-edges. */ GLUEShalfEdge* __gl_meshMakeEdge(GLUESmesh* mesh) { GLUESvertex* newVertex1 = allocVertex(); GLUESvertex* newVertex2 = allocVertex(); GLUESface* newFace = allocFace(); GLUEShalfEdge* e; /* if any one is null then all get freed */ if ( newVertex1 == NULL || newVertex2 == NULL || newFace == NULL) { if (newVertex1 != NULL) { memFree(newVertex1); } if (newVertex2 != NULL) { memFree(newVertex2); } if (newFace != NULL) { memFree(newFace); } return NULL; } e = MakeEdge(&mesh->eHead); if (e == NULL) { return NULL; } MakeVertex(newVertex1, e, &mesh->vHead); MakeVertex(newVertex2, e->Sym, &mesh->vHead); MakeFace(newFace, e, &mesh->fHead); return e; }

PVS-Studio警告：

  • V773在不释放'newVertex1'指针的情况下退出了该函数。 可能发生内存泄漏。 c.312
  • V773在不释放'newVertex2'指针的情况下退出了该函数。 可能发生内存泄漏。 c.312
  • V773在不释放'newFace'指针的情况下退出了该函数。 可能发生内存泄漏。 c.312

该函数为几种结构分配内存，并将其传递给指针newVertex1 ， newVertex2 （有趣的名称，对吗）和newFace指针 。 如果其中之一为零，则释放该函数内部保留的所有内存，然后控制流离开该函数。

如果正确分配了所有三个结构的内存，并且MakeEdge（＆mesh-> eHead）函数返回NULL ，会发生什么情况？ 控制流将到达第二个返回 。

由于指针newVertex1 ， newVertex2和newFace是局部变量，因此在退出函数后它们将不再存在。 但是不会释放属于他们的记忆。 它会保留，但我们将不再有权访问它。

这种情况称为内存泄漏。 出现这种错误的典型情况是：随着程序的长时间使用，它开始消耗越来越多的RAM，直到耗尽为止。

应当注意，在此示例中，第三次返回并非错误。 MakeVertex和MakeFace函数将分配的内存的地址传输到其他数据结构，从而委派了对其释放的责任。

下一个缺陷是该方法，需要90行。 为了方便起见，我减少了它，只留下了问题区域。

 void AstroCalcDialog::drawAngularDistanceGraph() { .... QVector<double> xs, ys; .... }

仅剩一行。 让我给您一个提示：这是xs和ys对象的唯一提及。

PVS-Studio警告：

  • 已创建“ QVector”类型的V808 “ xs”对象，但未使用。 AstroCalcDialog.cpp 5329
  • 已创建“ QVector”类型的V808 “ ys”对象，但未使用。 AstroCalcDialog.cpp 5329

向量xs和ys已创建，但未在任何地方使用。 事实证明，每次使用drawAngularDistanceGraph方法时，都会发生一个空容器的额外创建和删除。 我认为此广告在重构后仍保留在代码中。 这当然不是错误，但是您应该删除多余的代码。

奇怪的类型转换


经过一些格式化后的另一个示例如下所示：

 void SatellitesDialog::updateSatelliteData() { .... // set default buttonColor = QColor(0.4, 0.4, 0.4); .... }

为了理解错误是什么，您将必须查看Qcolor类构造函数的原型：


PVS-Studio警告：

  • V674在调用“ QColor”函数时，将“ double”类型的文字“ 0.4”隐式转换为“ int”类型。 检查第一个参数。 卫星对话框.cpp 413
  • V674在调用“ QColor”函数时，将“ double”类型的文字“ 0.4”隐式转换为“ int”类型。 检查第二个参数。 卫星对话框.cpp 413
  • V674在调用“ QColor”函数时，将“ double”类型的文字“ 0.4”隐式转换为“ int”类型。 检查第三个论点。 卫星对话框.cpp 413

Qcolor类没有接受double类型的构造函数，因此示例中的参数将隐式转换为int 。 这将导致buttonColor对象的字段r ， g和b的值为0 。

如果程序员打算从double类型的值创建对象，则应使用其他构造函数。

例如，您可以通过以下方式使用接受Qrgb的构造函数：

 buttonColor = QColor(QColor::fromRgbF(0.4, 0.4, 0.4));

可以做的不同。 Qt使用[0.0，1.0]范围内的实数值或[0，255]范围内的整数表示RGB颜色。

因此，程序员可以通过如下编写将值从实数转换为整数：

 buttonColor = QColor((int)(255 * 0.4), (int)(255 * 0.4), (int)(255 * 0.4));

或者只是

 buttonColor = QColor(102, 102, 102);

你很无聊吗 别担心：我们面前还有更多有趣的错误。


“太空中的独角兽。” 从恒星的视图。

其他错误


最后，我给你留下了一些好吃的:)让我们开始讨论其中之一。

 HipsTile* HipsSurvey::getTile(int order, int pix) { .... if (order == orderMin && !allsky.isNull()) { int nbw = sqrt(12 * 1 << (2 * order)); int x = (pix % nbw) * allsky.width() / nbw; int y = (pix / nbw) * allsky.width() / nbw; int s = allsky.width() / nbw; QImage image = allsky.copy(x, y, s, s); .... } .... }

PVS-Studio警告： V634 '*'操作的优先级高于'<<'操作的优先级。 表达式中可能应使用括号。 StelHips.cpp 271

好吧，您能够检测到该错误吗？ 更详细地考虑表达式（12 * 1 <<（2 *顺序）） 。 分析器忆及，运算符*的优先级高于移位运算符<<的优先级。 很容易看出，将12乘以1是没有意义的，不需要2 *阶左右的括号。

  ,    : int nbw = sqrt(12 * (1 << 2 * order));     <i>12 </i>     .

注意事项 另外，我想指出的是，如果右操作数' << '的值大于或等于左操作数的位数，则结果未定义。 由于数字文字默认情况下是int ，它需要32位，因此order参数的值不应超过15 。 否则，对表达式的求值可能会导致未定义的行为。

我们继续。 下面的方法非常令人困惑，但是我敢肯定，成熟的阅读器可以处理错误检测:)

 /* inherits documentation from base class */ QCPRange QCPStatisticalBox:: getKeyRange(bool& foundRange, SignDomain inSignDomain) const { foundRange = true; if (inSignDomain == sdBoth) { return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5); } else if (inSignDomain == sdNegative) { if (mKey + mWidth * 0.5 < 0) return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5); else if (mKey < 0) return QCPRange(mKey - mWidth * 0.5, mKey); else { foundRange = false; return QCPRange(); } } else if (inSignDomain == sdPositive) { if (mKey - mWidth * 0.5 > 0) return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5); else if (mKey > 0) return QCPRange(mKey, mKey + mWidth * 0.5); else { foundRange = false; return QCPRange(); } } foundRange = false; return QCPRange(); }

PVS-Studio警告： V779检测到无法访问的代码。 可能存在错误。 qcustomplot.cpp 19512。

事实是，所有if ... else分支都有回报 。 因此，控制流永远不会到达最后两行。

总的来说，该示例将正常运行并正常工作。 但是，仅存在无法访问的代码是一个信号。 在这种情况下，它表明该方法的结构不正确，从而极大地增加了代码的可读性和可理解性。

该代码片段应进行重构，以在输出端获得更简洁的功能。 例如，像这样：

 /* inherits documentation from base class */ QCPRange QCPStatisticalBox:: getKeyRange(bool& foundRange, SignDomain inSignDomain) const { foundRange = true; switch (inSignDomain) { case sdBoth: { return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5); break; } case sdNegative: { if (mKey + mWidth * 0.5 < 0) return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5); else if (mKey < 0) return QCPRange(mKey - mWidth * 0.5, mKey); break; } case sdPositive: { if (mKey - mWidth * 0.5 > 0) return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5); else if (mKey > 0) return QCPRange(mKey, mKey + mWidth * 0.5); break; } } foundRange = false; return QCPRange(); }

我们评论中的最后一个是我最喜欢的错误。 故障点代码简短明了：

 Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { Plane(v1, v2, v3, SPolygon::CCW); }

您注意到可疑之处吗？ 不是每个人都可以:)

PVS-Studio警告： V603已创建对象，但未使用该对象。 如果要调用构造函数，则应使用“ this-> Plane :: Plane（....）”。 平面cpp 29

程序员希望对象的某些字段会在嵌套构造函数中初始化，但结果是这样的：当调用Plane构造函数（Vec3f＆v1，Vec3f＆v2，Vec3f＆v3）时 ，将在其中创建未命名的临时对象，该对象立即被删除。 结果，对象的一部分保持未初始化。

为了使代码正常工作，您应该使用C ++ 11的便捷安全功能-委托构造函数：

 Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3) : Plane(v1, v2, v3, SPolygon::CCW) { distance = 0.0f; sDistance = 0.0f; }

但是，如果将编译器用于该语言的旧版本，则可以这样编写：

 Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { this->Plane::Plane(v1, v2, v3, SPolygon::CCW); }

大概：

 Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3) : distance(0.0f), sDistance(0.0f) { new (this) Plane(v1, v2, v3, SPolygon::CCW); }

我注意到后两种方法非常危险 。 因此，您应该非常小心并充分了解此类方法的工作方式。

结论


关于恒星代码的质量可以得出什么结论？ 老实说，没有太多错误。 此外，在整个项目中，我没有发现任何错误将代码与未定义的行为相关联。 对于开源项目，代码质量很高，为此我向开发人员表示敬意。 你们真棒！ 我很高兴也有兴趣审查您的项目。

那天文馆本身呢-我经常使用它。 不幸的是，住在城市里，我很少能享受到晴朗的夜空，而Stellarium可以让我在世界上任何地方都不必起床。 真的很舒服！

我特别喜欢“星座艺术”模式。 奇特的舞蹈中覆盖着整个天空的巨大人物的景色令人叹为观止。


“一个奇怪的舞蹈。” 从恒星的视图。

世俗的人容易犯错误，而这些错误会泄漏到代码中这一事实并没有什么可耻的。 为此，开发了代码分析工具，例如PVS-Studio。 如果您是其中之一-建议您自己下载并尝试一下 。

希望您对阅读我的文章感兴趣，并且对自己有所了解。 希望开发人员及早纠正发现的错误。

订阅我们的频道，并随时关注来自编程领域的新闻！



如果您想与讲英语的读者分享这篇文章，请使用翻译链接：George Gribkov。 再次进入太空：独角兽如何拜访恒星

Source: https://habr.com/ru/post/zh-CN432954/

More articles:


All Articles