Patch to expand tz checking scope in TimeZone_md.c

[David Holmes]

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.

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