代码审查

作者:Larry Colen

开源和自由软件最受推崇的优势之一就是同行评审。“人多眼杂,bug易除”是常见的说法。 这句话的前提是,确实有很多人在查看代码的各个部分,并且这些人知道要查找什么。 代码审查就像性行为一样,几乎任何人都可以做,但是技巧和训练可以让你做得更好。

对于那些还没有听过讲座的程序员来说,以下是进行代码和设计审查的优势:

  • 在产品生命周期的早期发现错误,修复它们的成本就越低,也越容易。

  • 如果其他人查看您的代码或设计,他们很可能会发现您遗漏的错误。

  • 当您知道有人要查看您的代码时,您更有可能整理代码并确保有准确且最新的文档。

  • 您可以通过阅读别人的代码学到很多东西。

  • 多个人熟悉一个程序是对抗“卡车撞击综合征”的最佳保险,这种情况是指唯一理解该软件的人被卡车撞了、离开了公司或因某种原因无法咨询。

  • 它可以作为建立质量指标的一种手段,以便衡量不同质量流程的有效性。

  • 向别人解释您的软件的过程可以帮助您真正审查自己的程序,而不是仅仅看着并看到您期望或想看到的东西。

  • 如果做得好,代码和设计审查可以在整个项目生命周期中节省时间并提高质量。

代码审查的缺点是它们会占用时间,不仅占用积极参与项目的人员的时间,还会占用通常也面临截止日期压力的其他人员的时间。 尽管有大量研究表明,如果正确进行审查,项目花费的总人工时数会更少,但始终存在一种诱惑,即赌定真的没有任何问题。 这意味着要等到代码编写完成并进行调试后才尝试查找问题。

我已经快要完成关于为什么同行评审是一件好事的布道了。 我要说的是,关于评审最重要的事情是它们实际上要完成。 快速而粗略的审查只发现三分之一的错误,也比没有人实际执行的彻底、详尽的审查更有效。

有两种基本的审查类型:走查和正式检查。 在走查中,作者带领一个或多个同事浏览正在审查的文档。 以我的经验,走查中发现的错误中约有 80% 实际上是作者在解释文档的过程中发现的。 前同事 Eli Weber 说:“如果我可以像对人说话一样对墙说话,我就不需要别人来做代码审查了。”

在正式检查中,一个人或多个人被给予一份文档设计或程序进行审查。 通常,每个人都专注于不同的事情:是否符合样式或编程标准、逻辑错误、文档的完整性等。 通常的做法是制作每个人要查找内容的清单,例如编码风格规则、常见错误、潜在的安全漏洞等。

如果有人发现他们没有关注的错误,那也没问题。 例如,如果检查编码标准合规性的人注意到使用了 AND 运算符代替 OR,他们绝对应该注意到它。 但并非每个人都试图确保文档的各个方面都是正确的。 当审查员发现问题时,他们会记录问题的位置及其严重程度。 优先级范围从“如果您发现自己绝对没事可做,而且老板心情不好,您可以考虑更改某些内容”,到“如果这个错误没有立即修复,它将引发一系列事件,导致我们所知的文明的终结”。

一旦每个人都审查了文档,就会举行会议讨论发现的错误。 错误的讨论可以按照几乎任何逻辑顺序进行:按文档的页码、按错误的严重程度,或者每个审查员可以列出他们发现的所有问题。 该会议通常由文档的作者主持,但也可以由任何人主持。 当提到每个问题时,与会人员就问题的严重程度达成共识。 通常,审查员可能无法评估问题的严重程度,但可能只是感觉有些不对劲,并会征求其他与会者的意见。 重要的是要记住,会议的重点不是解决问题,而仅仅是记录问题。 我非常清楚要阻止一屋子工程师立即尝试解决一个有趣的问题有多么困难,但主持人必须记住要使会议保持在正轨上,否则会议将持续数周,并且花费的成本将超过它节省的成本。

至少在理论上,这就是代码审查在工业或教育环境中运作的方式,在这些环境中,有多个人员彼此靠近

但是,在开源世界中,参与项目的人员不太可能在同一个国家,更不用说同一个城市了,情况又如何呢? 有一些管理严格的项目,其中有邮件列表,有人会在邮件列表中提出更改建议,并且邮件列表上的任何人都可以随时查看。 如果他们注意到他们认为可能是问题的事情(“你这个异教徒,你缩进了三列,而根据一切正确和神圣的东西,你应该缩进两列!”),他们可以向列表或作者发送电子邮件以进行冷静和理性的讨论。 其他项目将遵循这样的模式:有人需要一个程序来做某事,并且发现它还不可用,他们自己编写它。 一旦它满足了自己的需求,他们就会将其提供给公众,可能会将其发布到 freshmeat 或其他类似的论坛。 过一段时间后,其他人需要一个程序来执行相同或相似的任务。 这次他们找到了刚刚编写的那个程序,他们下载了它,但它没有运行。 他们找出它崩溃的地方,找到有问题的代码,并在他们的善良之心和对他们极客同胞的善意驱使下,将错误通知原始作者并发送建议的补丁。

请注意,在这些场景中,都不能保证如果您编写软件,它会被某人审查,或者如果被审查,它会及时审查以使其有用。 一个认真的程序员如何确保他们的代码不仅被审查,而且被及时审查以使其有用呢?

大多数极客都有极客朋友。 作家经常组成小组,每周聚会一次,审查和评论彼此的故事。 软件也可以这样做。 参与者可以在会议之前的某个时间通过电子邮件发送要审查的设计或代码。 使用正式检查方法,他们可以使用会议时间来收集关于发现问题的笔记。 或者,当设计或程序准备好进行审查时,可以向邮件列表发送电子邮件,邀请极客们在 Hans' Pizzhaus 聚会,享用披萨、啤酒和代码走查。

如果当地没有人可以帮助,那么大部分正式检查工作可以通过电子邮件完成。 关键是要找到一群真正会互相审查代码的人,而不是仅仅快速浏览一下。 同样,走查期间发现的大多数错误都是作者在解释代码时发现的。 如果您身边没有人可以劝说坐下来听您解释您的软件,并且如果您的猫不具备 Richard Stallman 和 Dennis Ritchie 的原始编程才能,您可以创建一个“互联网受众”来进行走查,方法是彻底记录您的软件。 坐下来使用您的程序并编写文档来解释它正在做什么的过程应该迫使您从一个角度来看待它,这个角度允许您找到烦人的错误,例如放错位置的括号。

进行代码审查可以显着提高产品的安全性。 首先,如果其他人查看一个程序,就很难在其中留下后门,例如“如果有人使用用户名 f00Bidoo 登录,则自动授予 root 权限”。 然而,大多数漏洞利用,尤其是开源软件的漏洞利用,都是通过探测或搜索已知的错误类型编写的——有点像“恶意代码审查”。 为了堵塞这些漏洞,有必要在有不友好意图的人之前找到它们。

正如数学家所说,在代码中搜索错误对于安全性来说是必要的,但不是充分的。 Open BSD 通过积极的代码审查来解决安全性问题。 Open BSD 的安全团队也做了大量的研究。 Theo de Raadt 说:“事实上,这是我们关注的重点。 试图在旧代码中找到新型的程序员错误。 存在的代码,其中有人们从未想到的常见错误。 例如 fd_set 溢出。 在 Open BSD 中,缓冲区溢出基本上已经灭绝了大约三年。” 有关 Open BSD 关于安全性的理念和目标的更多信息,请查看 www.openbsd.org/security.html

总而言之,代码审查中发现的大多数错误都是因为有人实际看到了代码,无论是同行评审员还是作者在向其他人解释代码的过程中。 同样,最好有一个实际实施的适度检查方法,而不是一个宏大的方案,但始终没有足够的时间来完全实施。 当人们关注安全性时,仔细检查代码中的错误可以获得巨大的收益,但这还不够。 还必须以想要危害系统安全性的人的眼光来看待代码。 不仅要查找代码无法按预期工作的情况,还要查找代码可以被强制执行不应执行的操作的情况。

我想感谢 Theo de Raadt 在讨论 Open BSD 和审查安全问题代码方面提供的宝贵帮助。

Code Reviews
Larry Colen (lrc@red4est.com) 自 1994 年 3 月以来一直在玩 Linux,自 1998 年 11 月以来一直将其作为专业工作。 他的专业兴趣包括操作系统、多处理、软件质量、计算机安全和信号处理。 为了娱乐,他教授高性能驾驶、摇摆舞和骑自行车。 关于他的更多您真正想知道的信息可以在他的个人主页上找到:www.red4est.com/lrc,包括一篇关于软件质量的漫谈文章,网址为:www.red4est.com/lrc/prof_html/debuggable.html
加载 Disqus 评论