Patch to expand tz checking scope in TimeZone_md.c (original) (raw)
Jonathan Lu luchsh at linux.vnet.ibm.com
Wed Nov 9 00:01:51 PST 2011
- Previous message: Java for Mac OS X 10.7 Update 1 and 10.6 Update 6 is live
- Next message: Patch to expand tz checking scope in TimeZone_md.c
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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(ALLBSDSOURCE) 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 TimeZonemd.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/TimeZonemd.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/TimeZonemd.c --- a/src/solaris/native/java/util/TimeZonemd.c Mon Oct 17 19:06:53 2011 -0700 +++ b/src/solaris/native/java/util/TimeZonemd.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/TimeZonemd.c --- a/src/solaris/native/java/util/TimeZonemd.c Thu Oct 20 10:32:47 2011 -0700 +++ b/src/solaris/native/java/util/TimeZonemd.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
- Previous message: Java for Mac OS X 10.7 Update 1 and 10.6 Update 6 is live
- Next message: Patch to expand tz checking scope in TimeZone_md.c
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]