Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(727)

Issue 10328043: code review 10328043: time: handle integer overflow in Sub (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 7 months ago by rick
Modified:
7 years, 7 months ago
Reviewers:
r
CC:
golang-dev, bradfitz, r, rsc
Visibility:
Public.

Description

time: handle integer overflow in Sub If time.Sub results in a value that won't fit in a Duration (int64), return either the min or max int64 value as appropriate. Fixes issue 5011.

Patch Set 1 #

Patch Set 2 : diff -r 2879112bff3d https://code.google.com/p/go #

Patch Set 3 : diff -r 2879112bff3d https://code.google.com/p/go #

Total comments: 6

Patch Set 4 : diff -r 0b9e3725cc84 https://code.google.com/p/go #

Total comments: 5

Patch Set 5 : diff -r 0b9e3725cc84 https://code.google.com/p/go #

Total comments: 2

Patch Set 6 : diff -r 0b9e3725cc84 https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+52 lines, -2 lines) Patch
M src/pkg/time/time.go View 1 2 3 4 2 chunks +18 lines, -2 lines 0 comments Download
M src/pkg/time/time_test.go View 1 2 3 4 5 1 chunk +34 lines, -0 lines 0 comments Download

Messages

Total messages: 13
rick
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
7 years, 7 months ago (2013-06-17 00:00:48 UTC) #1
bradfitz
R=rsc
7 years, 7 months ago (2013-06-18 00:35:29 UTC) #2
r
https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go File src/pkg/time/time.go (right): https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go#newcode624 src/pkg/time/time.go:624: if u.sec > 0 && t.sec < minDuration+u.sec { ...
7 years, 7 months ago (2013-06-18 00:47:11 UTC) #3
rsc
https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go File src/pkg/time/time.go (right): https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go#newcode427 src/pkg/time/time.go:427: const ( Delete from here. https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go#newcode428 src/pkg/time/time.go:428: minDuration = ...
7 years, 7 months ago (2013-06-21 19:00:08 UTC) #4
rsc
https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go File src/pkg/time/time.go (right): https://codereview.appspot.com/10328043/diff/2002/src/pkg/time/time.go#newcode427 src/pkg/time/time.go:427: const ( On 2013/06/21 19:00:09, rsc wrote: > Delete ...
7 years, 7 months ago (2013-06-21 19:00:34 UTC) #5
rick
I forgot to mail when I made comments earlier, sorry. Do I need to worry ...
7 years, 7 months ago (2013-06-21 21:24:13 UTC) #6
rick
PTAL
7 years, 7 months ago (2013-06-21 21:35:26 UTC) #7
r
https://codereview.appspot.com/10328043/diff/11001/src/pkg/time/time.go File src/pkg/time/time.go (right): https://codereview.appspot.com/10328043/diff/11001/src/pkg/time/time.go#newcode624 src/pkg/time/time.go:624: d := Duration(t.sec-u.sec)*Second + Duration(t.nsec-u.nsec) // Check for overflow ...
7 years, 7 months ago (2013-06-21 21:37:15 UTC) #8
rick
PTAL https://codereview.appspot.com/10328043/diff/11001/src/pkg/time/time.go File src/pkg/time/time.go (right): https://codereview.appspot.com/10328043/diff/11001/src/pkg/time/time.go#newcode624 src/pkg/time/time.go:624: d := Duration(t.sec-u.sec)*Second + Duration(t.nsec-u.nsec) On 2013/06/21 21:37:15, ...
7 years, 7 months ago (2013-06-21 21:44:52 UTC) #9
r
https://codereview.appspot.com/10328043/diff/11001/src/pkg/time/time.go File src/pkg/time/time.go (right): https://codereview.appspot.com/10328043/diff/11001/src/pkg/time/time.go#newcode632 src/pkg/time/time.go:632: } my mistake, sorry. the colors confuse me sometimes. ...
7 years, 7 months ago (2013-06-21 21:53:18 UTC) #10
rick
PTAL https://codereview.appspot.com/10328043/diff/17001/src/pkg/time/time_test.go File src/pkg/time/time_test.go (right): https://codereview.appspot.com/10328043/diff/17001/src/pkg/time/time_test.go#newcode1359 src/pkg/time/time_test.go:1359: t.Errorf("%d: got %d; want %d", i, got, st.d) ...
7 years, 7 months ago (2013-06-22 00:56:34 UTC) #11
r
LGTM
7 years, 7 months ago (2013-06-22 01:05:41 UTC) #12
r
7 years, 7 months ago (2013-06-22 01:08:01 UTC) #13
*** Submitted as https://code.google.com/p/go/source/detail?r=56909cb770fe ***

time: handle integer overflow in Sub

If time.Sub results in a value that won't fit in a Duration (int64),
return either the min or max int64 value as appropriate.

Fixes issue 5011.

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

Committer: Rob Pike <r@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b