02 把错误关在笼子里的五道关卡 上一讲中,我们一起讨论了什么是优秀的代码。简而言之,优秀的代码是经济、规范、安全的代码。在平时的工作中,我们要朝着这个方向努力,时常站在团队、流程、个人能力的角度去思考优秀代码。

作为一名软件工程师,我们都想写出优秀的代码。可是,怎么才能编写出经济、规范、安全的代码呢?这是个大话题,相信你之前也有过思考。

无心的过失

开始之前,我先给你讲个曾经发生过的真实案例。2014年2月,苹果公司的iOS和OS X操作系统爆出严重的安全漏洞,聪明的黑客们可以利用这一漏洞,伪装成可信网站或者服务,来拦截用户数据。而造成这一漏洞的原因,也让业界专家大跌眼镜。

下面我用 C语言的伪代码来给你简单描述下当时的漏洞情况。 if ((error = doSomething()) != 0) goto fail; goto fail; if ((error= doMore()) != 0) goto fail; fail: return error;

其实这段代码非常简单,它有两个判断语句,如果判断条件成立,那就执行“goto fail”语句,如果不成立,那就跳过判断语句继续执行。上面的“goto fail”语句,它的意思是略过它之后的所有语句,直接跳转到标有“fail”语句的地方,也就是第6行。

我们来分析下,第一个判断条件(第一行和第二行),如果error不等于零,那就跳转到fail语句,这逻辑上没什么问题。而第三行,没有任何附加条件,就能直接跳转到fail语句,也就是说,它下面的代码永远也无法执行,这里是不是有问题?是的,漏洞就是出在这里。

这一行多余的代码就是导致苹果操作系统那个安全漏洞的罪魁祸首。2014年2月21日,苹果发布了相关的安全补丁,你随便一搜“GoTo Fail漏洞”就能找到相关的细节,我这里不赘述了。

我们每天仰慕的苹果操作系统出现这样“低级”的错误,你是不是有点惊讶?这么一个“简单”的错误,引发了一个非常严重的安全漏洞,是不是也有点出乎意料?上面的错误,简单看,就是复制的时候多复制了一行,或者因为时间关系,或者因为粗心大意,苹果的工程师硬是没检查出来。这在我们平时的工作中,也经常出现。

这个具有重大杀伤力的bug是如此的“幼稚”,如此的“好玩”,如此的“萌萌哒”,以至于到现在,人们还可以买到印有“GoTo Fail”的T恤衫,更别提业界对于这个问题的兴趣了。有很多文章,专门研究这一个“低级”安全漏洞;甚至有人探讨这个“低级”错误对于计算机软件教育的积极影响。

所有的危机都不应该被浪费,这一次也不例外。这些年,我也一直在思考为什么我们会犯如此“低级”的错误?即使是在苹果这样的大公司。反过来再想,我们应该如何尽可能避免类似的错误呢?

人人都会犯错误

没有人是完美的,人人都会犯错误。这应该是一个共识。这里面既有技术层面的因素,也有人类的行为模式的因素,也有现实环境的影响。我们在此不讨论人类进化和心智模式这样的严肃研究成果。但是,有两三个有意思的话题,我想和你聊聊。

第一个比较普遍的观点是好的程序员不会写坏的代码,要不然,就是他还不足够优秀。我尊重这个观点背后代表的美好愿望,但是这个观点本身我很难认同。它一定程度上忽视了人类犯错误的复杂性,和影响因素的多样性。

我认为,即使一个非常优秀的程序员,他主观上非常认真,能力又非常强,但他也会犯非常“低级”、“幼稚”的错误。所以,你不能因为苹果那个程序员,犯了那个非常低级的错误,就一棒子把他“打死”,认为他不是一个好的程序员。

第二个更加普遍的观点是同样的错误不能犯第二次。作为一名程序员,我同样尊重这个观点背后代表的美好期望。但是,我想给这个观点加一点点限制。这个观点应该是我们对自身的期望和要求;对于他人,我们可以更宽容;对于一个团队,我们首先要思考如何提供一种机制,以减少此类错误的发生//。如果强制要求他人错不过三,现实中,我们虽然发泄了怨气,但是往往错失了工作机制提升的机会。

第三个深入人心的观点是一个人犯了错误并不可怕,怕的是不承认错误。同样的,我理解这个观点背后代表的美好诉求。这是一个深入人心的观点,具有深厚的群众基础,我万万不敢造次。在软件工程领域,我想,在犯错这件事情上,我们还是要再多一点对自己的谅解,以及对他人的宽容。错误并不可怕,你不必为此深深自责,更不应该责备他人。要不然,一旦陷入自责和指责的漩涡,很多有建设意义的事情,我们可能没有意识去做;或者即使意识到了,也没法做,做不好

我这么说,你是不是开始有疑惑了:人人都会犯错误,还重复犯,还不能批评,这怎么能编写出优秀的代码呢?换句话说就是,我们怎么样才会少犯错误呢?

把错误关在笼子里

人人都会犯错误,苹果的工程师也不例外。所以,“GoTo Fail”的“幼稚”漏洞,实在是在情理之中。可是,这样的漏洞是如何逃脱重重“监管”,出现在最终的发布产品中,这多多少少让我有点出乎意料。

我们先来看看,这个错误是经过了怎样的“工序”,穿越了多少障碍,需要多少运气,最终才能被“发布”出来。

我把这样的工序总结为“五道关卡”。

第一道关:程序员

提高程序员的修养,是一个永不过时的课题。从别人的失败和自己的失败中学习、积累、提高,是一个程序员成长的必修课。我知道,这是你和我一直都在努力做的事情。

第三行的“GoTo Fail”,实在算得上“漏网之鱼”,才可以逃过哪怕最平凡的程序员的眼睛,堂而皇之地占据了宝贵的一行代码,并且狠狠地玩耍了一把。

现在我们可以再回过来看看那段错误代码,如果是你写,你会怎么写呢?从你的角度来看,又有哪些细节可以帮助你避免类似的错误呢?这两个问题,你可以先停下来1分钟,想一想。

在我看来,上面那段代码,起码有两个地方可以优化。如果那位程序员能够按照规范的方式写代码,那“GoTo Fail”的漏洞应该是很容易被发现。我们在遇到问题的时候,也应该尽量朝着规范以及可持续改进的角度去思考错误背后的原因,而非一味地自责。

首先,他应该正确使用缩进。你现在可以再看下我优化后的代码,是不是第三行的代码特别刺眼,是不是更容易被“逮住”? if ((error = doSomething()) != 0) goto fail; goto fail; if ((error= doMore()) != 0) goto fail; fail: return error;

其次,他应该使用大括号。使用大括号后,这个问题是不是就自动消失了?虽然,多余的这一行依然是多余的,但已经是没有多大危害的一行代码了。

if ((error = doSomething()) != 0) { goto fail; goto fail; } if ((error= doMore()) != 0) { goto fail; } fail: return error;

从上面这个例子里,不知道你有没有体会到,好的代码风格带来的好处呢?工作中,像苹果公司的那位程序员一样的错误,你应该没少遇到吧?那现在,你是不是可以思考如何从代码风格的角度来避免类似的错误呢?

魔鬼藏于细节。很多时候, 优秀的代码源于我们对细节的热情和执着。可能,你遇到的或者想到的问题,不是每一个都有完美的答案或者解决办法。但是,如果你能够找到哪怕仅仅是一个小问题的一个小小的改进办法,都有可能会给你的代码质量带来巨大的提升和改变

当然,你可能还会说,我代码风格不错,但是那个问题就是没看到,这也是极有可能的事情。是这样,所以也就有了第二道工序:编译器。

第二道关:编译器

编译器在代码质量方面,作为机器,恪尽职守,它可以帮助我们清除很多错误。还是以上面的漏洞代码为例子, 这一次其实编译器的防守并没有做好,因为它毫无察觉地漏过了多余的“GoTo Fail”。

在Java语言里,对于无法访问的代码(第三行后的代码), Java编译器就会及时报告错误。而在2014年2月的GCC编译器里,并没有提供这样的功能。

至今,GCC社区对于无法访问代码的检查,还没有统一的意见 。然而,GCC社区并没有完全浪费这个“GoTo Fail”的问题 。为解决类似问题,从GCC 6开始,GCC社区为正确使用缩进提供了一个警告选项( -Wmisleading-indentation )。如果代码缩进格式没有正确使用,GCC就会提供编译时警告。现在,如果我们启用并且注意到了GCC编译器的警告,犯类似错误的机会应该会大幅度地降低了。

在这里,我要提醒你的是,对于编译器的警告,我们一定要非常警觉。能消除掉所有的警告,你就应该消除掉所有的警告。就算实在没有办法消除掉编译警告,那你也一定要搞清楚警告产生的原因,并确认编译警告不会产生任何后续问题。

第三道关:回归测试 (Regression Testing)

一般地,软件测试会尽可能地覆盖关键逻辑和负面清单,以确保关键功能能够正确执行,关键错误能够有效处理。一般情况下,无论是开发人员,还是测试人员,都要写很多测试代码,来测试软件是否达到预期的要求。

另外,这些测试代码还有一个关键用途就是做回归测试 。如果有代码变更,我们可以用回归测试来检查这样的代码变更有没有使代码变得更坏。

上述的“GoTo Fail”这样的代码变更,涉及到一个非常重要的负面检查点。遗憾的是,该检查点并没有包含在回归测试中;或者,在这个变更交付工程中,回归测试并没有被执行。

软件测试没有办法覆盖所有的使用场景。但是,我们千万要覆盖关键逻辑和负面清单。一个没有良好回归测试的软件,很难保证代码变更的质量;也会使得代码变更充满不确定性,从而大幅地提高代码维护的成本。

第四道关:代码评审 (Code Review)

代码评审是一个有效的在软件研发过程中抵御人类缺陷的制度。通过更多的眼睛检查软件代码,被忽视的错误更容易被逮住,更好的设计和实现更容易浮现出来。

那代码评审是怎么实现的呢?一般情况下,代码评审是通过阅读代码变更进行的。而代码变更一般通过某种形式的工具呈现出来。比如OpenJDK采用的Webrev 。你可以访问我的一个代码评审使用的代码变更页面 ,感受下这种呈现方式。

回到上面那个“GoTo Fail”的代码变更,看起来什么样子呢?下面是其中的一个代码变更版本示例: if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0) goto fail; + goto fail; if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0) goto fail;

添加的这行代码,还是相当刺眼的。多一些眼睛盯着这些代码,多一些形式展现这些变更,就会大幅度地降低问题藏匿的几率。

上述的“GoTo Fail”这样的代码变更,怎么就逃过代码评审者的眼睛呢?我想说的是,评审者也是人,我们不能期望评审者能发现所有的问题。

第五道关:代码分析 (Code Analysis)

静态代码分析(Static Code Analysis)是通过对源代码的检查来发现潜在问题的一种软件质量保障方式。有很多静态代码分析工具可以帮助你检查代码缺陷,比如说商业软件Coverity,以及开源软件FindBugs。你可以试试看,有哪些工具可以检测到这个“GoTo Fail”问题。

代码覆盖率(Code Coverage)是一个反映测试覆盖程度的指标。它不仅仅量化测试的指标,也是一个检测代码缺陷的好工具。如果你的代码覆盖率测试实现了行覆盖(Line Coverage),这个“GoTo Fail”问题也很难闯过这一关。

很显然,苹果的这一关也没有拦截住“GoTo Fail”。这样,“GoTo Fail”就像千里走单骑的关云长,闯过了五关(有些软件开发流程,也许会设置更多的关卡)。

代码制造的流水线

我们分析了这重重关卡,我特别想传递的一个想法就是,编写优秀的代码,不能仅仅依靠一个人的战斗。代码的优秀级别,依赖于每个关卡的优秀级别。高质量的代码,依赖于高质量的流水线。每道关卡都应该给程序员提供积极的反馈。这些反馈,在保障代码质量的同时,也能帮助程序员快速学习和成长。

可是,即使我们设置了重重关卡,“GoTo Fail”依然“过关斩将”,一行代码一路恣意玩耍。这里面有关卡虚设的因素,也有我们粗心大意的因素。我们怎么样才能打造更好的关卡,或者设置更好的笼子?尤其是,身为程序员,如何守好第一关?

欢迎你在留言区说说自己的思考。下一讲,我们再接着聊这个话题。

一起来动手

下面的这段代码,有很多疏漏的地方。你看看自己读代码能发现多少问题?上面我们讨论的流程能够发现多少问题。不妨把讨论区看作代码评审区,看看在讨论区都有什么不同的发现。 package com.example; import java.util.Collections; import java.util.List; import javax.net.ssl.SNIServerName; class ServerNameSpec { final List serverNames; ServerNameSpec(List serverNames) { this.serverNames = Collections.unmodifiableList(serverNames); } public void addServerName(SNIServerName serverName) { serverNames.add(serverName); } public String toString() { if (serverNames == null || serverNames.isEmpty()) return “"; StringBuilder builder = new StringBuilder(512); for (SNIServerName sn : serverNames) { builder.append(sn.toString()); builder.append("\n"); } return builder.toString(); } }

你也可以把这篇文章分享给你的朋友或者同事,一起来讨论一下这道小小的练习题。

参考资料

https://learn.lianglianglee.com/%e4%b8%93%e6%a0%8f/%e4%bb%a3%e7%a0%81%e7%b2%be%e8%bf%9b%e4%b9%8b%e8%b7%af/02%20%e6%8a%8a%e9%94%99%e8%af%af%e5%85%b3%e5%9c%a8%e7%ac%bc%e5%ad%90%e9%87%8c%e7%9a%84%e4%ba%94%e9%81%93%e5%85%b3%e5%8d%a1.md