RFR: 8250636: iso8601_time returns incorrect offset part on MacOS

[David Holmes]

Hi Dmitry,

On 31/07/2020 8:29 pm, Dmitry Cherepanov wrote:
> Hi Gerard,
> 
> Thanks for reviewing this, moving the logic of get_timezone to
> iso8601_time looks good to me too.
> Here's an updated webrev
> http://cr.openjdk.java.net/~dcherepanov/8250636/webrev.v4/

This version also looks good to me. But now I see all the code clearly 
in one place I have to wonder why we are not using tm_gmtoff on all 
platforms except Windows? That would simplify the logic even further.

That goes beyond fixing the current actual bug though so I'll leave that 
choice to you.

Thanks,
David

> Thanks,
> 
> Dmitry
> 
> On 30.07.2020 19:52, gerard ziemski wrote:
>> hi Dmitry,
>>
>> Looks good when it comes to the core approach of the fix, but may I
>> suggest simplifying the code a bit? Perhaps something like this:
>>
>> http://cr.openjdk.java.net/~gziemski/8250636_rev1/index.html
>>
>> I do not like the duplication of "const time_t” constants and I don’t
>> think that including more logic code in “get_timezone()” does much for
>> readability of the code here.
>>
>> Thank you for catching and fixing it!
>>
>>
>> cheers
>>
>>
>> On 7/30/20 4:32 AM, Dmitry Cherepanov wrote:
>>> Hello,
>>>
>>> Please review a small change for fixing offset (timezone) part of the
>>> string returned by os::iso8601_time on MacOS. The patch negates
>>> tm_gmtoff and skips the DST adjustment when tm_gmtoff is used. The
>>> behavior on other platforms should remain unchanged. More details are in
>>> the bug report.
>>>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8250636
>>> Webrev: http://cr.openjdk.java.net/~dcherepanov/8250636/webrev.v3/
>>>
>>> Thanks,
>>>
>>> Dmitry
>>>
>