code review 15080043: time: fix ParseDuration overflow when given more than 9...

time: fix ParseDuration overflow when given more than 9 digits on 32-bit arch
Fixes issue 6617.

[minux1, 2013-10-18 16:16:08]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[rsc, 2013-10-18 17:26:36]

https://codereview.appspot.com/15080043/diff/50001/src/pkg/time/format.go
File src/pkg/time/format.go (right):

https://codereview.appspot.com/15080043/diff/50001/src/pkg/time/format.go#new...
src/pkg/time/format.go:1207: scale := int64(1)
might as well do 1.0 (float64).
the int64 is good for 19 digits, the float64 is good for 22 digits, and the
float64 will degrade much better.

[minux1, 2013-10-20 01:20:44]

PTAL.

https://codereview.appspot.com/15080043/diff/50001/src/pkg/time/format.go
File src/pkg/time/format.go (right):

https://codereview.appspot.com/15080043/diff/50001/src/pkg/time/format.go#new...
src/pkg/time/format.go:1207: scale := int64(1)
On 2013/10/18 17:26:36, rsc wrote:
> might as well do 1.0 (float64).
> the int64 is good for 19 digits, the float64 is good for 22 digits, and the
> float64 will degrade much better.
i originally used int64 because if scale can be represented in int64,
leadingInt(s) above would already give errors.

but then further testing that giving 19 digits after the decimal point is ok,
but even the int in amd64 can't handle it (so the issue is not specific to 386
anymore).

i also changed the test so that it would fail on amd64 without the fix.

[rsc, 2013-10-21 20:11:18]

LGTM

[r, 2013-10-22 20:21:12]

LGTM

https://codereview.appspot.com/15080043/diff/100001/src/pkg/time/time_test.go
File src/pkg/time/time_test.go (right):

https://codereview.appspot.com/15080043/diff/100001/src/pkg/time/time_test.go...
src/pkg/time/time_test.go:1334: //{"0.01234567890123456789h", false, 0},
delete?

[rsc, 2013-10-22 22:33:14]

*** Submitted as https://code.google.com/p/go/source/detail?r=fbf3b853e00b ***

time: fix ParseDuration overflow when given more than 9 digits on 32-bit arch
Fixes issue 6617.

R=golang-dev, rsc, r
CC=golang-dev
https://codereview.appspot.com/15080043

Committer: Russ Cox <rsc@golang.org>

[minux1, 2013-10-23 04:31:52]

Thank you Russ for submitting.

Should I mail CL to release-apply this CL to release-branch.go1.2?

[r, 2013-10-23 04:53:03]

Andrew will do that.

-rob