0 AD是由志愿者社区开发的具有历史实时性的3D游戏。 代码库很小，我决定对这个游戏进行检查，以免与大型项目（例如Android和XNU Kernel）分离。 因此，我们有一个包含165000行C ++代码的项目。 让我们看看使用PVS-Studio静态分析器可以在其中找到哪些有趣的东西。
0 AD是免费的，开放源代码的古代战争实时战略游戏，由志愿者社区开发（Wildfire Games联合了一组主要开发人员）。 游戏允许控制存在于公元前500年至前1年之间的文明。 截至2018年夏季，该项目处于alpha版本状态。 [描述摘自维基百科]。
- 6个有用的提示和技巧，可以完全控制您的Android设备（无挂机问题）
- 士兵与射线广播— UnityGameDebut
- 多人游戏如何同步其状态？ 第1部分
- 反思377G
- 教程：通过noobtuts.com制作飞扬的小鸟游戏
为什么选择0 AD？
我请我的同事Egor Bredikhin选择并检查了一个小型开源项目，可以在其他任务之间进行研究。 他向我发送了项目日志0 AD。在问了“为什么要这个项目？”之后，他回答：“我刚刚玩了这个游戏，这是一个很好的实时策略”。 确定，然后将其设为0 AD :)。
对于C ++代码的高质量，我想赞扬0 AD的作者。 做得好，我很少遇到如此低的错误密度。 我的意思是，当然，不是所有的错误，而是可以借助PVS-Studio检测到的那些错误。 正如我已经说过的，尽管PVS-Studio并没有发现所有错误，但是尽管如此，您还是可以放心地谈谈错误密度与代码质量之间的联系。
一些数字。 非空白代码行的总数为231270。其中28.7％是注释。 总共有165000行纯C ++代码。
分析仪发出的警告数量很少，查看了所有警告后，我写下了19个错误。 我将在本文后面考虑所有这些错误。 也许我跳过了一些，认为该错误是无害的草率代码。 但是，这不会改变整体情况。
因此，我发现每165000行代码有19个错误。 让我们计算错误的密度：19 * 1000/165000 = 0.115。
为简单起见，我们将进行四舍五入，并假设PVS-Studio分析器在游戏代码中每1000行代码检测到0.1个错误。
好结果！ 为了进行比较，在最近有关Android的文章中，我发现每1000行代码至少发现0.25个错误。 实际上，那里的错误密度更大，我只是没有找到足够的精力来仔细查看整个报告。
另一方面，我们可以举一个库Core EFL库为例，我对它进行了彻底的分析和计数。 PVS-Studio每1000行代码可检测到0.71个错误。
因此，0位AD作者-做得好！ 但是，为了公平起见，应该注意的是，用C ++编写的少量代码对作者有利。 不幸的是，项目越大，其复杂性增长得越快，错误的密度呈非线性增加（更多信息）。
现在让我们看一下我在游戏中发现的19个错误。 为了进行分析，我使用了PVS-Studio分析仪6.24版。 我建议尝试下载演示版本，并测试您正在处理的项目。
注意。 我们将PVS-Studio定位为B2B解决方案。 对于小型项目和个人开发人员，我们有免费的许可证选项：如何免费使用PVS-Studio。
错误N1
让我们开始考虑一个复杂的错误。 实际上，它并不复杂，但是我们必须熟悉一大段代码。
无效 WaterManager :: CreateWaveMeshes（）
{
....
int nbNeighb = 0;
....
发现布尔 =假;
nbNeighb = 0;
对于 （ int p = 0; p <8; ++ p）
{
如果 （CoastalPointsSet.count（xx + around [p] [0] +
（yy +左右[p] [1]）* SideSize））
{
如果 （nbNeighb> = 2）
{
CoastalPointsSet.erase（xx + yy * SideSize）;
继续 ;
}
++ nbNeighb;
//我们发现了一个新的观点。
//移到那里
xx = xx +大约[p] [0]；
yy = yy +周围[p] [1]；
indexx = xx + yy * SideSize;
如果 （i == 0）
Chain.push_back（CoastalPoint（indexx，CVector2D（xx * 2，yy * 2）））;;
其他
Chain.push_front（CoastalPoint（indexx，CVector2D（xx * 2，yy * 2）））;;
CoastalPointsSet.erase（xx + yy * SideSize）;
发现=真;
休息 ;
}
}
如果 （找到）
endChain = true;
....
}
PVS-Studio警告：V547 CWE-570表达式’nbNeighb> = 2’始终为false。 水管理器.cpp 581
乍一看，分析器消息似乎很奇怪。 为什么条件nbNeighb> = 2始终为假？ 在循环的主体中， nbNeighb变量增加了！
看下面，您将看到操作符中断 ，从而中断了循环的执行。 因此，如果变量nbNeighb递增，则循环将停止。 因此，变量nbNeighb的值将永远不会达到大于1的值。
该代码显然包含逻辑错误。
错误N2
虚空
CmpRallyPointRenderer :: MergeVisibilitySegments（
std :: deque 和segment）
{
....
segment.erase（segments.end（））;
....
}
PVS-Studio警告：V783 CWE-119可能会取消引用无效的迭代器“ segments.end（）”。 CCmpRallyPointRenderer.cpp 1290
这段代码很奇怪。 也许，开发人员想从容器中删除最后一个元素。 在这种情况下，正确的代码应如下所示：
segment.erase（segments.back（））;
虽然，甚至可以写出这样一个简单的变体：
segment.pop_back（）;
老实说，我不太明白应该在这里写什么。
错误N3，N4
我决定共同考虑两个错误，因为它们与资源泄漏有关，并且需要显示什么是WARN_RETURN宏。
＃定义WARN_RETURN（状态）\
做\
{\
DEBUG_WARN_ERR（状态）; \
返回状态； \
} \
同时（0）
因此，如您所见，宏WARN_RETURN导致函数主体退出。 现在，我们来看一下使用此宏的混乱方式。
第一个片段。
状态sys_generate_random_bytes （u8 * buf， size_t计数）
{
FILE * f = fopen（“ / dev / urandom”，“ rb”）;
如果 （！f）
WARN_RETURN（ERR :: FAIL）; 同时 （计数）
{
size_t numread = fread（buf，1，count，f）;
如果 （numread == 0）
WARN_RETURN（ERR :: FAIL）;
buf + = numread;
计数-= numread;
} fclose（f）;
返回 INFO :: OK;
}
PVS-Studio警告：V773 CWE-401在不释放’f’手柄的情况下退出了该功能。 资源泄漏是可能的。 unix.cpp 332
如果函数fread无法读取数据，则函数sys_generate_random_bytes将终止而不释放文件描述符。 实际上，这几乎是不可能的。 令人怀疑的是，将无法从“ / dev / urandom”读取数据。 但是，代码编写得很差。
第二个片段。
状态sys_cursor_create （....）
{
....
sys_cursor_impl * impl = 新的 sys_cursor_impl;
impl-> image =图片；
impl-> cursor = XcursorImageLoadCursor（wminfo.info.x11.display，image）;
如果 （impl-> cursor == None）
WARN_RETURN（ERR :: FAIL）; * cursor = static_cast （impl）;
返回 INFO :: OK;
}
PVS-Studio警告：V773 CWE-401在不释放“ impl”指针的情况下退出了该功能。 可能发生内存泄漏。 第421章
如果无法加载游标，则会发生内存泄漏。
错误N5
状态LoadHeightmapImageOs （....）
{
....
shared_ptr fileData = shared_ptr （ new u8 [fileSize]）;
....
}
PVS-Studio警告：V554 CWE-762错误使用了shared_ptr。 分配给’new []’的内存将使用’delete’清除。 MapIO.cpp 54
这是正确的版本：
shared_ptr fileData = shared_ptr （ new u8 [fileSize]）;
错误N6
FUTrackedPtr（ObjectClass * _ptr = NULL）：ptr（_ptr）
{
如果 （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，动作，m_pathname.string（）。c_str（），
（ unsigned long ）m_size）;
返回 buf;
}
PVS-Studio警告：V576 CWE-628错误格式。 考虑检查“ swprintf_s”函数的第五个实际参数。 char类型参数是预期的。 trace.cpp 93
在这里，我们面临着一个混乱而奇怪的故事，即Visual C ++中swprintf函数的另一种实现。 我不会对此进行复述，您可以参考诊断V576上的文档（请参阅“宽字符串”一节）。
在这种情况下，最有可能的是，此代码在Visual C ++ for Windows中编译时将正确运行，而在Linux或macOS中进行编译时将不正确。
类似的错误：V576 CWE-628格式错误。 考虑检查“ swprintf_s”函数的第四个实际参数。 char类型参数是预期的。 vfs_tree.cpp 211
错误N9，N10，N11
经典。 首先，指针已被使用，然后才被检查。
静态 无效 TEST_CAT2 （ char * dst，....）
{
strcpy（dst，dst_val）; // <=
int ret = strcat_s（dst，max_dst_chars，src）;
TS_ASSERT_EQUALS（ret，Expected_ret）;
if （dst！= 0） // <=
TS_ASSERT（！strcmp（dst，Expected_dst））;
}
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; 无效的 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进行比较看起来很奇怪。 我想应该在这里使用其他变量代替bIsOrientationPreserving 。
错误N13
虚拟状态ReplaceFile （ const VfsPath＆pathname，
const shared_ptr ＆fileContents， size_t大小）
{
ScopedLock s;
VfsDirectory *目录；
VfsFile *文件；
状态st;
st = vfs_Lookup（路径名，＆m_rootDirectory，目录，
＆file，VFS_LOOKUP_ADD | VFS_LOOKUP_CREATE）; //没有此类文件，请创建它。
如果 （st == ERR :: VFS_FILE_NOT_FOUND）
{
s。〜ScopedLock（）;
返回 CreateFile（pathname，fileContents，size）;
}
....
}
PVS-Studio警告：V749 CWE-675离开对象范围后，将再次调用“ s”对象的析构函数。 vfs.cpp 165
在创建文件之前，我们需要ScopedLock对象来解锁某些内容。 为此，显式调用析构函数。 麻烦的是，退出函数时，将自动再次调用s对象的析构函数。 即，析构函数将被调用两次。 我没有研究ScopedLock类的配置，但是无论如何都不值得这样做。 通常，对析构函数的这种两次调用会导致未定义的行为或其他不愉快的错误。 即使现在代码可以正常工作，通过更改ScopedLock类的实现，也很容易破坏所有内容。
错误N14，N15，N16，N17
CFsmEvent * CFsm :: AddEvent（ unsigned int eventType）
{
....
pEvent = 新的 CFsmEvent（eventType）;
如果 （！pEvent） 返回 NULL;
....
}
PVS-Studio警告：V668 CWE-570对’pEvent’指针测试是否为null没有意义，因为使用’new’运算符分配了内存。 如果内存分配错误，将生成异常。 fsm.cpp 259
指针检查没有意义，因为在内存分配错误的情况下，将引发异常std :: bad_alloc 。
因此，检查是多余的，但这不是一个严重的错误。 但是， 如果执行不清楚的逻辑，则在操作员体内时，一切都会变得更加糟糕。 让我们考虑这种情况。
CFsmTransition * CFsm :: AddTransition（....）
{
....
CFsmEvent * pEvent = AddEvent（eventType）;
如果 （！pEvent） 返回 NULL; //创建新的过渡
CFsmTransition * pNewTransition = 新的 CFsmTransition（state）;
如果 （！pNewTransition）
{
删除 pEvent;
返回 NULL;
}
....
}
分析器警告：V668 CWE-570对’pNewTransition’指针测试是否为空没有意义，因为使用’new’运算符分配了内存。 如果内存分配错误，将生成异常。 fsm.cpp 289
发生释放内存的尝试，该地址存储在pEvent指针中。 自然，这不会发生，并且会发生内存泄漏。
实际上，当我开始处理这段代码时，结果发现一切都变得更加复杂，也许没有一个错误，而是两个错误。 现在，我将解释这段代码有什么问题。 为此，我们需要熟悉AddEvent函数的配置。
CFsmEvent * CFsm :: AddEvent（ unsigned int eventType）
{
CFsmEvent * pEvent = NULL; //按类型查找事件
EventMap :: iterator it = m_Events.find（eventType）;
如果 （它！= m_Events.end（））
{
pEvent = it-> second;
}
其他
{
pEvent = 新的 CFsmEvent（eventType）;
如果 （！pEvent） 返回 NULL; //将新事件存储到内部地图中
m_Events [eventType] = pEvent;
} return pEvent;
}
请注意，该函数并不总是返回指向使用new运算符创建的新对象的指针。 有时，它从容器m_Events中获取一个现有对象。 顺便说一下，指向新创建对象的指针也将放置在m_Events中 。
这里出现了一个问题：谁拥有并必须销毁存储在容器m_Events中的指针的对象？ 我对这个项目不熟悉，但是很可能是在某个代码破坏了所有对象的地方。 然后删除功能CFsm :: AddTransition中的对象是多余的。
我觉得您可以简单地删除以下代码片段：
如果 （！pNewTransition）
{
删除 pEvent;
返回 NULL;
}
其他错误：
- V668 CWE-571对“ ret”指针针对null进行测试没有任何意义，因为使用“ new”运算符分配了内存。 如果内存分配错误，将生成异常。 TerrainTextureEntry.cpp 120
- V668 CWE-571没有必要测试“答案”指针是否为null，因为使用“ new”运算符分配了内存。 如果内存分配错误，将生成异常。 声音管理器.cpp 542
错误N18，N19
静态 void dir_scan_callback （struct de * de， void * data）{
struct dir_scan_data * dsd =（ struct dir_scan_data *） 数据 ; 如果 （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]））;
}
如果 （dsd-> entries == NULL）{
// TODO（lsm）：将错误传播给调用方
dsd-> num_entries = 0;
} 其他 {
dsd-> entries [dsd-> num_entries] .file_name = mg_strdup（de-> file_name）;
dsd-> entries [dsd-> num_entries] .st = de-> st;
dsd-> entries [dsd-> num_entries] .conn = de-> conn;
dsd-> num_entries ++;
}
}
PVS-Studio警告：V701 CWE-401 realloc（）可能泄漏：当realloc（）分配内存失败时，原始指针’dsd-> entries’丢失。 考虑将realloc（）分配给一个临时指针。 mongoose.cpp 2462
如果数组的大小不足，则使用函数realloc进行内存重新分配。 问题在于，指向源存储块的指针的值会立即被realloc函数返回的新值覆盖。
如果不可能分配内存，则realloc函数将返回NULL，并且该NULL将存储在dsd-> entries变量中。 之后，将不可能释放一块内存，该内存块的地址先前存储在dsd-> entries中 。 将会发生内存泄漏。
另一个错误：V701 CWE-401 realloc（）可能泄漏：当realloc（）分配内存失败时，原始指针’Buffer’丢失。 考虑将realloc（）分配给一个临时指针。 Preprocessor.cpp 84
我不能说这次文章真有趣，或者我设法表现出很多可怕的错误。 这取决于。 我写我所看到的。
感谢您的关注。 我将通过邀请您在Instagram @pvsstudio和Twitter @Code_Analysis中关注我们来完成本文。
