惨，给 Go 提的代码被批麻了

2022 年 3 月 08 日
本文字数：2453 字
阅读完需：约 8 分钟

hello 大家好，我是小楼。

不知道大家还记不记得我上次找到了一个 Go 的 Benchmark 执行会超时的 Bug？就是这篇文章《我好像发现了一个Go的Bug？》。

之后我就向 Go 提交了一个 PR 进行修复，本想等着代码被 Merge 进去，以后也可以吹牛说自己是个 Go 的 Contributor，但事情并不顺利，今天就来分享一下这次失败的代码提交。

第一次提交

在我意识到 Bug 时，就迫不及待想去修复，于是有了这一次提交。

在说代码前，先说点关于 Go 仓库的问题，Go 并没有直接托管在 github，而是自建的 Gerrit Code Review，github 上只是个镜像仓库，所有在 github 上提交的 issue 和代码都会被一个机器人搬运到 Gerrit 上。

而且 Go 对提交代码的要求是必须关联一个 issue，于是我就提了一个，自问自答了属于是。

描述了一下遇到的问题，但隔天被一位大佬认为是重复问题，并且关闭了这个 issue

但我点进去仔细看了下，和我说的应该没有关系，他们讨论的是单测超时不生效的问题，于是我狡辩了一下。

果然狡辩是有用的，另一位大佬同意我的观点，于是我给他点了个赞，但他也指出我的代码存在问题。

下面进入今天的正题，为了便于讲解，我先把有问题的代码段摘出来：

func (b *B) launch() {   ...    // n（int64）可能会溢出   n = goalns * prevIters / prevns   ...}

复制代码

既然知道 n 会溢出，还不简单？加个判断就完了。

溢出考虑不全

这位大佬说我的代码在防止 int64 溢出时不够安全，难道溢出不是这样判断吗？

不过还好，大佬给了一点点指导

同时也发来一段演示代码

果然「show me the code」最好使，简单点来说就是正数溢出成了负数，再溢出就又是正数，只要溢出足够多，结果可正可负。

得有测试

大佬还指出了另一个问题，兄弟，你写的代码得有有测试啊！

虽然我给开源项目提交代码不多，但也知道这点，为什么这次没写呢？主要是我觉得单测不太好写，既然大佬提出来，硬着头皮也得写了。

第二次提交

第二次提交，改掉了之前判断 int64 溢出的方法，用逆运算还原回去和原值做对比来看是否溢出，这个方法上次用到还是在大学的 C 语言课程中

还附加了一个单元测试

这个单元测试稍微解释下：

设置了 150s 的单测时间，每次试探单测时，次数都加 1，如果试探次数超过 6 次，就说明有问题，终止单测。

这段代码在上述溢出判断加之前执行，一定是失败的，溢出判断加了之后，则可正常执行。

接下来就是等待回复，等了很久很久，Go 的研发周期是以半年记，等得我都差点忘了这件事了，直到一天邮件提醒我。

前方高能，来看看另一位大佬是如何 review 我的代码的。

commit message 不规范

首先，commit message 不规范，我的 commit message 是这样的，我是在 github 上提交的，被机器人搬运过去。

给出的意见是

原来 Go 的 commit message 是有一个文档专门介绍的，之前没注意到，点进去看了下

翻译下就是 commit message 的第一行应该是简短的摘要，并且要指出影响了哪些包，第一行后得有一个空行。

commit message 的主要内容应该详细说明变更的上下文，并解释其作用，语句完整、标点正确，不要使用 HTML、Markdown 等标记语言。相关的信息，如基准测试数据等也需要写进来。

最后需要有关联的 issue，如果是修复某问题，需要用Fixes #12345来关联 12345 号问题，如果只是解决部分问题，使用Updates #12345，如果修复的是 golang.org/x/库，使用Fixes golang/go#159。

一个好的例子如下：

math: improve Sin, Cos and Tan precision for very large arguments
The existing implementation has poor numerical properties forlarge arguments, so use the McGillicutty algorithm to improveaccuracy above 1e10.
The algorithm is described at https://wikipedia.org/wiki/McGillicutty_AlgorithmFixes #159

看来我得好好改下 commit message。