Hi Kurt,
On 11/10/2011 11:09 AM, Kurt Miller wrote:
On 11/09/11 03:01, Jonathan Lu wrote:
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.
Hi Jonathan,
The above email is a bit hard to follow due to the mixture of top and
bottom replies.
I can confirm that OpenBSD and Mac OS X 10.5.8 follow the Linux behavior
which confirms the need for platform ifdef's in this code.
Seems like you need to make the following change:
-#ifdef __solaris__
+#if defined(__solaris__) || defined(__AIX__)
or something similar to maintain compatibility.
In general the approach taken for adding BSD support was to never
assume you can change other supported code paths. If your architecture
follows an existing code path behavior add it like I did above.
Otherwise just create a #ifdef myarch section for it.
But for this case, I think it is a good idea to leave a default code
path here.
Since in src/solaris/native/java/util/TimeZone_md.c starting from line
624, the return value of getenv("TZ"); has to be tested afterward on any
platforms.
So to improve portability for OpenJDK, how about leaving Solaris style
checking as the default approach?
Unifying or changing another architecture's code path requires access
to the arch, research and confirmation that the change is ok. Typically
this may be done by writing independent test programs and running them
on each arch.
Regards,
-Kurt
