戈多：关于定期使用静态代码分析器的问题

我们的读者人数在增长，因此我们一遍又一遍地撰写文章，说明如何正确使用静态代码分析方法。 我们认为，解释静态分析工具不应偶尔使用，而应定期使用是非常重要的。 我们再次通过一个实际的例子来证明这一点，重新检查Godot项目。

定期使用分析仪

准备在游戏开发者大会上发言时，我决定获得PVS-Studio工具可以检测到的有趣错误的新示例。 为此，测试了多个游戏引擎，其中之一是Godot。 我没有发现报告中有任何特别有趣的错误，但是我想写一篇有关普通错误的文章。 这些错误很好地证明了定期使用静态代码分析工具的相关性。

应当指出的是，我们已经在2015年检查了这个项目，作者处理了我们写出的错误。 在这里您可以看到相应的提交。

3年过去了。 项目已更改。 PVS-Studio分析仪也进行了更改，并且出现了新的诊断方法。 因此，毫不奇怪的是，我能够轻松快速地写出足够的错误示例来撰写本文。

但是，还有其他重要的事情。 在开发Godot或任何其他项目时，新错误不断出现并得到修复。 未检测到的错误在代码中长期存在，因此在运行静态代码分析时可以检测到许多错误。 因此，有时会产生一种错误的感觉，即静态分析器在很少使用的代码段中仅发现一些有趣的错误。 当然，如果您不正确地使用分析仪而仅不时地运行它（例如，在发行版发行前不久），就会出现这种情况。

是的，在撰写文章时，我们自己对开放项目进行一次性检查。 但是我们有一个不同的目标。 我们想证明代码分析器检测缺陷的功能。 这项任务与提高整个项目代码的质量以及减少与纠错相关的成本无关。

再次谈到主要问题。 静态代码分析的重点不是发现旧错误。 这些旧错误通常很小，否则它们会干扰用户，并且已经找到并修复了。 静态分析的重点是快速修复新编写或修改的代码中的错误。 这减少了调试时间，减少了用户投诉的数量，并最终减少了开发项目的预算。

现在，让我们继续研究我们的出版物读者非常喜欢的错误。

复制粘贴错误

让我们看看我在研究PVS-Studio报告时注意到的内容。 我将从最喜欢的V501诊断程序开始，该诊断程序会在我们检查的几乎每个项目中都发现错误 ：)。

错误N1

virtual bool can_export(....) { .... if (!exists_export_template("uwp_" + platform_infix + "_debug.zip", &err) || !exists_export_template("uwp_" + platform_infix + "_debug.zip", &err)) { valid = false; r_missing_templates = true; } .... } 

PVS-Studio警告：V501 CWE-570'||'的左侧和右侧有相同的子表达式'！Exists_export_template（“ uwp_” + platform_infix +“ _debug.zip”＆err）'。 操作员。 出口.cpp 1135

经典复制粘贴错误。 该函数调用已被复制但尚未编辑。 要处理的第二个文件的名称必须以“ _release.zip”结尾。

错误N2，N3

 static String dump_node_code(SL::Node *p_node, int p_level) { .... if (bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW || bnode->statements[i]->type == SL::Node::TYPE_CONTROL_FLOW) { code += scode; //use directly } else { code += _mktab(p_level) + scode + ";\n"; } .... } 

PVS-Studio警告：V501 CWE-570'||'的左侧和右侧有相同的子表达式'bnode->语句[i]-> type == SL :: Node :: TYPE_CONTROL_FLOW' 操作员。 test_shader_lang.cpp 183

 void EditorSpinSlider::_notification(int p_what) { if (p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT || p_what == MainLoop::NOTIFICATION_WM_FOCUS_OUT) { if (grabbing_spinner) { Input::get_singleton()->set_mouse_mode(Input::MOUSE_MODE_VISIBLE); grabbing_spinner = false; grabbing_spinner_attempt = false; } } .... } 

PVS-Studio警告：V501 CWE-570在'||'的左右两侧有相同的子表达式'p_what == MainLoop :: NOTIFICATION_WM_FOCUS_OUT' 操作员。 editor_spin_slider.cpp 157

我认为这些错误清晰可见，不需要任何解释。 与第一种情况完全相同的经典复制粘贴。

错误N4

 String SoftBody::get_configuration_warning() const { .... Transform t = get_transform(); if ((ABS(t.basis.get_axis(0).length() - 1.0) > 0.05 || ABS(t.basis.get_axis(1).length() - 1.0) > 0.05 || ABS(t.basis.get_axis(0).length() - 1.0) > 0.05)) { if (!warning.empty()) .... } 

PVS-Studio警告：V501 CWE-570'||'的左侧和右侧有相同的子表达式 操作员。 第399章

在这里，第一行被复制了两次。 但是坐标轴号仅在第二行中更改。 他们忘了编辑第三行。 这就是“ 最后一行效果 ”。

注意事项 目前，除了“最后一行效果”之外，我还做了以下有趣的观察：“ C / C ++世界中最危险的功能 ”，“ 邪恶存在于比较功能中 ”。 现在，我将宣布一个新文章，我打算在不久的将来写这篇文章。 工作标题是“ 0、1、2”。 它应该是有趣且有启发性的。 我邀请您订阅一个频道，以免错过： twitter ， vk.com ， Instagram ， 电报和“ old school” rss 。

错误N5

 void ScrollContainer::_notification(int p_what) { .... if (h_scroll->is_visible_in_tree() && h_scroll->get_parent() == this) size.y -= h_scroll->get_minimum_size().y; if (v_scroll->is_visible_in_tree() && v_scroll->get_parent() == this) size.x -= h_scroll->get_minimum_size().x; .... } 

PVS-Studio警告：V778 CW​​E-682发现了两个相似的代码片段。 也许这是一个错字，应该使用“ v_scroll”变量而不是“ h_scroll”。 scroll_container.cpp 249

关于这段代码，我不能完全确定是否有错误。 但是，我同意分析器的观点，第二个块看起来非常可疑。 该代码很可能是使用Copy-Paste编写的，在第二段文本中，他们忘记了用v_scroll替换h_scroll 。

该代码可能应该是这样的：

 if (h_scroll->is_visible_in_tree() && h_scroll->get_parent() == this) size.y -= h_scroll->get_minimum_size().y; if (v_scroll->is_visible_in_tree() && v_scroll->get_parent() == this) size.x -= v_scroll->get_minimum_size().x; 

错误N6

另一种情况是复制了足够大的代码片段，但未成功更改。 错误行由我的注释“ // <=“标记。

 void ShaderGLES2::bind_uniforms() { .... const Map<uint32_t, Variant>::Element *E = uniform_defaults.front(); while (E) { int idx = E->key(); int location = version->uniform_location[idx]; if (location < 0) { E = E->next(); continue; } Variant v; v = E->value(); _set_uniform_variant(location, v); E = E->next(); } const Map<uint32_t, CameraMatrix>::Element *C = uniform_cameras.front(); while (C) { int idx = E->key(); // <= int location = version->uniform_location[idx]; if (location < 0) { C = C->next(); continue; } glUniformMatrix4fv(location, 1, GL_FALSE, &(C->get().matrix[0][0])); C = C->next(); } uniforms_dirty = false; } 

PVS-Studio警告：V522 CWE-476可能会取消引用空指针“ E”。 shader_gles2.cpp 102

间接检测到错误。 通过对数据流的分析， PVS-Studio显示指针E在解引用时可能为零。

错误是，在复制的代码片段中，他们忘记了一次替换C中的E。 由于该错误，该函数以一种非常奇怪的方式工作并且做了奇怪的事情。

错别字

错误N7

对于使用C或C ++以外的语言编写的程序员，很难想象可以通过写逗号而不是星号*来打错文字，并且代码将被编译。 但是，是这样。

 LRESULT OS_Windows::WndProc(....) { .... BITMAPINFO bmi; ZeroMemory(&bmi, sizeof(BITMAPINFO)); bmi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER); bmi.bmiHeader.biWidth = dib_size.x; bmi.bmiHeader.biHeight = dib_size.y; bmi.bmiHeader.biPlanes = 1; bmi.bmiHeader.biBitCount = 32; bmi.bmiHeader.biCompression = BI_RGB; bmi.bmiHeader.biSizeImage = dib_size.x, dib_size.y * 4; .... } 

PVS-Studio警告：V521 CWE-480这种使用'，'运算符的表达式很危险。 确保表达式正确。 os_windows.cpp 776

变量bmi.bmiHeader.biSizeImage被分配了变量dib_size.x的值。 接下来，执行逗号运算符'，'，其优先级低于运算符'='的优先级 。 绝不使用表达式dib_size.y * 4的结果。

应该使用乘法运算符'*'代替表达式中的逗号。 首先，这样的表达是有意义的。 其次，下面有一个相似但已经正确的选项用于初始化相同的变量：

 bmi.bmiHeader.biSizeImage = dib_size.x * dib_size.y * 4; 

错误N8，N9

 void Variant::set(....) { .... int idx = p_index; if (idx < 0) idx += 4; if (idx >= 0 || idx < 4) { Color *v = reinterpret_cast<Color *>(_data._mem); (*v)[idx] = p_value; valid = true; } .... } 

PVS-Studio警告：V547 CWE-571表达式'idx> = 0 || idx <4'始终为true。 variant_op.cpp 2152

任何索引都将被认为是正确的。 要解决该错误，您需要替换||。 在&&上 ：

 if (idx >= 0 && idx < 4) { 

这种逻辑错误很可能是由于粗心而引起的，因此我倾向于将其归因于错别字。

完全相同的错误可以在下面的同一文件中观察到。 繁殖错误的错误显然是复制粘贴。

多个错误：V547 CWE-571表达式'idx> = 0 || idx <4'始终为true。 variant_op.cpp 2527

错误N10

一个错误的例子：WTF？！

 void AnimationNodeBlendSpace1D::add_blend_point( const Ref<AnimationRootNode> &p_node, float p_position, int p_at_index) { ERR_FAIL_COND(blend_points_used >= MAX_BLEND_POINTS); ERR_FAIL_COND(p_node.is_null()); ERR_FAIL_COND(p_at_index < -1 || p_at_index > blend_points_used); if (p_at_index == -1 || p_at_index == blend_points_used) { p_at_index = blend_points_used; } else { for (int i = blend_points_used - 1; i > p_at_index; i++) { blend_points[i] = blend_points[i - 1]; } } .... } 

PVS-Studio警告：V621 CWE-835考虑检查“ for”操作员。 循环有可能执行不正确或根本不执行。 animation_blend_space_1d.cpp 113

注意循环停止条件： i> p_at_index 。 始终如此，因为变量i是使用值blend_points_used-1初始化的。 此外，从之前的两次检查中可以得出blend_points_used> p_at_index 。

仅当符号变量i发生溢出（这是未定义的行为）时，条件才能变为false。 此外，它不会达到溢出状态，因为阵列会更早地超出边界。

我认为摆在我们面前的是错字，他们写了'>'而不是'<'。 是的，我对错误之美的看法是错误的：)。

正确的周期：

 for (int i = blend_points_used - 1; i < p_at_index; i++) { 

错误N11

在周期情况下，错字的另一个同样明亮的情况。

 void AnimationNodeStateMachineEditor::_state_machine_pos_draw() { .... int idx = -1; for (int i = 0; node_rects.size(); i++) { if (node_rects[i].node_name == playback->get_current_node()) { idx = i; break; } } .... } 

PVS-Studio警告：V693 CWE-835考虑检查循环的条件表达式。 可能应该使用“ i <X.size（）”代替“ X.size（）”。 animation_state_machine_editor.cpp 852

由于i的值不受控制地增加，因此可能会发生数组溢出。 安全代码：

 for (int i = 0; i < node_rects.size(); i++) { 

错误N12

 GDScriptDataType GDScriptCompiler::_gdtype_from_datatype( const GDScriptParser::DataType &p_datatype) const { .... switch (p_datatype.kind) { .... case GDScriptParser::DataType::NATIVE: { result.kind = GDScriptDataType::NATIVE; result.native_type = p_datatype.native_type; } break; case GDScriptParser::DataType::SCRIPT: { result.kind = GDScriptDataType::SCRIPT; result.script_type = p_datatype.script_type; result.native_type = result.script_type->get_instance_base_type(); } case GDScriptParser::DataType::GDSCRIPT: { result.kind = GDScriptDataType::GDSCRIPT; result.script_type = p_datatype.script_type; result.native_type = result.script_type->get_instance_base_type(); } break; .... } 

PVS-Studio警告：V796 CWE-484 switch语句中可能缺少'break'语句。 gdscript_compiler.cpp 135

一不小心忘了写一个break语句。 因此，当涉及到GDScriptParser :: DataType :: SCRIPT时，将为变量分配值，就像GDScriptParser :: DataType :: GDSCRIPT一样 。

错误N13

以下错误可以归类为“复制粘贴”。 但是，我不确定是否复制了这么短的一行。 因此，我们将在键入时将其视为简单的错字。

 void CPUParticles::_particles_process(float p_delta) { .... if (flags[FLAG_DISABLE_Z]) { p.velocity.z = 0.0; p.velocity.z = 0.0; } .... } 

PVS-Studio警告：V519 CWE-563'p.velocity.z'变量已连续两次分配值。 也许这是一个错误。 检查行：664、665。cpu_particles.cpp 665

两次分配相同的变量。 在下面，您可以看到以下代码片段：

 if (flags[FLAG_DISABLE_Z]) { p.velocity.z = 0.0; p.transform.origin.z = 0.0; } 

对于第一种情况，很可能应该以相同的方式编写。

错误N14

 bool AtlasTexture::is_pixel_opaque(int p_x, int p_y) const { if (atlas.is_valid()) { return atlas->is_pixel_opaque( p_x + region.position.x + margin.position.x, p_x + region.position.y + margin.position.y ); } return true; } 

PVS-Studio警告：V751参数“ p_y”未在功能体内使用。 第1085章

V751诊断说明的片段：

分析仪检测到可疑功能，从未使用过其中一个参数。 同时，其另一个参数被使用了几次，这可能表明存在错误。

如您所见，确实如此，而且非常可疑。 变量p_x被使用两次，而p_y不被使用。 很可能应该这样写：

 return atlas->is_pixel_opaque( p_x + region.position.x + margin.position.x, p_y + region.position.y + margin.position.y ); 

顺便说一下，在源代码中，函数调用是一行编写的。 因此，错误更难以发现。 如果代码的编写者像我在文章中那样在列中编写了实际的参数，那么错误将立即引起我的注意。 请记住，表格式非常有用，可以避免很多错别字。 请参阅文章“ 编程，重构的主要问题以及所有这些 。”一文中的“使用表”将相同类型的代码对齐。

错误N15

 bool SpriteFramesEditor::can_drop_data_fw(....) const { .... Vector<String> files = d["files"]; if (files.size() == 0) return false; for (int i = 0; i < files.size(); i++) { String file = files[0]; String ftype = EditorFileSystem::get_singleton()->get_file_type(file); if (!ClassDB::is_parent_class(ftype, "Texture")) { return false; } } .... } 

PVS-Studio警告：V767通过循环内的常量索引可疑访问“文件”数组的元素。 sprite_frames_editor_plugin.cpp 602

循环在循环的所有迭代中处理相同的文件。 错字在这里：

 String file = files[0]; 

必须是：

 String file = files[i]; 

其他错误

错误N16

 CSGBrush *CSGBox::_build_brush() { .... for (int i = 0; i < 6; i++) { .... if (i < 3) face_points[j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); else face_points[3 - j][(i + k) % 3] = v[k] * (i >= 3 ? -1 : 1); .... } .... } 

PVS-Studio分析仪会立即对此代码产生两个响应：

• V547 CWE-570表达式'i> = 3'始终为假。 csg_shape.cpp 939
• V547 CWE-571表达式'i> = 3'始终为true。 csg_shape.cpp 941

确实，这两个表达式中的这个三元运算符看起来都很奇怪：

 i >= 3 ? -1 : 1 

在一种情况下，条件始终为true，在另一种情况下为false。 很难说这段代码看起来应该如何。 也许这只是多余的，可以这样写：

 for (int i = 0; i < 6; i++) { .... if (i < 3) face_points[j][(i + k) % 3] = v[k]; else face_points[3 - j][(i + k) % 3] = -v[k]; .... } 

我可能是错的，并且需要以完全不同的方式来修复代码。

错误N17

尽管几乎在任何项目中都经常发现 V595，但几乎没有像V595这样的错误。 显然，这些错误在之前的检查之后已得到修复，然后几乎没有出现这种类型的错误。 我只看到了一些误报和一个错误。

 bool CanvasItemEditor::_get_bone_shape(....) { .... Node2D *from_node = Object::cast_to<Node2D>( ObjectDB::get_instance(bone->key().from)); .... if (!from_node->is_inside_tree()) return false; //may have been removed if (!from_node) return false; .... } 

PVS-Studio警告：V595 CWE-476在针对nullptr对其进行验证之前，已使用'from_node'指针。 检查行：565，567。canvas_item_editor_plugin.cpp 565

首先取消引用from_node指针以调用is_inside_tree函数，然后才检查其是否为nullptr相等性。 支票应互换：

 if (!from_node) return false; if (!from_node->is_inside_tree()) return false; //may have been removed 

错误N18

 enum JoystickList { .... JOY_AXIS_MAX = 10, .... }; static const char *_axes[] = { "Left Stick X", "Left Stick Y", "Right Stick X", "Right Stick Y", "", "", "L2", "R2" }; int InputDefault::get_joy_axis_index_from_string(String p_axis) { for (int i = 0; i < JOY_AXIS_MAX; i++) { if (p_axis == _axes[i]) { return i; } } ERR_FAIL_V(-1); } 

PVS-Studio警告：V557 CWE-125阵列可能超限。 “ i”索引的值可能达到9。input_default.cpp 1119

_axes数组由八个元素组成。 在这种情况下，设置循环迭代次数的常量JOY_AXIS_MAX为10。事实证明，循环超出了数组边界。

错误N19

最后一个很奇怪的功能，显然是为了测试某些东西而设计的。 该功能很长，因此我将其作为图片给出（单击图片放大）。

PVS-Studio警告：V779 CWE-561检测不到代码。 可能存在错误。 test_math.cpp 457

函数中有几个无条件的返回语句 。 在图片中，我用红色椭圆形标记了它们。 似乎为此功能收集了几个不同的单元测试，但忘记删除多余的return NULL 。 结果，该功能不检查应检查的内容。 函数的几乎整个主体都包含无法访问的代码。

当然，也许这是一种狡猾的想法。 但是在我看来，这是偶然发生的，因此代码应该固定。

让我们就此结束。 当然，如果您仔细查看分析仪的报告，还会发现其他错误。 但是，即使写出来写一篇文章也绰绰有余。 然后，这对我和读者来说将变得无聊：)。

结论

本文介绍了如果使用PVS-Studio定期分析代码则不会出现的错误。 但是，更重要的是，使用常规分析，可以立即发现并消除许多其他错误。 我的同事在他的笔记中更详细地描述了这个想法：“ 静态代码分析的哲学：我们有100个程序员，分析器发现了很少的错误，这没用吗？ ”。 我强烈建议您花10分钟阅读这篇简短但非常重要的文章。

谢谢您的关注。 我邀请所有人下载并尝试对PVS-Studio进行静态分析以测试您自己的项目。

如果您想与说英语的读者分享这篇文章，请使用以下链接：Andrey Karpov。 Godot：关于静态分析仪的常规使用 。