RFR 9: 8133022: Instant.toEpochMilli() silently overflows

[Ivan Gerasimov]

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
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>
>
>