不久前,我们的一位同事离开了团队,加入了一家开发嵌入式系统软件的公司。这没什么特别的:在任何公司,人员来来往往都是常事。他们的选择取决于提供的奖金、便利性以及个人偏好。我们感兴趣的是另一件事。我们这位前同事对他现在工作中遇到的代码质量深感担忧。这促使我们写下了这篇联合文章。您看,一旦您明白了静态分析是怎么回事,您就不想再满足于“只是编程”了。
我发现当今世界出现了一种有趣的现象。当一个软件开发部门变成一个与公司基本业务领域没有紧密联系的次要实体时,会发生什么?就会出现一个森林保护区。无论公司的业务领域多么重要和关键(例如,医疗或军事设备),总会出现一片小沼泽,新想法在那里停滞不前,而十年前的技术仍然在使用。
这里是来自一位在核电站软件开发部门工作的人的通信摘录
然后他说,“我们要 git 干什么?你看,我这里都写在我的笔记本上了。”
...
你们有版本控制吗?
2 个人使用 git。其他团队最多使用编号的 zip 文件。虽然只有 1 个人使用 zip 文件,但我对此是确定的。
别害怕。核电站开发的软件可能有不同的用途,而且硬件安全还没有被废除。在那个特定的部门,人们收集和处理统计数据。然而,沼泽化的趋势相当明显。我不知道为什么会发生这种情况,但事实是肯定的。有趣的是,公司越大,沼泽化效应越强烈。
我想指出,大公司的停滞是一个国际现象。国外情况也是如此。有一篇关于这个主题的文章,但我记不起它的标题了。我花了很多时间试图找到它,但徒劳无功。如果有人知道,请给我链接,以便我发布。在那篇文章中,一位程序员讲述了他在某个军事部门工作的经历。那自然是极其秘密和官僚的——如此秘密和官僚,以至于他们花了几个月时间才同意他可以获得哪个级别的访问权限来操作他的电脑。结果,他用记事本写程序(不编译),很快就因效率低下而被解雇了。
现在让我们回到我们的前同事。来到新办公室后,他感到一种文化冲击。你看,在花费了大量时间和精力学习和使用静态分析工具后,看到人们甚至忽略编译器警告,真是令人痛苦。这就像一个独立的世界,他们按照自己的规则编程,甚至使用自己的术语。他告诉我一些关于它的故事,其中我最喜欢的是当地程序员普遍使用的短语“grounded pointers”(接地指针)。看看他们离硬件有多近!
我们很自豪在我们团队中培养了一位技术娴熟、关心代码质量和可靠性的专家。他没有默默接受既定状况,而是努力改进它。
作为开始,他做了以下工作。他研究了编译器警告,然后使用 Cppcheck 检查了项目,并考虑在进行一些修复的同时防止典型错误。
他的第一步是准备一篇旨在提高团队代码质量的文章。将静态代码分析器引入和集成到开发过程中可能是下一步。当然不会是 PVS-Studio:首先,它们在 Linux 下运行;其次,向此类公司销售软件产品非常困难。所以,他目前选择了 Cppcheck。这个工具非常适合人们开始使用静态分析方法。
我邀请您阅读他准备的文章。它题为“你不应该这样写程序”。其中许多条目可能看起来像是“船长明显”风格。然而,这些是该男子试图解决的实际问题。
忽略编译器警告。当警告列表很多时,您可能会轻易错过最近代码中的真正错误。这就是为什么您应该全部处理它们。
在“if”语句的条件判断中,给变量赋值而不是测试该值。
if (numb_numbc[i] = -1) { }
在这种情况下,代码可以正常编译,但编译器会发出警告。正确的代码如下所示:
if (numb_numbc[i] == -1) { }
在头文件中写“using namespace std;”可能会导致包含该头文件的所有文件都使用此命名空间,这反过来可能导致调用错误函数或发生名称冲突。
比较有符号变量和无符号变量
|
|
请记住,混合使用有符号和无符号变量可能导致
上述代码示例无法正确处理“ba”数组为空的情况。“ba.size() - 1”表达式会计算为一个无符号的 size_t 值。如果数组为空,表达式将计算为 0xFFFFFFFFu。
忽略使用常量可能导致难以消除的错误被忽视。例如:
|
|
错误地使用了“=”运算符而不是“==”。如果“str”变量被声明为常量,编译器甚至不会编译代码。
比较字符串指针而不是字符串本身。
|
|
即使字符串“S”存储在变量 TypeValue 中,比较也将始终返回“false”。比较字符串的正确方法是使用特殊的“strcmp”或“strncmp”函数。
缓冲区溢出。
memset(prot.ID, 0, sizeof(prot.ID) + 1);
此代码可能会清除“prot.ID”之后的几字节内存区域。
不要混淆 sizeof() 和 strlen()。sizeof() 运算符返回项目总字节大小。strlen() 函数以字符为单位返回字符串长度(不计算空终止符)。
缓冲区下溢。
|
|
在这种情况下,只有 N 字节会被清除,而不是整个“*ptr”结构(N 是当前平台上的指针大小)。正确的方法是使用以下代码:
|
|
错误的表达式。
if (0 < L < 2 * M_PI) { }
编译器在这里没有看到任何错误,但表达式没有意义,因为执行它时您将始终得到“true”或“false”,具体结果取决于比较运算符和边界条件。编译器会为这样的表达式生成警告。此代码的正确版本是:
if (0 < L && L < 2 * M_PI) { }
|
|
无符号变量不能小于零。
将变量与它永远无法达到的值进行比较。例如:
|
|
编译器对此类情况发出警告。
内存使用“new”或“malloc”分配,但忘记通过“delete”/“free”相应地释放。它可能看起来像这样:
|
|
也许之前保存到“v2”的是指向“std::vector<int>”的指针。现在,由于某些代码部分的修改,它不再需要了,并且只保存了“int”值。同时,为“v1”分配的内存没有被释放,因为以前不需要。为了修复代码,我们应该在函数末尾添加“delete v1”语句,或者使用智能指针。
甚至更好的是将重构进行到底,将“v1”变成局部对象,因为您不再需要将其传递给任何地方。
|
|
内存通过“new[]”分配,并通过“delete”释放。或者反之,内存通过“new”分配,并通过“delete[]”释放。
使用未初始化的变量。
|
|
在 C/C++ 中,变量默认不会初始化为零。有时代码似乎运行良好,但事实并非如此——这仅仅是运气。
函数返回局部对象的引用或指针。
|
|
离开函数时,“FileName”将指向一个已被释放的内存区域,因为所有局部对象都在栈上创建,因此无法进一步正确处理。
未检查函数返回值,而它们可能在出错时返回错误代码或“-1”。可能会发生函数返回错误代码,但我们继续工作而不注意或以任何方式对其做出反应,这将导致程序在某个时刻突然崩溃。此类缺陷需要花费很长时间进行调试。
忽略使用特殊的静态和动态分析工具,以及创建和使用单元测试。
在数学表达式中过于贪婪地添加括号,导致出现以下情况:
D = ns_vsk.bit.D_PN_ml + (int)(ns_vsk.bit.D_PN_st) << 16;
在这种情况下,首先执行加法,然后执行左移。请参阅“C/C++ 中的运算符优先级”。根据程序逻辑,运算符的执行顺序应该是相反的:先移位,然后加法。以下片段中也出现了类似的错误:
|
|
这里的错误是:程序员忘记将 TYPE 宏括在括号中。这导致首先执行“type & A”表达式,然后才执行“(type & A ) | B”表达式。因此,条件始终为真。
数组索引越界。
|
|
“mas[3] = 4;”表达式引用了一个不存在的数组项,因为从“int mas[N]”数组的声明可以看出,其项的索引范围是 [0...N-1]。
逻辑运算符“&&”和“||”的优先级被混淆。 “&&”运算符优先级更高,所以在这种条件中:
if (A || B && C) { }
“B && C”将首先执行,而表达式的其余部分将在之后执行。这可能不符合所需的执行逻辑。通常认为逻辑表达式是从左到右执行的。编译器会为这种可疑的片段生成警告。
分配的值在函数外部无效。
|
|
指针“a”不能被赋予不同的地址值。要做到这一点,您需要这样声明函数:
void foo(int *&a, int b) {....}
或
void foo(int **a, int b) {....}
我没有得出任何具体和重大的结论。我只确定在一个特定地方,软件开发的状况开始改善。这令人高兴。
另一方面,我感到悲伤的是,许多人甚至没有听说过静态分析。而这些人通常负责严肃而重要的事务。编程领域发展非常快。结果,那些一直在“埋头工作”的人未能跟上行业当代趋势和工具。最终,他们的工作效率远低于自由职业者以及从事初创公司和小型公司工作的程序员。
因此,我们得到了一个奇怪的局面。一位年轻的自由职业者可以做得更好(因为他有知识:TDD、持续集成、静态分析、版本控制系统等等),而不是一位在俄罗斯铁路/核电站/...(添加您自己的大型企业变体)工作了 10 年的程序员。谢天谢地,情况并非总是如此。但仍然会发生。
为什么我为此感到悲伤?我希望我们能把 PVS-Studio 卖给他们。但他们甚至没有丝毫怀疑这类工具的存在和有用性。:)