0 AD是由志愿者社区开发的实时历史策略类型的三维游戏。 代码库的大小很小，因此我决定将游戏与大型项目（例如Android和XNU Kernel）分开检查。 因此，摆在我们面前的是一个包含165,000行C ++代码的项目。 让我们看看使用PVS-Studio静态分析器可以发现的有趣之处。
0公元
AD 0 （A.D.）是一款实时的历史策略类型的免费三维游戏，由一个志愿者社区（主要开发者联合在Wildfire游戏团队中）开发。 该游戏允许您控制存在于公元前500年的文明。 e。—公元前1年。 e。 截至2018年夏季，该项目为Alpha版本。 [
描述取自维基百科]。
为什么正好是0 AD？
我请
Egor Bredikhin （
EgorBredikhin ）的同事选择并检查了一些小型的开放项目，我可以用它完成其他任务。 他给我发送了一个0 AD项目的日志，问题是：“为什么要这个项目？” -有一个答案：“是的，我刚刚玩了这个游戏，这是一个很好的实时策略。” 好的，那将是0 AD :)。
误差密度
我要赞扬0 AD的作者的C ++代码的高质量。 做得好，您很少会遇到如此低的错误密度。 当然，这些并非全部错误，而是可以使用PVS-Studio检测到的那些错误。 就像我说的那样，尽管PVS-Studio并没有发现所有错误，但是我们可以放心地讨论错误的密度与整体代码质量之间的关系。
一些数字。 非空代码行的总数为231270。其中，注释占28.7％。 总共有165,000行纯C ++代码。
分析器发出的消息数量很少，查看了所有消息后，我写出了19个错误。 我将在本文后面考虑所有这些错误。 可能我错过了一些东西，认为该错误是无害的草率代码。 但是，通常，这不会改变图片。
因此，我在165,000行代码中发现了19个错误。 我们计算误差密度：19 * 1000/165000 = 0.115。
为简单起见，我们四舍五入并假设PVS-Studio分析器在游戏代码中每1000行代码检测到0.1个错误。
好结果！ 为了进行比较，在最近
有关Android的文章中，我计算出每1000行代码至少检测到0.25个错误。 实际上，错误的密度甚至更大，我只是没有发现仔细分析整个报告的能力。
或以EFL核心库为例，我仔细
研究并计算了缺陷数。 在其中，PVS-Studio每1000行代码检测到0.71个错误。
因此，0 AD的作者是好伙伴，但是，为了公平起见，应注意的是，用C ++编写的少量代码对他们有帮助。 不幸的是，项目越大，其复杂性增长得越快，并且错误密度非线性地增加（
更多 ）。
失误
现在，让我们看一下我在游戏中发现的19个错误。 为了分析该项目，我使用了
PVS-Studio版本6.24。 我建议您尝试下载演示版本并检查您正在处理的项目。
注意事项 我们将PVS-Studio定位为B2B解决方案。 对于小型项目和个人开发人员，我们有一个免费的许可证选项：“
如何免费使用PVS-Studio ”。
错误N1让我们从一个复杂的错误开始。 实际上，它并不复杂，但是您必须熟悉相当大的一段代码。
void WaterManager::CreateWaveMeshes() { .... int nbNeighb = 0; .... bool found = false; nbNeighb = 0; for (int p = 0; p < 8; ++p) { if (CoastalPointsSet.count(xx+around[p][0] + (yy + around[p][1])*SideSize)) { if (nbNeighb >= 2) { CoastalPointsSet.erase(xx + yy*SideSize); continue; } ++nbNeighb;
PVS-Studio
警告 ：
V547 CWE-570表达式'nbNeighb> = 2'始终为false。 水管理器.cpp 581
乍一看，分析器消息似乎很奇怪。 为什么条件
nbNeighb> = 2始终为假？ 实际上，在循环的主体中，变量
nbNeighb的增量是！
看下面，您将看到一个
break语句，该语句中断循环的执行。 因此，如果增加变量
nbNeighb ，则循环将停止。 因此，变量
nbNeighb的值将永远不会达到大于1的值。
该代码显然包含某种逻辑错误。
错误N2 void CmpRallyPointRenderer::MergeVisibilitySegments( std::deque<SVisibilitySegment>& segments) { .... segments.erase(segments.end()); .... }
PVS-Studio警告：
V783 CWE-119可能会取消引用无效的迭代器“ segments.end（）”。 CCmpRallyPointRenderer.cpp 1290
非常非常奇怪的代码。 也许程序员希望从容器中删除最后一个元素。 在这种情况下，代码应如下所示：
segments.erase(segments.end() - 1);
虽然，这样写起来会更容易：
segments.pop_back();
老实说，我目前还不太清楚应该在这里写什么。
错误N3，N4我决定一起考虑两个错误，因为它们与资源泄漏有关，并且需要首先显示
WARN_RETURN宏
是什么。
#define WARN_RETURN(status)\ do\ {\ DEBUG_WARN_ERR(status);\ return status;\ }\ while(0)
因此，如您所见，
WARN_RETURN宏使函数退出主体。 现在，让我们看一下使用此宏的不正确方法。
第一个片段。
Status sys_generate_random_bytes(u8* buf, size_t count) { FILE* f = fopen("/dev/urandom", "rb"); if (!f) WARN_RETURN(ERR::FAIL); while (count) { size_t numread = fread(buf, 1, count, f); if (numread == 0) WARN_RETURN(ERR::FAIL); buf += numread; count -= numread; } fclose(f); return INFO::OK; }
PVS-Studio警告：
V773 CWE-401在不释放'f'手柄的情况下退出了该功能。 资源泄漏是可能的。 unix.cpp 332
如果
fread函数无法读取数据，则
sys_generate_random_bytes函数将退出而不释放文件描述符。 实际上，这几乎是不可能的。 令人怀疑的是，您将无法从“ / dev / urandom”读取数据。 但是，代码很草率。
第二个片段。
Status sys_cursor_create(....) { .... sys_cursor_impl* impl = new sys_cursor_impl; impl->image = image; impl->cursor = XcursorImageLoadCursor(wminfo.info.x11.display, image); if(impl->cursor == None) WARN_RETURN(ERR::FAIL); *cursor = static_cast<sys_cursor>(impl); return INFO::OK; }
PVS-Studio警告：V773 CWE-401在不释放“ impl”指针的情况下退出了该功能。 可能发生内存泄漏。 第421章
如果游标无法加载，则会发生内存泄漏。
错误N5 Status LoadHeightmapImageOs(....) { .... shared_ptr<u8> fileData = shared_ptr<u8>(new u8[fileSize]); .... }
PVS-Studio警告：
V554 CWE-762错误使用了shared_ptr。 分配给'new []'的内存将使用'delete'清除。 MapIO.cpp 54
正确的选项：
shared_ptr<u8[]> fileData = shared_ptr<u8>(new u8[fileSize]);
错误N6 FUTrackedPtr(ObjectClass* _ptr = NULL) : ptr(_ptr) { if (ptr != NULL) FUTracker::TrackObject((FUTrackable*) ptr); ptr = ptr; }
PVS-Studio警告：
V570为其自身分配了“ ptr”变量。 FUTracker.h 122
错误N7，N8 std::wstring TraceEntry::EncodeAsText() const { const wchar_t action = (wchar_t)m_action; wchar_t buf[1000]; swprintf_s(buf, ARRAY_SIZE(buf), L"%#010f: %c \"%ls\" %lu\n", m_timestamp, action, m_pathname.string().c_str(), (unsigned long)m_size); return buf; }
PVS-Studio警告：
V576 CWE-628格式错误。 考虑检查“ swprintf_s”函数的第五个实际参数。 char类型参数是预期的。 trace.cpp 93
在这里，我们遇到了Visual C ++中
swprintf函数的替代实现的困惑和模糊的历史。 我不会
重述 ，但是请参考
V576诊断程序的文档（请参阅“宽线”部分）。
在这种情况下，很可能该代码在Windows的Visual C ++中编译时将正确运行，而在Linux或macOS上进行编译时将不正确。
类似错误：V576 CWE-628格式错误。 考虑检查“ swprintf_s”函数的第四个实际参数。 char类型参数是预期的。 vfs_tree.cpp 211
错误N9，N10，N11经典版 首先，使用指针，然后才检查指针。
static void TEST_CAT2(char* dst, ....) { strcpy(dst, dst_val);
PVS-Studio警告：
V595 CWE-476在针对nullptr进行验证之前，已使用了'dst'指针。 检查行：140，143。test_secure_crt.h 140
我认为该错误不需要解释。 类似警告：
- V595 CWE-476在针对nullptr对其进行验证之前，已使用了'dst'指针。 检查行：150，153。test_secure_crt.h 150
- V595 CWE-476在针对nullptr对其进行验证之前，已使用了'dst'指针。 检查行：314、317。test_secure_crt.h 314
错误N12 typedef int tbool; void MikkTSpace::setTSpace(...., const tbool bIsOrientationPreserving, ....) { .... m_NewVertices.push_back(bIsOrientationPreserving > 0.5 ? 1.0f : (-1.0f)); .... }
V674 CWE-682将'double'类型的'0.5'文字与'int'类型的值进行比较。 考虑检查“ bIsOrientationPreserving> 0.5”表达式。 第137章
比较类型为
int的变量和常量为0.5毫无意义。 此外，就含义而言，这通常是布尔类型的变量，这意味着将其与0.5进行比较看起来很奇怪。 假定此处应使用
bssOrientationPreserving代替另一个变量。
错误N13 virtual Status ReplaceFile(const VfsPath& pathname, const shared_ptr<u8>& fileContents, size_t size) { ScopedLock s; VfsDirectory* directory; VfsFile* file; Status st; st = vfs_Lookup(pathname, &m_rootDirectory, directory, &file, VFS_LOOKUP_ADD|VFS_LOOKUP_CREATE);
PVS-Studio警告：
V749 CWE-675离开对象范围后，将再次调用“ s”对象的析构函数。 vfs.cpp 165
在创建文件之前，必须使
ScopedLock类型的对象“解锁”对象。 为此，显式调用析构函数。 问题是当函数退出时，
s对象的析构函数将再次被自动调用。 即 析构函数将被调用两次。 我尚未研究
ScopedLock类的设备，但是无论如何，您都不应这样做。 通常，对析构函数的这种双重调用会导致未定义的行为或其他不愉快的错误。 即使代码现在工作正常，通过更改
ScopedLock类的实现也很容易破坏所有内容。
错误N14，N15，N16，N17 CFsmEvent* CFsm::AddEvent( unsigned int eventType ) { .... pEvent = new CFsmEvent( eventType ); if ( !pEvent ) return NULL; .... }
PVS-Studio警告：
V668 CWE-570没有针对空测试'pEvent'指针的意义，因为使用'new'运算符分配了内存。 如果内存分配错误，将生成异常。 fsm.cpp 259
检查指针没有意义，因为在发生内存分配错误的情况下，将
引发异常
std :: bad_alloc 。
因此，检查是多余的，但错误并不严重。 但是，
如果在
if语句的主体中执行某些逻辑，则一切都会变得更加糟糕。 考虑以下情况：
CFsmTransition* CFsm::AddTransition(....) { .... CFsmEvent* pEvent = AddEvent( eventType ); if ( !pEvent ) return NULL;
分析器警告：V668 CWE-570没有针对空测试'pNewTransition'指针的意义，因为使用'new'运算符分配了内存。 如果内存分配错误，将生成异常。 fsm.cpp 289
试图释放其地址存储在
pEvent指针中的
内存 。 自然，这不会发生，并且会发生内存泄漏。
实际上，当我开始处理这段代码时，结果发现一切都变得更加复杂，也许不是一个错误，而是两个错误。 现在，我将解释这段代码有什么问题。 为此，我们需要熟悉
AddEvent函数设备。
CFsmEvent* CFsm::AddEvent( unsigned int eventType ) { CFsmEvent* pEvent = NULL;
请注意，该函数并不总是返回指向使用
new运算符创建的新对象的指针。 有时，它从
m_Events容器中获取一个现有对象。 顺便说一下，指向新创建对象的指针也将放置在
m_Events中 。
问题就出现了：谁拥有并应销毁其指针存储在
m_Events容器中的
对象 ？ 我对这个项目不熟悉，但是很可能是某个地方的代码破坏了所有这些对象。 然后，在
CFsm :: AddTransition函数内部删除对象通常
是多余的。
我的印象是，您可以简单地删除以下代码片段：
if ( !pNewTransition ) { delete pEvent; return NULL; }
其他错误：
- V668 CWE-571对“ ret”指针针对null进行测试是没有意义的，因为使用“ new”运算符分配了内存。 如果内存分配错误，将生成异常。 TerrainTextureEntry.cpp 120
- V668 CWE-571没有必要针对空值测试“答案”指针，因为使用“新”运算符分配了内存。 如果内存分配错误，将生成异常。 声音管理器.cpp 542
错误N18，N19 static void dir_scan_callback(struct de *de, void *data) { struct dir_scan_data *dsd = (struct dir_scan_data *) data; if (dsd->entries == NULL || dsd->num_entries >= dsd->arr_size) { dsd->arr_size *= 2; dsd->entries = (struct de *) realloc(dsd->entries, dsd->arr_size * sizeof(dsd->entries[0])); } if (dsd->entries == NULL) {
PVS-Studio
警告 ：
V701 CWE-401 realloc（）可能泄漏：当realloc（）分配内存失败时，原始指针“ dsd->条目”丢失。 考虑将realloc（）分配给一个临时指针。 mongoose.cpp 2462
如果数组的大小不足，则使用
realloc函数分配内存。 错误是指向原始内存块的指针的值立即被
realloc函数返回的新值覆盖。
如果内存分配失败，则
realloc函数将返回NULL，并将此NULL写入
dsd-> entrys变量。 之后，将不可能释放其地址先前存储在
dsd-> entry中的内存块。 将会发生内存泄漏。
另一个错误：V701 CWE-401 realloc（）可能泄漏：当realloc（）分配内存失败时，原始指针“ Buffer”丢失。 考虑将realloc（）分配给一个临时指针。 Preprocessor.cpp 84
结论
我不能说这一次这篇文章引人入胜，或者我设法表现出许多可怕的错误。 一次无需执行任何操作。
我所看到的，然后我写 。
谢谢您的关注。 要进行更改，我将在邀请结束时在Instagram
@pvs_studio_unicorn和Twitter
@Code_Analysis上关注我。
如果您想与说英语的读者分享这篇文章，请使用以下链接：Andrey Karpov。
干得好，游戏的作者0 AD！