没错,你猜对了——答案是“42”。在这篇文章中,你就会看到关于 C++编程的 42 条建议,
这些建议可以帮助程序员避免很多错误,节省时间和精力。本文的作者是 Andrey Karpov
——“程序验证系统”项目(Program Verification Systems,PVS)的技术总监,他们这个项目组
主要是负责 PVS-Studio 静态代码分析器。检查过那么多开源项目的代码,我们看到过很多
犯错的方式,所以我们有很多经验可以分享给读者。每一条建议我们都用一个例子来解释以
证明问题的时效性。这些建议主要是面向 C/C++开发人员的,但是大多数是通用的,所以可
能对其他语言的开发人员也有益。
前 言
关于作者,我叫 Andrey Karpov。我的兴趣就是 C/C++以及改进代码分析的方法论。在 Visual
C++的五年里,我获得了微软最具价值专家奖(Microsoft MVP)。我的这些文章和工作的首要目的就是让代码更安全。若这些建议能够让你避免一些常见的错误并写出更好的代码,我将不胜荣幸。那些为企业写代码规范的人也可从本文中得到一些有用的信息。
有个小背景。不久前,我创建了一个资源文件夹,在那里我分享了一些对 C++编程比较有用
的技巧。但这个资源文件夹的订阅用户并没有我预料的那么多,所以我觉得没有在这里放链
接的必要。这个文件夹应该会在网上放一段时间,但最终,会被删掉。然而,文件夹中的技
巧依旧有价值。这也是为什么我要更新它们,增加了几条新的技巧,然后把它们整合在一个
单独的文本里。阅读愉快。
1. 别 做 编 译 器 的 活
来看一段选自 MySQL 项目代码块,这段代码包含有一个错误,在 PVS-Studio 代码分析器中
表现为:V525 代码包含一系列相同的代码块。检查在 680、682、684、684、689、691、693、695 行
的“0”、“1”、“2”、“3”、“4”、“1”、“6”项。
解 释
这是一个关于代码复制粘贴的典型错误。很显然,程序员复制代码“ if (a[1] != b[1])
return (int) a[1] - (int) b[1];”,然后开始改变下标,但是忘了将“1”改为“5”。结果导致在这
个比较函数中会返回不正确的值。这个问题比较不容易注意到,而且很难被检测到,因为在
我们使用 PVS-Studio 浏览 MySQL 之前所有的测死用力都没有检测出来。
正 确 代 码 if (a[5] != b[5])
return (int) a[5] - (int) b[5]; 建 议
尽管这段代码简洁、易读,但是开发人员还是比较容易忽视掉这个错误。在阅读类似这样的
代码的时候你会比较难集中精力,因为你所看的都是相似的代码。
这些相似的语句块经常会导致一个结果,就是程序员会想要尽可能的优化代码。他手动“展开循环”。我认为在这里,循环展开并不是一个好法子。
首先,我会怀疑程序员能否真的从这里获得任何益处。现代的编译器都非常的智能,也非常善于在可以提升代码能力的情况下自动展开循环。
其次,这段代码块的 bug 是因为他想要优化代码。如果你把它写成一段比较简单的循环的话,
犯错的几率应该会小一点。
我会建议用下面的方式重写这个函数: static int rr_cmp(uchar *a,uchar *b)
{
for (size_t i = 0; i < 7; ++i)
{
if (a != b)
return a - b;
}
return a[7] - b[7];
}
优 点
我敢肯定这个函数不会比原来那个版本运行慢。
所以,我的建议就是——写简单和易于理解的代码。具体来说,简单的代码就是正确的代码。
不要尝试去做编译器的活——比方说,展开循环。如果展开会比较好的话,编译器会去做的。
做这种手动优化的工作只有在一些比较重要的代码块那里才会有意义,而且这只有在
profiler 已经将这些代码块判断为有问题(慢)的时候才成立。
2. 大于 0 并不意味着是 1
下面这个代码块选自 CoreCLR 项目。这个代码包含一个错误,在 PVS-Studio 分析器中诊断
为:V698 表达式“memcmp(....) == -1”不正确。这个函数的返回值不是只有“-1”,可以是任意
的负数值。可以考虑用“memcmp(....) < 0”来代替。 bool operator( )(const GUID& _Key1, const GUID& _Key2) const
{ return memcmp(&_Key1, &_Key2, sizeof(GUID)) == -1; } 解 释
让我们来看一下 memcmp() 函数的描述: int memcmp ( const void * ptr1, const void * ptr2, size_t num ); 将 ptr1 指向的存储单元里的前 num 个字节和 ptr2 指向的存储单元里的前 num 个字节做比较。
如果它们都相等就返回 0,如果不等,返回一个非 0 的数来表示大于。
返回值:
- 0 ——两个存储单元里的字节不相等,ptr1 里的值大于 ptr2 里的值(如果以无符号char 值作为比较)
注意到如果存储块里的东西不一样的话,函数的返回值会大于或小于 0 。大于或小于。这很重要。你不能将 memcmp(), strcmp(), strncmp(),等等这些函数的返回值与常数 1 或 -1 进行比较。
有趣的是,这段将返回值和 1/-1 进行比较的错误代码,多年来也能返回程序员所期望的值。
但只是运气而已,别无其他。函数的行为可能产生意想不到的变化。比如说,你可能换了一
个编译器,或者开发人员会以一种新的方式优化 memcmp() 函数,那你的代码就不起作用了。
正 确 代 码 bool operator( )(const GUID& _Key1, const GUID& _Key2) const
{ return memcmp(&_Key1, &_Key2, sizeof(GUID)) < 0; } 建 议
不要觉得函数现在起作用就好。如果文档说一个函数可以返回大于和小于 0 的值,那么它就
真的会。它意味着这个函数可以返回-10 ,2 ,或 1024. 你经常见到它返回-1 ,0 ,1 并不意
味着什么。
还有,函数能返回类似于 1024 的事实表明了 memcmp() 的运行结果不能以 char 变量进行排序。
这是一个比较多人会犯的一个错误,其后果很严重。这个错误就是 MySQL/MariaDB
5.1.61、5.2.11, 5.3.5 ,5.5.22 版本以前一些高危漏洞的根源。当用户连上 MySQL/MariaDB
的时候,代码会记录一个值(对 hash 和密码进行 SHA 安全哈希算法后的值),然后这个值
将会用来和 memcmp() 函数的返回值进行比较。但是在一些平台下,返回值会超出 {-128…
127}。结果就是,有 1/256 的几率在比较 hash 值与预期值的时候总返回 true,不管 hash 值是
怎样的。因此,只要一条简单的命令行黑客就能无须密码黑进 MySQL 服务器。原因就是
“sql/password.c”文件里的这部分代码:
typedef char my_bool;
...
my_bool check(...) {
return memcmp(...);
} 3. 一次复制,多次检查
下面这段代码块选自 Audacity 项目。PVS-Studio 检测到的错误是:V501 在“-”的左边和右边
有相同的子表达式。 sampleCount VoiceKey::OnBackward (....) {
...
int atrend = sgn(buffer[samplesleft - 2]-
buffer[samplesleft - 1]);
int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]-
buffer[samplesleft - WindowSizeInt-2]);
...
} 解释
“buffer[samplesleft – WindowSizeInt-2]”这个表达式减去它本身。出现这个错误是因为在复制
代码块( 复制 -粘贴)的时候,程序员忘了把 2 改为 1。
这真是一个很不起眼的错误,但它终究是个错误。这样的错误于程序员而言确实是一个残酷
的事实,所以我们要在这里多讲几遍。我要向它们宣战。
正 确 代 码 int ztrend = sgn(buffer[samplesleft - WindowSizeInt-2]-
buffer[samplesleft - WindowSizeInt-1]); 建 议
在复制代码的时候须谨慎小心。
让程序员拒绝使用复制粘贴是没意义的。因为它太方便,太有用了,我们怎能弃之不用。
所以,要谨慎小心,不要急——凡事欲则立。
记得复制代码可能会引起很多错误。大部分被诊断为 V501 错误的代码都是因复制粘贴引起
的。
如果你要复制代码后编辑它,记得检查一下代码,不要偷懒。
我们稍后会详谈复制粘贴。我希望你能记住,这个问题比我们想象的要严重。
4. 当心三元运算符“?:”,用括号把它括起来
下面这段代码块选自 Haiku 项目(BeOS 的继承者)。这段代码中的错误被 PVS-Studio 诊断
为:V502,有可能“?:”不能返回你所期望的值。“?:”的优先级低于“-”。
[quote]bool IsVisible(bool ancestorsVisible) const
{
int16 showLevel = BView::Private(view).ShowLevel();
return (showLevel - (ancestorsVisible) ? 0 : 1) |
|