Hi, Please review the update to include the additional case and fix. http://cr.openjdk.java.net/~rriggs/webrev-overflow-8133022/ Thanks, Roger On 8/6/2015 12:37 PM, Roger Riggs wrote:

Hi Ivan,

I looked at that but didn't find a reproducer. I'll add that test case and respin.

Thanks, Roger

On 8/6/2015 12:34 PM, Ivan Gerasimov wrote:

...

Hi Roger!

There seems to be another numeric overflow possibility a the line# 1235, when 'seconds' is a large negative and 'nanos' is a small positive.

For example, ------------ Instant inst = Instant.ofEpochSecond(-9223372036854776L, 1); System.out.println(inst.toEpochMilli()); ------------ prints out 9223372036854775616.

Sincerely yours, Ivan

On 06.08.2015 18:33, Roger Riggs wrote:

...

Please review a small fix and test for Instant.toEpochMilli ArithmeticOverflow.

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-overflow-8133022/

Issue: https://bugs.openjdk.java.net/browse/JDK-8133022

Thanks, Roger

Fine by me. Stephen On 6 Aug 2015 19:15, "Chris Hegarty" <chris.hegarty@oracle.com> wrote:

On 6 Aug 2015, at 19:07, Roger Riggs <Roger.Riggs@oracle.com> wrote:

...

Hi,

Please review the update to include the additional case and fix.

http://cr.openjdk.java.net/~rriggs/webrev-overflow-8133022/

The updated version looks good Roger.

-Chris.

...

Thanks, Roger

On 8/6/2015 12:37 PM, Roger Riggs wrote:

...

Hi Ivan,

I looked at that but didn't find a reproducer. I'll add that test case and respin.

Thanks, Roger

On 8/6/2015 12:34 PM, Ivan Gerasimov wrote:

...

Hi Roger!

There seems to be another numeric overflow possibility a the line#

1235, when 'seconds' is a large negative and 'nanos' is a small positive.

...

...

...

For example, ------------ Instant inst = Instant.ofEpochSecond(-9223372036854776L, 1); System.out.println(inst.toEpochMilli()); ------------ prints out 9223372036854775616.

Sincerely yours, Ivan

On 06.08.2015 18:33, Roger Riggs wrote:

...

Please review a small fix and test for Instant.toEpochMilli

ArithmeticOverflow.

...

...

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-overflow-8133022/

Issue: https://bugs.openjdk.java.net/browse/JDK-8133022

Thanks, Roger

Thanks Roger! On 06.08.2015 21:07, Roger Riggs wrote:

Hi,

Please review the update to include the additional case and fix.

http://cr.openjdk.java.net/~rriggs/webrev-overflow-8133022/

In TCKInstant.java, the second testcase could be written as Instant.ofEpochSecond(Long.MIN_VALUE / 1000 - 1, 1).toEpochMilli(); for more readability and consistency with the other testcase. In Instant.java, I think, the check for (nanos > 0) at the line #1232 can be omitted, as it only optimizes one of 100'0000'000 possible cases. It seems that both branches should handle the case (nanos == 0) correctly. Otherwise looks good to me! Sincerely yours, Ivan

Thanks, Roger

On 8/6/2015 12:37 PM, Roger Riggs wrote:

...

Hi Ivan,

I looked at that but didn't find a reproducer. I'll add that test case and respin.

Thanks, Roger

On 8/6/2015 12:34 PM, Ivan Gerasimov wrote:

...

Hi Roger!

There seems to be another numeric overflow possibility a the line# 1235, when 'seconds' is a large negative and 'nanos' is a small positive.

For example, ------------ Instant inst = Instant.ofEpochSecond(-9223372036854776L, 1); System.out.println(inst.toEpochMilli()); ------------ prints out 9223372036854775616.

Sincerely yours, Ivan

On 06.08.2015 18:33, Roger Riggs wrote:

...

Please review a small fix and test for Instant.toEpochMilli ArithmeticOverflow.

Webrev: http://cr.openjdk.java.net/~rriggs/webrev-overflow-8133022/

Issue: https://bugs.openjdk.java.net/browse/JDK-8133022

Thanks, Roger