Patch to expand tz checking scope in TimeZone_md.c

Wed Nov 9 08:01:51 UTC 2011

On 11/04/2011 01:26 PM, David Holmes wrote:
> On 4/11/2011 2:53 PM, David Holmes wrote:
>> On 4/11/2011 2:13 PM, Masayoshi Okutsu wrote:
>>> Probably the difference isn't documented. I tried Solaris 10 and Ubuntu
>>> 10.03. The difference still exists.
>>>
>>> Solaris 10:
>>> $ unset TZ
>>> $ date
>>> Fri Nov 4 13:04:45 JST 2011
>>> $ TZ="" date
>>> Fri Nov 4 13:04:53 JST 2011
>>>
>>> Ubuntu 10.04:
>>> $ unset TZ
>>> $ date
>>> Fri Nov 4 13:05:50 JST 2011
>>> $ TZ="" date
>>> Fri Nov 4 04:05:55 UTC 2011
>>>
>>> When the TZ value is an empty string, Ubuntu uses UTC while Solaris
>>> still looks up the system default.
>>
>> I have to take back my comment regarding this not seeming to be platform
>> specific code - it is highly platform specific! It seems that on Linux
>> we are happy to report a TZ of "" but not so on Solaris. I presume this
>> is an attempt to keep Java's use of TZ consistent with how other apps
>> handle it on that platform. (environ(5) gives a little insight on
>> Solaris as to how TZ is used.)
>>
>> So the key thing here is to not disturb the existing behaviour on either
>> linux or Solaris - which suggests the original patch. That said I'm not
>> convinced - given this is so platform specific - that simply treating
>> non-linux the same as Solaris is a reasonable thing to do. I think it
>> would be useful to see what the BSD/OSX port(s) had to do with this code
>> - if anything.
>
> To answer my own queries BSD/OSX does
>
>       511 #if defined(__linux__) || defined(_ALLBSD_SOURCE)
>       512     if (tz == NULL) {
>       513 #else
>       514 #ifdef __solaris__
>       515     if (tz == NULL || *tz == '\0') {
>       516 #endif
>       517 #endif
>
> so the suggested patch would at least not interfere.
>
> Anyway this needs input from other core-libs folk. I didn't intend to 
> get quite so heavily involved. ;-)
>
> David
> -----
>
>
>
>> David
>> -----
>>
>>
>>> Thanks,
>>> Masayoshi
>>>
>>> On 11/3/2011 4:16 PM, Jonathan Lu wrote:
>>>> Hi Masayoshi,
>>>>
>>>> I did find some references about date-time related functions / TZ
>>>> variables on Linux but got only a few about Solaris, so could not see
>>>> any differences between those two platforms about the changes
>>>> described in my patch. Have you got any links or references about
>>>> these differences? I'm interested in it and may update the patch again
>>>> after reading them.
>>>>
>>>> Thanks a lot!
>>>> - Jonathan
>>>>
>>>> On 11/02/2011 10:27 PM, Masayoshi Okutsu wrote:
>>>>> Hi Jonathan,
>>>>>
>>>>> IIRC, the difference came from some behavioral difference between the
>>>>> Linux and Solaris libc date-time functions and/or the date command,
>>>>> and TimeZone_md.c tries to follow the difference. But the code was
>>>>> written looooong ago. The difference may no longer exist.
>>>>>
>>>>> Thanks,
>>>>> Masayoshi
>>>>>
>>>>> On 11/2/2011 8:39 PM, Jonathan Lu wrote:
>>>>>> On 11/02/2011 07:00 PM, David Holmes wrote:
>>>>>>> On 2/11/2011 7:01 PM, Jonathan Lu wrote:
>>>>>>>> On 11/02/2011 04:56 PM, Jonathan Lu wrote:
>>>>>>>>> Hi core-libs-dev,
>>>>>>>>>
>>>>>>>>> In jdk/src/solaris/native/java/util/TimeZone_md.c, starting from
>>>>>>>>> line
>>>>>>>>> 626, I found that the scope of "#ifdef __solaris__" might be too
>>>>>>>>> narrow, since it also works for some kind of OS which I'm 
>>>>>>>>> currently
>>>>>>>>> working on, such as AIX.
>>>>>>>>> So I suggest to just remove the '#ifdef __solaris__' and leave 
>>>>>>>>> the
>>>>>>>>> "#else" to accommodate more conditions, see attachment
>>>>>>>>> 'patch.diff'. I
>>>>>>>>> think this may enhance the cross-platform ability, any ideas 
>>>>>>>>> about
>>>>>>>>> this modification?
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> - Jonathan Lu
>>>>>>>> I'm not sure why the attachment got filtered, here paste it to the
>>>>>>>> mail
>>>>>>>> content directly.
>>>>>>>>
>>>>>>>> diff -r 4788745572ef src/solaris/native/java/util/TimeZone_md.c
>>>>>>>> --- a/src/solaris/native/java/util/TimeZone_md.c Mon Oct 17 
>>>>>>>> 19:06:53
>>>>>>>> 2011 -0700
>>>>>>>> +++ b/src/solaris/native/java/util/TimeZone_md.c Thu Oct 20 
>>>>>>>> 13:43:47
>>>>>>>> 2011 +0800
>>>>>>>> @@ -626,10 +626,8 @@
>>>>>>>> #ifdef __linux__
>>>>>>>> if (tz == NULL) {
>>>>>>>> #else
>>>>>>>> -#ifdef __solaris__
>>>>>>>> if (tz == NULL || *tz == '\0') {
>>>>>>>> #endif
>>>>>>>> -#endif
>>>>>>>> tz = getPlatformTimeZoneID();
>>>>>>>> freetz = tz;
>>>>>>>> }
>>>>>>>
>>>>>>> I'm unclear why any of that code needs to be platform specific - is
>>>>>>> an empty TZ string somehow valid on linux? I would have thought the
>>>>>>> following would be platform neutral:
>>>>>>>
>>>>>>> if (tz == NULL || *tz == '\0') {
>>>>>>> tz = getPlatformTimeZoneID();
>>>>>>> freetz = tz;
>>>>>>> }
>>>>>>>
>>>>>> Hi David,
>>>>>>
>>>>>> getenv("TZ") returns NULL when TZ environment variable is not set at
>>>>>> all and returns '\0' when TZ was exported as empty string. After
>>>>>> more checking for both cases, I agree with you, nothing useful can
>>>>>> be retrieved from that environment variable.
>>>>>> So I changed the patch to this,
>>>>>>
>>>>>> diff -r 7ab0d613cd1a src/solaris/native/java/util/TimeZone_md.c
>>>>>> --- a/src/solaris/native/java/util/TimeZone_md.c Thu Oct 20 10:32:47
>>>>>> 2011 -0700
>>>>>> +++ b/src/solaris/native/java/util/TimeZone_md.c Wed Nov 02 19:34:51
>>>>>> 2011 +0800
>>>>>> @@ -623,13 +623,7 @@
>>>>>>
>>>>>> tz = getenv("TZ");
>>>>>>
>>>>>> -#ifdef __linux__
>>>>>> - if (tz == NULL) {
>>>>>> -#else
>>>>>> -#ifdef __solaris__
>>>>>> if (tz == NULL || *tz == '\0') {
>>>>>> -#endif
>>>>>> -#endif
>>>>>> tz = getPlatformTimeZoneID();
>>>>>> freetz = tz;
>>>>>> }
>>>>>>
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>> Regards
>>>>>>>> - Jonathan Lu
>>>>>> Regards
>>>>>>
>>>>>> - Jonathan
>>>>>>
>>>>
Copy to bsd-port-dev and macosx-port-dev lists to see if anybody here 
has some ideas about this issue.
-Jonathan