关于C++编程的42条建议（二）

下面的代码选自Godot Engine项目。该错误被PVS-Studio诊断为：V567 未定义行为（undefined behavior）。在序列点（sequence point）中使用两次会修改变量“t”的值。

解释

有的时候，你会遇到那种代码，作者通过复杂的结构把所有的运算都压缩在一小段代码中。这样做并不会帮到编译器半分，但是在让代码更难懂，让其他程序员（甚至作者自己）更难理解上面确实起了不少作用。而且这种代码出错的概率也比较高。

就是这种代码，程序员想要只用短短几行来放大量代码，通常会出现有关未定义行为的错误。他们经常在一个序列点内读写同一个变量。为更好理解这个问题，我们需要讨论一下“无定义行为”和“序列点”。

未定义行为是一些编程语言的一个属性，表现为一个问题的结果取决于编译器的执行或者优化的选择。很多出现未定义行为的例子（包括我们现在讲的例子）大都和“序列点”相关。

序列点的定义是一个程序执行中的点，这个点的特殊性在于，在这个点之前语句产生的所有副作用都将生效，而后面语句的副作用还没有发生。在C/C++中的序列点出现在下述位置：

"&&", "||", ","。如果不过分使用，这些操作遵循从左至右的运算顺序

三元运算符“？：”

完整表达式结束处（通常以“;”为标记）;

函数调用时的函数入口点，但是是在参数初始花后

函数返回时，在返回值已经复制到调用上下文

注意。在新的C++标准中已经摒弃了“序列点”这个概念，但是我们还是用上面的解释，因为这能让不熟悉这门语言的你们更容易更快理解一点。这个解释比新的解释更简单些，也更方便我们理解为什么不能把所有的操作都压缩在一起。

在一开始我们给出的例子中，并没有上面的所提到的序列点。但是，“=”，还有括号，不能这样处理。因此，我们我们不知道在计算这个式子的时候t的值是多少。

换句话说，这个表达式有一个序列点，所以编译器不知道以什么样的顺序来使用t。比如说，“t*t”可以在“t=t/d-1”之前或之后进行。

正确代码

建议

很明显，把所有的操作都放在一行代码中并不明智。除了难读，还容易放错。

把表达式分成两部分我们能同时解决两个问题——让代码更易读，和避免了由于增加序列点带来的未定义行为。

上面讨论的代码并不是出错的唯一例子。这里是另外一个：

跟前面的例子一样，这段复杂难懂的代码也犯了类似的错误。程序员打算把addr的自增操作放在一个会导致未定义行为的表达式中，因为编译器不知道在表达式右边，addr会取何值——它原来的值还是自增后的值。

对于这个问题最后的解决方案跟前面那个一样——不要没缘由地做那么复杂的操作。把操作分成几部分，而不是把它们都放在同一行里：

从此处我们可以得到一个简单而有用的结论——不要尝试着把一系列的操作用短短几行来完成。把代码分成几部分会更好，这样不但易于理解也减少了犯错的几率。

下次当你打算写一个复杂的结构的时候，停下来一会，然后想想，你这样做会付出什么代价，而你，是否已经准备好付出这个代价。

这个bug是在Source SDK中发现的。该错误被PVS-Studio诊断为：V525 代码包含了相同的语句块。检查'SetX', 'SetY', 'SetZ', 'SetZ'这几项。

inline void SetX( float val );

inline void SetY( float val );

inline void SetZ( float val );

inline void SetW( float val );

inline void Init( float ix=0, float iy=0,

float iz=0, float iw = 0 )

{

SetX( ix );

SetY( iy );

SetZ( iz );

SetZ( iw );

}

解释

我敢肯定，这段代码是复制粘贴的。复制第一行，然后修改特定的字母。最后，复制粘贴没能让程序员如愿：他的注意力被减弱，而且他最后忘了把‘Z’改为‘W’。

在这个例子中，我们不关注作者犯错了这个事实，我们关心的是，这种错误是由于一系列单调的动作引起的。

我非常建议阅读“最后一行效应”这篇文章。由于公共利益，这篇文章的科学版本已经出版。

简单来说，但使用复制-粘贴术来复制代码的时候，你在最后几行出错的可能性非常高。这不是我的猜测，有统计数据的。

正确代码

建议

我希望你以阅读了上面我所提到的文章。所以，再一次，我们要解决接下来的问题。当要写差不多的代码块，程序员会复制粘贴，然后做些轻微的修改。这么做的时候，他们常常会忘了修改特定的单词或字母，而由于注意力被削弱，这种事一般会出现在最后几行。

为减少出错的次数，这里有些小贴士：

把你那些看起来相似的代码放在一个“表格”里：这样错误应该会更突出。我们会在下一节讨论“表格”的布局。可能在这个例子里表格布局不会有多大用处，但在编程中它依旧是个很有用的东西。

在复制粘贴的时候要谨慎小心，注意力集中。集中精神，再次检查你的代码——尤其是最后几行。

现在你已经知道了最后几行效应，把它记在心上，顺便告诉你的同事。知道错误是怎样产生的，将有助于你避开它们。

把“最后一行效应”这篇文章分享给你的同事。

下面的代码来自ReactOS项目（兼容于Windows的开源操作系统）。该错误被PVS-Studio诊断为：V560 部分条件表达式的值总为true：10035L

解释

上面给的代码很少，你应该很容易定位其错误。但是在现实世界的代码，bug都很难注意到。当阅读这样的代码的时候，你会无意识的跳过有类似比较的代码块，直接到下一片段。

这种错误的出现与判断条件的无格式有关。你不会愿意在其上花费太多注意力，因为需要精力，而且我们会想，既然判断条件都差不多，应该不会出错吧，一切应该都好好的吧。

一种能解决这种错误就是，把代码表格化。

如果你很懒，不想去找出上面那段代码的错误。我可以告诉你，有一个判断条件少了“errno==”，这样就会导致条件永远为真，因为EWOULDBLOCK不等于0.

正确代码

建议

首先，让我们来看一段最简单的表格化代码。其实我也不喜欢这样。

现在看起来好多了，但是还不够好。

我不喜欢这种布局有两种原因。首先，错误还是不够明显; 其次，为了对齐，要插入好多空格。

这就是为什么我们需要对这种格式化风格进行两点改进。第一，一行不要超过一个比较：这样容易发现错误。比如：

第二个改进就是用更合理的方式写&&, ||这些运算符。比如，写在左边而非右边。

看一下用空格来对齐有多么诡异：

把运算符写在左边的话，编程会更快更简单的：

让我们把这两种改进运用到刚开始的代码块里：

是，这样更长了——但是错误也更明显了。

我同意，这看起来很诡异，然而我还是很推荐用这种法子。我用这个法子已经半年了，觉得不错。所以我自信这个建议能帮到你。

我并不认为代码变得更长是个问题。我曾经用这样写过：

代码太长太密集了，很失望？我同意，所以我们把它写成一个函数吧！

你可能会觉得我有点夸张，太过完美主义。但我向你保证，在复杂的表达式中，这种错误很常见。如果不是它们如此常见，我才不会提出来呢。它们随处可见，并且难以发现。

下面是另外一个例子，来自WinDjView：

这个函数虽然只有几行，但是仍然有个错误。函数返回值永远为true。从长远来看，这种错误跟格式混乱，以及程序员坚持使用这种代码多年，不愿去认真阅读它有很大关系。

让我们重构这段代码，将其表格化。我会增加一些括号：

你无须用我说的这种方法来设计你的代码。这一条建议的目的是让你注意因书写混乱引起的错误。通过将代码表格化，你可以避免很多细微的拼写错误，这就够了。所以，我希望这条建议有帮到你。

注意

坦白说，我不得不提醒你，表格化有时也有害。来看这个例子：

代码来自 eLynx SDK。程序员想要对齐代码，所以他在713前面加了个0. 不幸的是，他忘记了，0在首位表示该数字是八进制。

字符串数组

我希望，我有把代码格式化这个建议表达清楚。但是我还想在给几个例子。让我们再来看几个例子。在这里，我要说的是，表格化不仅仅可以用于判断条件，也可以用在其他结构中。

下面的代码块来自Asterisk项目。其错误被PVS-Studio诊断为：V653 一个含有两部分的可疑字符串被用来数组初始化。可能是少了逗号。检查："KW_INCLUDES" "KW_JUMP".

这里有个拼写错误——少了一个逗号。结果就是两个完全不同含义的字符串被合成一个了。这样，我们其实是用它来初始化了：

若是程序员用表格化，应能避免此错。那样，如果少了逗号，很容易就能定位到。

注意，就像上次一样，如果我们把分割符（在这个例子中是逗号）放在右边，我们要用到很多空格，这样很不方便。如果有一行/词组很长要加空格是非常不方便的：所以我们需要重新设计整个表格。

这就是我为什么要再次建议用接下来的法子来表格化：

现在就很容易定位到缺失逗号的地方，而且无需使用过多的空格——这样的代码漂亮又易懂。可能这样的格式化并不常见，但你很快就会 适应的——试一下吧。

最后，我的座右铭就是：一般说来，漂亮的代码往往就是正确的代码。

我们已经将了好的编程风格，但这次，我们来看个反面例子。对于写出好代码来说，好的编程风格还不够：仍然会有各种错误，好的编程风格并非包治百病。

下面的代码片段来自PostgreSQL. 其错误被PVS-Studio诊断为：V575 'memcmp'函数要处理‘0’元素。检查第三个参数。

Cppcheck分析器也检测到了同样的错误。它给出一个警告：memcmp() 第三个参数无效。这里需要一个非布尔类型的值。

解释

反括号放错了地方。这仅仅是一个拼写错误，但不幸的是，它完全改变了代码的意思。

sizeof(zero_clientaddr) == 0这个表达式永远为‘false’，因为任何对象的大小都大于0。false又等于0,这样就导致memcmp()要比较0字节。如此，函数就会认为数组相等然后返回0.这就意味着上面的代码可以化简为if (false)。

正确代码

建议

这就是一个我无法给出任何安全编程技巧来避免拼写错误的例子。我能想到的唯一一个就是"尤达条件"，把常数写在比较运算符的左边：

if (0 == memcmp(&(beentry->st_clientaddr), &zero_clientaddr,

sizeof(zero_clientaddr)))

但我不建议这种写法。我不喜欢也不用它出于两个原因：

第一，这样判断条件的可读性变差。我不知道要怎样表达，但是这种风格以尤达命名并非没有理由。

第二，这样对于解决把括号放在错误的地方并没有什么帮助。你出错的方式有千万种。下面的例子就是尤达条件无法阻止括号放错的：

if (0 == LoadStringW(hDllInstance, IDS_UNKNOWN_ERROR,

UnknownError,

sizeof(UnknownError) / sizeof(UnknownError[0] -

20)))

上面的代码片段来自ReactOS 项目。它的错误很难发现，所以让我帮你指出来：sizeof(UnknownError[0] - 20)

所以尤达条件在这里没用。

我们可以创造一些人为的风格来确保每一个反括号都很其正括号匹配。但这样会使代码很笨重、难看，而且没有人会愿意那样写。

所以，再说一遍，我无法推荐任何一种编程风格来避免反括号放在错误的地方。

而这也就是编译器派上用场的地方，编译器应该会提醒我们这个奇怪的结构的，对不对？额，它应该，但它没有。我运行Visual Studio 2015,详述了/Wall switch...然后没有收到任何提示。但我们不能因此责备编译器，事实上，它有很多工作要做。

从这里我们得出的最重要的结论就是：好的编程风格和编译器（我确实喜欢VS2015编译器）并不总能解决问题。有有时会听到这样的言论：“你只需要把编译器的提醒设到最高级，然后使用好的编程风格，那么一切都会好的。”不，并非如此。我不是说有些程序员不善于编程，只是，每个程序员都会出错。每一个，没有例外。在编译器和好的变成风格下面还是会有很多拼写错误。

所以好的编程风格+编译器提醒这个组合很重要，但是还不够。这也是为什么我们需要使用多种法子来搜寻bug。并没有什么杀手锏，高质量的代码只能通过使用多种搜索bug的技术结合来得到。

我们这里讨论的错误可以用下面的法子来找到：

检查代码

单元测试

手动测试

静态代码分析

等等

我想你已经猜到了，我个人从对静态代码分析方法最感兴趣。顺便说一句，静态代码分析是解决这种特殊问题最适当的方法。因为它能在最早检测到错误，比如说，就在代码写完后。

事实上，这种错误用类似Cppcheck 或PVS-Studio的工具很快就能找到。

结论 有些人没有足够的技巧来避免错误。每个人都会出错——无可避免。即使是大师级的人物，也会时不时犯些拼写错误。那么既然它无可避免，责备程序员或者编译器，编程风格都没有意义。这样并没有任何帮助。所以，我们要使用多种软件的组合来提高技术。

所有关于这个错误的代码都很长。我已经选了最短的那一个了，但还是比较长，非常抱歉。

这个bug是在Source SDK库中发现的。该错误被PVS-Studio诊断为：V556 比较不同枚举类型的值：Reason == PUNTED_BY_CANNON.

解释

Reason是枚举类型PhysGunDrop_t的一个变量。这个变量现在是跟一个属于另一个枚举类型的常量PUNTED_BY_CANNON在做比较，这样的比较很明显是逻辑错误。

这种bug非常常见。我在类似Clang, TortoiseGit, and Linux Kernel的项目里都有遇到过。

这种错误那么常见的原因就是，枚举在标准C++里并非拼写安全的。你很容易对哪个应该跟哪个比较感到困惑。

正确代码

我不知道这段代码的正确版本应该是怎么样的。我猜，在PUNTED_BY_CANNON那个位置的应该是DROPPED_BY_CANNON 或者 LAUNCHED_BY_CANNON。就当是 LAUNCHED_BY_CANNON吧。

建议

如果你用C++编程，你很幸运。我建议你开始使用enum class，而且编译器不会让你比较两个不同枚举类型的值。你不会再拿英镑和便士做比较了。

我对有些C++的创新不太信任。比如说，auto关键字。我坚信过度使用它会有害。这是我对它的看法：程序员看代码比写代码的时间要多，所以我们要确保，编程文档要易读。在C中，变量要在函数的前面声明，所以当你写到中间或者后边的时候，总是不容易指出一些变量到底指什么。这也是为什么有那么多命名方式。比如说，前置命名，pfAlice可能代表“浮点类型指针(pointer to float)”。

在C++中，你可以在任何你需要的时候声明变量，这种方式也被认为是一种好的风格。使用前缀或者后缀命名法都不再流行。此时，auto关键词出现了，导致了程序员开始使用大量类似“auto Alice = Foo();”的诡异结构。

很抱歉，离题了。我就是想告诉你，有些新特性既好又坏。但enum class不是这样的:我坚信enum class只好不坏。

在使用enum class的时候，你要明确指出哪个常量是属于哪个枚举类的。这有助于代码出现新的错误。然后，代码会长这样：

没错，修改旧的代码会有一定的困难。但我强烈建议你从今天起，在新的代码中开始使用enum class。你的项目会从中受益的。

我不觉得我要在这里介绍enum class的必要。下面的几条链接可以帮助你学到C++11 这个完美的新特性。

维基百科。C++11.Strongly typed enumerations

Cppreference。Enumeration declaration.

StackOverflow. Why is enum class preferred over plain enum?

这一部分会和“不要把所有的操作运算都压缩到同一行”有些许相似，但这一次我想要把焦点放在另一个地方。有时候我会觉得程序员好像在跟某个人较劲，想要写最短的代码。

我不是要讲复杂的模板。我们要讨论的是一个不同的课题，因为很难讲清楚模板在哪里有害，又在哪里有益。现在我要提及一个更简单的情况，和C与C++程序员有关。他们会倾向于写很复杂的结构，想着，“我这么做是因为我能做到”。

下面的代码来自KDE4项目。其错误被PVS-Studio诊断为：V593 检查类似'A = B == C'的表达式。这种表达式是按'A = (B == C)'的顺序运算的。

解释

看完这段代码，我总有这样一个问题：这么做有什么意义呢？你想要节省行数？你想展示说，你可以把几个动作放在同一个表达式里？

结果我们得到一个 典型的错误模板——用类似if (A = Foo() == Error)这种表达式。

比较运算的优先级大于赋值运算的优先级。这也是为什么"mOp.del( usrc.dn() ) == -1"先执行，然后就只有true (1) 或 false (0) 这两值可以赋给变量id。

[招生]科锐逆向工程师培训(2024年11月15日实地，远程教学同时开班, 第51期)