事实证明：有效的代码评审(Code Review，也有叫代码审查的），对保证代码质量具有十分重要的作用。因此这两年来我一直尝试着在这块不断改进和完善，以期望能形成一套合理、规范、有 效且高效的代码评审流程，这包括引入在线代码评审系统、走查和在线评审结合、规范评审Request的规模与有效性、设立评审专员等，用心不可谓不良苦 ^_^。大家也的确形成了及时提交Code Review Request或组织进行代码走查的良好习惯。不过我还是发现了一些问题。

* 有些组（我对其影响力不足的^_^）依旧没有严格执行代码评审环节，代码屡屡出现低级错误；
* 走查形式的会议评审缺乏全面性，效果好坏与参与者的“状态”直接相关；
* 在线评审环节缺乏“责任制”，常出现的一种情况是：请求大家评审，结果可能却是大家都没有评审。出现"Request Review Miss"的现象。

这让我陷入思考：长期以来我们在代码评审这块过于依赖人的自觉性，理想地认为每个人都能认识到代码评审的重要性，并认真地执行代码评审的流程或充满激情地 参与到其他人发起的代码评审过程中去，但结果事与愿违。这就像党员如何保持纯洁性一样，如果仅仅依靠个人道德/职业水平约束，这事往往是不成的。事实证明 人治在中国社会是会造成各种社会问题的。我们的代码评审环节也是一样，我们不能再期望所有人都能和我站在一条认知和激情水平线上，于是我打算尝试向“法 治”过渡。

"法"，规则制度也，是团队一致认同的可以提升产品质量的规则制度。以此为前提，我要做的就是设立“检查和预防”机构，即以很低的Cost，检查大家是否按“法”完成了代码评审环节，提醒大家要按“法”进行。我采取了几个措施：

【规范Commit Log 】

这是一个前提工作。实现规范的Commit Log便于后续的检查和监督，同时细化规范的Commit Log信息对代码维护是大有裨益的。在Commit Log中还增加了一些关联信息，方便维护者了解该Commit的背景。初期的模板是这么来确定的：

模板结构：

TITLE
BODY
RELATIONSHIPS

展开后如下：

[Category] Title content

Body content

[BUGID] QC#733 | JIRA#766
[REVIEWID] RB#767
[REVIEWED BY] xx, yy, zz
[SIGNOFF BY] xx

TITLE Category：
   – BUGFIX 代码修复
   – FEATURE 新功能特性添加
   – TASK 诸如代码美化、调整版本号等
   – URGENT 紧急提交，对此类commit，可不做review和拦截

BODY Content：
   有关此次修改的详细信息说明

RELATIONSHIPS：
   – [BUGID] 一般用Bug跟踪系统的ID号
   – [REVIEWID] reviewboard上的ID号
   – [REVIEWED BY] xx, yy, zz
   – [SIGNOFF BY] xx

【"全覆盖"原则】

所有变更代码都要发起在线“Code Review Request”，即便是会议走查的代码，会后也要补提“Review Request”。

【“低保”原则】

每个Review Request至少选择两名评审负责人，填到"Request"中，这两个人必须对此Request给出评审意见，这是一个评审的最低保障了，这总比没有人评审要好。当然了其他人也都可以参与评审。只有这两名评审负责人明确提交"ship it" Comment后，该代码才算是通过评审。

【关键路径拦截】

"对不起，若不符合规定，你的工作将无法进行下去"。有了统一的Commit Log模板，我们就可以对大家的代码Commit环节做检查和拦截了。如果代码没有进行评审，无法填写模板中的字段内容，那代码将无法提交到代码库中。如 果虚构Commit log内容，这将是极大的错误，在抽查中一旦发现，后果将是很严重的^_^。

当然这一过程中还有很多细节需要考虑，比如Reviewer的选择不能集中在一个人身上，否则会造成热点；再比如紧急提交代码应该如何处理等等。“法治” 是与一定的“国情”相匹配的，并不是所有的组织都需要进行这么严格且略有死板“法治”手段，依团队内组员的专业能力和认知水平而定。

有些公司开发了自己的统一开发平台，将一系列流程都在一套系统中规范了起来，这当然是更好的“法治”了。但在没有这样的平台的前提下，初步使用上述的几个手段，还是会收获一些改进的。

如果你看到一个C程序员在通宵熬夜神情紧张地对着电脑敲代码或阅读代码，多数只有两种可能：一是为了赶进度；二就是查找内存Bug。
                                                                                                                              — 个人感悟
 
昨晚搞到凌晨一点多，终于算是把一个棘手的Bug的来龙去脉搞清楚了。截至到今天，这个Bug已经困扰了项目组两个核心开发同事达三周之久了。

这个Bug的确很难查找：

   – 首先模拟环境下无法复现该Bug；
   – 生产环境下该Bug是随机出现的，发生频率十分低；
   – Bug出现时并未有dump core等明显异常现象出现，系统依旧运行良好。

得到Bug报告后，我的两位同事就开始对bug引发的问题现象进行了分析，得出了内存被污染的初步结论。之后又在生产环境做了GDB attach到进程的调试，甚至替换了生产环境的版本，利用传统的print语句在关键路径上输出提示信息，试图找到引发Bug的真正原因。但做过这些 后，所能得到的结论依旧停留在内存被污染，至于怎么被污染的、在哪个业务流程上被污染的却无从得知。无奈之下，两位同事开始根据 subversion的commit history进行代码比对和分析，试图查找到哪些新增或修改的代码引发了Bug。代码修改量小还好，如果修改数量巨大，这种代码比对就好比大海捞针，我 们无法保证注意力自始自终是集中的，结果两位同事也的确没有从代码变更中发现什么蛛丝马迹。这类Bug会让你有一种有力无处施展的感觉，面对这样 的Bug，我的两位开发人员似乎也失去了信心和思路。

下面简要描述一下这个Bug：

有这样一个字段数目众多的结构体foo_t，这里仅列出bug相关的几个字段e、c、flag、pdata：

struct foo_t {
    … …
    char e[XX_SIZE];
    char c[XX_SIZE];
    char flag;
    data_t *pdata;
    … …
}；

业务逻辑是：

if (flag) {
    处理e、c两个字段；
}
   
bug现象：值本是1的flag字段被污染，值变成了0，导致e、c两个字段没有被做处理，从而引发业务异常，导致客户投诉。我的同事曾经做过如 下尝试，以确定内存污染的行为特点，她在flag之前又加了一个字段flag1：

struct foo_t {
    … …
    char e[XX_SIZE];
    char c[XX_SIZE];
    unsigned int flag1;
    char flag;
    data_t *pdata;
    … …
}；

在生产环境下运行得到的结果是flag1和flag值正常，但字段c的尾部字节遭到了污染。现象已经十分明确，离真相就差那最后一公里了。

对于上面的内存污染问题，我首先会怀疑在处理flag或c之前的字段时出现了缓冲区溢出，导致后面字段的内容被整体或局部覆盖。不过从bug现象 来看，这个思路也有说不通的地方，那就是为何是c的尾部字段被污染，而不是从头部开始呢？不过我们依旧沿着这个思路追查了e以及e的诸多前驱字 段，细致的分析了代码，但没有发现溢出点。

c或flag的后继字段比如pdata要想污染c或flag则必须具备更多条件，至少要有操作&pdata的代码，之前基本认为这不太可 能。但现在仅有这一条路可以继续走下去了，也只能沿着这条路走下去。事实证明我们走的没错。在后续的处理流程中有这样的一个函数：

int func(void *p, int size)

这个函数本来是用于处理data_t*变量的，但由于编码者的疏忽，将&pdata传给了p，另外size这个参数也传了一个错误的值， 估计是滥用了copy&paste。而func函数体中对p指向的内存地址做了修改，这个修改直接污染了 ((char*)&pdata + size)起始的那片内存块儿，这就是问题的真正原因所在。这样看来pdata并未污染其所在的foo_t实例中的flag或c字段，而是污染了其他foo_t实例中的flag或c字段，因为这些实例都放在一个mem block pool中的，所以这还是一个随机的远距离内存污染^_^。

我走完了BUG查找的最后那一公里，到达了终点。这个BUG的查找确实不易，但并非遥不可及，为何我的两位同事就停在离真相只有一公里的地方而踌 躇不前了呢？对此我也做了一些考量，希望能在日后的BUG查找方面给予帮助。

要跨过BUG查找的那最后一公里，可从如下几个方面着手努力：

* 收罗证据，不放过一处可疑之处

这是准备工作，就好比警察查看罪案现场，哪怕是一根毛发，一处异物也不能放过。一般来说我们至少要收集到Bug发生时的各方面信息，包括：

 - 系统日志
        这个时间点上各个模块的日志都要搜罗到；

 - core文件
        如果bug引发core dump，那core文件是bug查找的最佳入口；

 - 通信数据包内容
        对于很多后端服务程序而言，不合法的通信数据包常常会引发Bug，我经手的类似Bug就不止一起了。必要时通过抓包工具将通信包抓到文件中以备后用。

 - CPU/内存/磁盘实时状况
       千万不要小视这些信息。如果发现CPU过高，则很可能代码存在死循环的可能（后pstack进程号，则可直接找到问题所在）；如果磁盘满，则可以很好解释 数据不完整的异常；如果mem占用过高，则可以解释分配内存异常或性能下降等问题。

 - 系统操作日志
       如果有管理员的操作行为的话，我们也不要放过，将操作日志（一般系统都有保存，并需要对这些日志进行定期审核）截取并保留，以备后用。

 - 操作系统/硬件相关异常信息等。
       如果是因为OS或硬件异常导致的Bug，那搜集到这些信息就太重要了，否则你将付出惨重的Bug查找代价。

Bug查多了你就会有这种感悟：证据用时方恨少啊！

* 沉下心，保持清晰思路

BUG有难有易，简单的Bug大家都能应付，而困难的Bug，就要比拼能力和经验了。要想解决掉Bug，务必要沉下心，不急不躁，这是保持大脑始 终有清晰思路的前提。

能用工具（比如GDB）调试出来的Bug，都不是最难的问题，因为现场就摆在你的面前，你可以看到一切蛛丝马迹。最难的问题最终都是要通过脑力分 析出来的。

解决问题前，要根据之前搜罗的证据，形成自己的查找思路。没有思路是可怕的。没有思路的时候，也不要急于开始查，那样只会乱套。应根据已有的蛛丝 马迹，行成一些思路，哪怕这个思路你自己都不是很肯定，先按这个思路做做看，也许走出一步后，你又能收获新的信息，形成新的思路。就这样敏捷地向 前进，边向前探索边定期回顾。

* 知晓原理，缩小查找范围，形成正确思路

要保持清晰正确的思路，开发人员对系统的运行原理要做到十分清楚，这样可以缩小查找范围，重点突破。就好比上面的那个bug例子，我们要知道 c/flag被污染有几种潜在的可能，并形成多种思路，然后沿着这几种可能的思路继续走下去。在这次查找过程中，想必两位同事恰恰是在原理这方面 没有理解透彻吧。

* 质疑，从自己的代码开始

查Bug就要抛弃“不可能”，拥抱“质疑一切”。而质疑要从自己的代码开始。程序员或多或少都有一种“自负”的心态，骨子里会认为自己的代码肯定 是正确的。如果出现问题，一定是其他人代码的问题，哪怕是OS这样总体来说十分稳定的平台也会成为被首先质疑的对象。不过事实证明，错误多出在我 们自己的代码中，毫无理由的去怀疑操作系统、怀疑你使用的第三方库，多半会南辕北辙，浪费你宝贵的查找时间。

* 拥抱调试技巧和工具

必要的调试技巧是Bug查找的基本功底，这些技巧在涉及内存问题查找过程中相当有用。

  — print语句
        不用多说，print语句是最简单、最常用的调试手段，在代码任意位置，根据你的需要，输出信息，帮助你分析bug原因。其唯一的缺点就是可能需要你重新 build代码和部署你的应用。

  — gdb切入进程地址空间查看堆栈
         利用gdb一类的专用调试工具可在代码运行时切入进程地址空间，实时查看数据变化。你也可以在gdb下执行应用，获得同样的效果（适合单进程应用）。
 
  — 调试版中采用magic number + assert
         C程序的bug多为内存问题。常见的内存越界访问或污染的调试手段是在代码中为内存块添加magic number，并在特定环节用assert保证该magic number的值是没有被修改的。一旦值改变了，则说明问题发生在执行流的两次assert之间的某个地方，后续可进一步缩小assert间隙，直到定位 到问题。

  — 让bug尽可能的容易复现
         一个可以在模拟环境下复现的Bug总是比较好查的。出于这个考虑，我们可通过放大问题区域来尽可能更容易的复现bug，比如将一个字节的字段改为4个字 节，这样可能占据更多被污染的区域，比较利于Bug的复现（但这不总是ok的）。

* 把握节奏，避免陷入惯性思维

一些比较难fix的Bug，其查找过程可能会十分漫长，就像这次我们遇到的这个问题。这就需要我们的开发人员把握好Bug查找的节奏，因为长时间 调试和查问题容易让人陷入惯性思维，反倒不利于Bug的查找。一旦意识到自己进入惯性思维后，可考虑换种活动做做，比如出去散散步、洗个热水澡 等。或者给其他人员讲解你的查找思路，这个过程中自己可能会发现思路上的缺陷，或者由他人指出你思路方面的问题。

感觉Bug查找是一门手艺活，要学会慢工出细活，这总比不出活儿的要好，尤其是在面对那些十分诡异的内存Bug时。