C 编程的 42 条建议（五）

刀首木 2017-03-17

展开全文

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./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).

听上去很有逻辑，对不对？事情并不是这样的。这是一个未定义行为，任何事情都有可能发生。

为获得更多内容，我推荐下面的链接：

Integer overflow
Understanding Integer Overflow in C/C++
Is signed integer overflow still undefined behavior in C++?

好玩的是——当我或者其他人说这段代码会引发未定义行为的时候，就会有人抱怨。我不知道为啥，但看起来好像是，他们觉得自己对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
{
  ....
}