小赖子的英国生活和资讯

一行代码修改引起的血案 – (二)

阅读 桌面完整版

上一篇说到乱改了一行代码引来的错误, 自己也检讨了一下. 以下已经同事同意:

看了XX在群里发的内容 http://coolshell.cn/articles/4576.html,然后到
http://thread.gmane.org/gmane.linux.kernel/1124982/focus=1126216
仔细看了Linus Torvalds发的邮件,受益匪浅,摘了几段内容附在了 < how to fix bug.xls > 中.

举个咱工作中与此相同的真实例子:
我昨晚加班fix bug—NSD无法加入设备,
首先尝试将几个相关的文件恢复到以前的版本,问题依然存在.
肯定是没找对文件或恢复的不对,于是将代码更新到最新版本,开始调试.
发现在LayoutView类的Initialize方法中,有这么一行

// set default tool             
activeTool = LayoutToolType.Pointer;

我想把这句删掉,但是怕万一出现activeTool为null的情况,于是就保留并提交了代码.
Bug被”修复”了.

// set default tool             
if (null == activeTool) activeTool = LayoutToolType.Pointer;

这个过程中有几个问题:
1.我查看svn log,恢复相关文件到以前版本时,生怕漏掉某个修改,于是看着稍微有点相关的修改,都恢复到了以前的版本,但是依然把关键的一处修改漏掉了. (总结:遇到类似问题时,一定不要放过任何修改,很多bug正是由看似没问题的代码导致的.)

2.第一次调试发现 activeTool 被赋值为Pointer时,意识到问题就在这里,去核对文件修改历史,
咋看之下,并无问题.想着可能是其他文件的修改导致的,于是继续调试,无果,又回头来看这个地方,并恢复旧版本,发现原来的if (null == activeTool)始终不会进入if分支(最初看到if被删掉的修改记录时,以为始终会进入分支,因为方法名为Initialize,summary为”初始化操作”,编写合适的方法名及必要的summary还是很必要的),而新版本始终会走这个分支.由此确定这就是问题的根源.(总结:一旦发现可疑之处,就应先确定是不是它的问题,而不应仅凭直觉.)

3.我经过简单思考后,决定不删这行,而是保留if的判断,提交代码.没有继续查找activeTool为什么不会为null,其实我以前fix bug都是会追根究底的,但昨晚加班媳妇催着回家,我就提交了代码,并把问题抛给了XX.(总结:bug不能看似解决就算ok,要understanding the problem 并 fix root cause,像这个问题,我恢复了原来的if并提交,难免以后不会再有人把if删掉,那问题又出来了,所以遗留下隐患,问题不能算真正的解决了;每个人都很忙,把问题抛给别人不是解决办法;其实解决问题很简单,只要多花两秒看看LayoutToolType的类型就行了,因为它是enum类型,所以始终不会为null,最后我把if整行删掉并提交了代码.)

强烈推荐

微信公众号: 小赖子的英国生活和资讯 JustYYUK

阅读 桌面完整版
Exit mobile version