31. 在 C 和C++中,数组不是按值传递的
下面的代码来自游戏'Wolf'.代码中包含的错误被 PVS-Studio诊断为:V511在'sizeof (src)'这个表达式中,sizeof()返回的是指针的大小,而不是数组的大小。
解释
有时候,程序员会忘记在C/C++中,你不可以把一个数组值传递给一个函数。因为传数组给一个函数,数组类型自动转换为指针类型。方括号里里的数字没什么意思,它们只是用来告诉程序员,多大的数组被传进去了。事实上,你可以传任意大小的数组。比如,下面的代码也能编译成功:
void F(int p[10]) { }
void G()
{
int p[3];
F(p);
}
类似的,sizeof(src) 不是计算数组的大小,而是指针的大小。然后结果就是,memcpy() 只是复制了一部分数组而已。也就是,4或者8字节,这都取决于指针的大小(不算奇怪的结构)。
正确代码
最简单的修正是这样的:
ID_INLINE mat3_t::mat3_t( float src[ 3 ][ 3 ] ) {
memcpy(mat, src, sizeof(float) * 3 * 3);
}
建议
有多种方法可以让你的代码更安全。
知道数组大小。你可以在函数中用数组的引用做参数。但并不是所有人都知道可以这么做。甚至更少的人知道怎么写。所以我希望下面的例子能够有用,有趣:
ID_INLINE mat3_t::mat3_t( float (&src)[3][3] )
{
memcpy( mat, src, sizeof( src ) );
}
现在,就可以在传递数组的时候只用右边的部分。而且最重要的是,sizeof()计算的是数组的大小了,不再是一个指针的。
解决这个问题的另一个方法是用std::array类。
不知道数组大小。有些编程书的作者建议使用std::vector类和其他相似的类。但是,这实际应用做,并不总是那么方便。
有时,你也会想要用指针来解决这个问题。在这个例子中,你可以传两个参数给那个函数:一个指针,元素个数。然而,一般来说,这样做都不太好,因为它会导致很多bug。
在这样的例子中,"C++ Core Guidelines"这篇文章里的观点蛮有用的。我建议读"Do not pass an array as a single pointer"。总的来说,在你有空的时候读"C++ Core Guidelines"真的不失为一件有益的事。它里面包含了很多有用的观点。
32. 危险的 printf
下面的代码选自 TortoiseSVN 项目。代码中包含的错误被 PVS-Studio 诊断为:V618 以一种危险的方式调用 printf 函数,因为传递进去的那一行应该包含格式化说明。安全使用 printf 的例子:: printf("%s", str);
BOOL CPOFile::ParseFile(....)
{
....
printf(File.getloc().name().c_str());
....
}
解释
当你打算打印或者,比如说,写一个字符串到文件中,很多程序员会这样写:
printf(str);
fprintf(file, str);
一个优秀的程序员应该时刻记得,这样组织代码是非常不安全的。事情是这样的,如果格式化说明不知怎样进入一个字符串里,将会导致无法预测的后果。
让我们回过头来看最初的例子。如果文件名是"file%s%i%s.txt",那么这个程序会崩溃,或者输出一些不知所云的东西。但这不还是问题的全部。事实上,这样的函数调用是一个真正的漏洞。在它的帮助下,我们能发动攻击。用一些刻意的字符串,就可以打印内存里的私有数据。
更多关于这个漏洞的信息可以查看这篇文章。花点时间去通读一遍,我保证,会很有趣的。你不仅能看到理论基础,还可以看到实际的例子。
正确代码
printf("%s", File.getloc().name().c_str());
建议
像 printf() 这样的函数会引起很多有关安全的问题。最好一点也不要使用它们,你可以用其他的来代替啊。比如说,你会发现 boost::format 或者 std::stringstream 也很有用。
一般来说,草率地使用 printf(), sprintf(), fprintf(),等等函数不仅会导致运行不当,而且会引发潜在的漏洞,然后别人就可以利用这个漏洞来攻击你。
33. 永远不要间接引用空指针
bug 是在 GIT 的源代码中发现的。代码中包含的错误被 PVS-Studio 诊断为:V595 在还没有验证‘tree‘指针是否为空之前就使用它。检查134,136行。
void mark_tree_uninteresting(struct tree *tree)
{
struct object *obj = &tree->object;
if (!tree)
return;
....
}
解释
无疑,这是一个糟糕的做法,因为它间接引用了空指针,而这样间接引用的结果就是未定义行为。我们都同意这背后的理论基础。
但是,当具体运用的时候,程序员们就开始争论不休了。总有人声称,这段代码能够正确运行。他们甚至以项上人头做担保——对他们来说,它也总是能运行。所以,我要给出更多的理由来证明我的观点。这就是为什么这篇文章是改变他们观点的又一尝试。
我故意选了这么一个能够引发更多讨论的例子。当tree指针被调用,类成员不仅是在使用,也在计算该成员的地址。那么,如果(tree == nullptr),就永远不会用到成员的地址,而且函数已经退出了。很多人都认为这段代码是正确的。
但并不是。你不应该这么写代码。未定义行为不一定造成程序崩溃,比如赋值给空地址,或者诸如此类的行为。只要你调用了一个等于null的指针,未定义行为可以是任何操作。这个时候再讨论这段代码会如何运行已经没有意义了,因为此时它可以做它任何想做的操作。
未定义行为的一个标志是,编译器会把"if (!tree) return;"删掉——编译器看到指针已经被调用了,而指针不是空的,那么这一行检查就会被编译器移除。这只是众多版本中的一个,而这个版本会引起程序崩溃。
我建议阅读这一篇文章:http://www.viva64.com/en/b/0306/,里面给出了更多细节。
正确代码
void mark_tree_uninteresting(struct tree *tree)
{
if (!tree)
return;
struct object *obj = &tree->object;
....
}
要注意未定义行为,即使一切看上去都没什么问题。没必要冒险。即使我已经写了,但还是很难表现出它的价值。尝试着去避免未定义行为,即使一切看上来都没问题。
有人会想,他清楚的知道,未定义行为是怎样运作的。而且,他可能会想,这意味着,他可以做一些其他人不能做的事,还能保证代码不出错。但并非如此。下一章节将会说明未定义行为真的非常危险。
34. 未定义行为比你想象的要贴近我们的生活
这次很难给出实际应用的例子。但是,我经常有看到会导致下面要所描述问题的可疑代码。这个错误是在处理大数组的时候出现的,而我不知道哪个项目会用到那么大的数组。我们没有真的收集到64位的错误,所以今天的例子是刻意的。
让我们来看一段刻意出错的代码:
size_t Count = 1024*1024*1024; // 1 Gb
if (is64bit)
Count *= 5; // 5 Gb
char *array = (char *)malloc(Count);
memset(array, 0, Count);
int index = 0;
for (size_t i = 0; i != Count; i++)
array[index++] = char(i) | 1;
if (array[Count - 1] == 0)
printf("The last array element contains 0.\n");
free(array);
解释
如果你构建的是这个项目的32位版本,代码是可以正确运行的。但是如果我们要编译64位版本,情况就会变得很复杂。
这个64位项目开始的时候申请5GB的缓冲区,然后初始化为0.接着,用循环修改为非0值:用“|1”来保证非0.
现在来猜一下,如果在x64位版本下用Visual Studio 2015编译的时候,这段代码会如何运行?你有答案吗?如果有,那我们继续。
如果你运行的是这个项目的调试版本,它会因为下标溢出而崩溃。在一定程度上,下标会溢出,而且其值会变成?2147483648 (INT_MIN).
听上去很有逻辑,对不对?事情并不是这样的。这是一个未定义行为,任何事情都有可能发生。
为获得更多内容,我推荐下面的链接:
好玩的是——当我或者其他人说这段代码会引发未定义行为的时候,就会有人抱怨。我不知道为啥,但看起来好像是,他们觉得自己对C++有绝对的了解,而且知道编译器是怎样运作的。
但事实上他们都没有注意到它。如果他们知道,他们不会这么说的(大众的观点):
这在理论上没什么意义。好吧,是,‘int’溢出会导致未定义行为。但是这没什么,不过老生常谈而已。在实际应用中,我们知道代码运行后能得到什么。1 加 INT_MAX 等于 INT_MIN 嘛。但是,可能在宇宙中的某一个角落存在着能让这种情况(整形一出也能得我们想要的结果)发生的结构,但就是我的 Visual C++/GCC 没能给出正确的结果而已。
现在,没有任何魔法。我会用一个简单的例子来证明未定义行为,而且没有用到什么奇怪的结构,只是一个 win64 的项目。
把上面的例子构建成发布版本并运行就已经足够了。代码会崩溃结束,而且“最后一个数组元素包含0”的提示也不会出现。
未定义行为一般以如下的方式出现。所有的数组元素都会被赋值,尽管数组下标的类型 int 并不足以覆盖所有的元素。那些至今还在怀疑我的人,可以看一下下面的代码:
for (size_t i = 0; i != Count; i++)
000000013F6D102D xor ecx,ecx
000000013F6D102F nop
array[index++] = char(i) | 1;
000000013F6D1030 movzx edx,cl
000000013F6D1033 or dl,1
000000013F6D1036 mov byte ptr [rcx+rbx],dl
000000013F6D1039 inc rcx
000000013F6D103C cmp rcx,rdi
000000013F6D103F jne main+30h (013F6D1030h)
这里就有一个未定义行为。而且没有用到什么特殊的编译器,用的就是VS2015.
如果你用 unsigned 代替 int,就不会有未定义行为。而数组就只有一部分被填充,最后我们会收到一条信息——“最后一个数组元素包含0”。
相似的代码用 unsigned 的情况:
unsigned index = 0;
000000013F07102D xor r9d,r9d
for (size_t i = 0; i != Count; i++)
000000013F071030 mov ecx,r9d
000000013F071033 nop dword ptr [rax]
000000013F071037 nop word ptr [rax+rax]
array[index++] = char(i) | 1;
000000013F071040 movzx r8d,cl
000000013F071044 mov edx,r9d
000000013F071047 or r8b,1
000000013F07104B inc r9d
000000013F07104E inc rcx
000000013F071051 mov byte ptr [rdx+rbx],r8b
000000013F071055 cmp rcx,rdi
000000013F071058 jne main+40h (013F071040h)
正确代码
在程序中你要用对正确的类型,这样才能确保它顺利运行。如果你要处理大数组,忘了 int 和 unsigned 吧。正确的类型有 ptrdiff_t, intptr_t, size_t, DWORD_PTR, std::vector::size_type 等等,在这里用 size_t.
size_t index = 0;
for (size_t i = 0; i != Count; i++)
array[index++] = char(i) | 1;
建议
如果根据 C/C++ 的语言机制会导致未定义行为的话,就别跟它们争了,也不要尝试去预测它们会在将来做出什么表现。只要不写危险的代码就好了啊。
还是有很多固执的程序员不愿意在转换负数、比较 this 和 null 或者有符号类型溢出时看任何表示怀疑的言论。
不要这样。就算现在代码运行得好好的也不意味着一切都是好的。未定义行为是不可预测的。可预测的程序行为是未定义行为的一个变体。
35. 在枚举中加了新的枚举常量后别忘了修改switch运算
下面的代码来自 Appleseed 项目。代码中包含的错误被 PVS-Studio 诊断为:V719 switch 语句没有覆盖枚举“InputFormat”的所有值,少了InputFormatEntity.
enum InputFormat
{
InputFormatScalar,
InputFormatSpectralReflectance,
InputFormatSpectralIlluminance,
InputFormatSpectralReflectanceWithAlpha,
InputFormatSpectralIlluminanceWithAlpha,
InputFormatEntity
};
switch (m_format)
{
case InputFormatScalar:
....
case InputFormatSpectralReflectance:
case InputFormatSpectralIlluminance:
....
case InputFormatSpectralReflectanceWithAlpha:
case InputFormatSpectralIlluminanceWithAlpha:
....
}
解释
有的时候我们需要在已经存在的枚举里加入新的元素,当我们做这个操作的时候,我们需要特别的谨慎——因为我们要检查全部代码看看哪里有用到这个枚举,比如说在 switch 和 if 中。上面给出的代码就是这种情况。
在 InputFormat 中加了 InputFormatEntity——这里是我想象的,因为确实在 InputFormat 的后面有加入这个常量。很多时候,程序员在枚举后面加了新的常量,然后忘了检查代码以确保他们有正确处理这个常量,也没有修改 switch 操作。
最后的结果就是,在这个例子中,并没有处理到"m_format==InputFormatEntity"的情况。
正确代码
switch (m_format)
{
case InputFormatScalar:
....
case InputFormatSpectralReflectance:
case InputFormatSpectralIlluminance:
....
case InputFormatSpectralReflectanceWithAlpha:
case InputFormatSpectralIlluminanceWithAlpha:
....
case InputFormatEntity:
....
}
建议
让我们想想,怎样在代码重构的时候避免这种错误?最简单,但不那么有效的解决方法就是加一个‘default’,它可以输出一个信息,像这样:
switch (m_format)
{
case InputFormatScalar:
....
....
default:
assert(false);
throw "Not all variants are considered"
}
现在,如果变量 m_format 是 InputFormatEntity,我们就可以看到一个异常。这样的处理方法有两个不好的地方:
因为有可能在测试阶段这个错误并没有显现出来(如果在测试的时候,m_format不等于InputFormatEntity),那这个错误就会流入发布版本,以后才会显现——在客户运行时出现。如果要顾客来反映这种问题,真的很糟糕。
如果我们把default考虑为一个错误,那我们就不得不写一个case来解决枚举所有可能的值。这样就很不方便,尤其是当一个枚举中有很多常量的时候。有时,用 default 来处理不同 case 真的很方便。
我建议用以下的方法来解决这个问题,我不敢说这个方法很完美,但至少它有解决到问题。
当你定义一个枚举的时候,要确保你也加了一条特殊的注释。你也可以用关键词和枚举名。
例子:
enum InputFormat
{
InputFormatScalar,
....
InputFormatEntity
//If you want to add a new constant, find all ENUM:InputFormat.
};
switch (m_format) //ENUM:InputFormat
{
....
}
在上面的代码中,当你要改变枚举InputFormat,你就可以直接在项目的源代码中查找“ENUM:InputFormat”。
如果你是在一个开发者团队中,你可以告诉你的小伙伴们这个约定,然后把它加入到你们的编程标准和风格指引中。如果有人没能遵守这条原则,真遗憾。
原文链接:https://software.intel.com/en-us/articles/the-ultimate-question-of-programming-refactoring-and-everything
本文由 看雪翻译小组 lumou 编辑
相 关 阅 读:
C++编程的 42 条建议(二)
C++编程的 42 条建议(三)
C++编程的 42 条建议(四)
....
更多优秀文章点击左下角“关注原文”查看!
看雪论坛:http://bbs.pediy.com/
微信公众号 ID:ikanxue
微博:看雪安全
投稿、合作:www.kanxue.com