RFR: 8250636: iso8601_time returns incorrect offset part on MacOS
David Holmes
david.holmes at oracle.com
Mon Aug 3 07:53:38 UTC 2020
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
>>>
>
More information about the hotspot-runtime-dev
mailing list