RFR : 7149608 (tz): Default TZ detection fails on linux when symbolic links to non default location used.
Issue seen on somewhat irregular linux system configuration where /etc/localtime is a symbolic link to a directory outside of /usr/share/zoneinfo. In past, when a symbolic link was seen, the end target file was assumed to be under /usr/share/zoneinfo and a string comparison match was attempted. We need to factor in the case where symbolic link points to any other location and match the end target file with a TZ file from /usr/share/zoneinfo via findZoneinfoFile function. webrev : http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8/ bug report : http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7149608 regards, Sean.
On 09/03/2012 16:00, Seán Coffey wrote:
Issue seen on somewhat irregular linux system configuration where /etc/localtime is a symbolic link to a directory outside of /usr/share/zoneinfo.
In past, when a symbolic link was seen, the end target file was assumed to be under /usr/share/zoneinfo and a string comparison match was attempted. We need to factor in the case where symbolic link points to any other location and match the end target file with a TZ file from /usr/share/zoneinfo via findZoneinfoFile function.
webrev : http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8/ bug report : http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7149608
regards, Sean.
cc'ing i18n-dev as that is where the TZ code is maintained. The changes look okay but I think it would be more efficient to fstat the zone info file after opening it. -Alan
Ok - good point on the stat change Alan. I think this is what you're after : http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.2/ regards, Sean. On 12/03/12 11:04, Alan Bateman wrote:
On 09/03/2012 16:00, Seán Coffey wrote:
Issue seen on somewhat irregular linux system configuration where /etc/localtime is a symbolic link to a directory outside of /usr/share/zoneinfo.
In past, when a symbolic link was seen, the end target file was assumed to be under /usr/share/zoneinfo and a string comparison match was attempted. We need to factor in the case where symbolic link points to any other location and match the end target file with a TZ file from /usr/share/zoneinfo via findZoneinfoFile function.
webrev : http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8/ bug report : http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7149608
regards, Sean.
cc'ing i18n-dev as that is where the TZ code is maintained.
The changes look okay but I think it would be more efficient to fstat the zone info file after opening it.
-Alan
On 12/03/2012 14:31, Seán Coffey wrote:
Ok - good point on the stat change Alan. I think this is what you're after :
At L295 then I assume itshould open DEFAULT_ZONEINFO_FILE, otherwise if the sym link is a relative path then it would be opened relative to the working directory rather than the link. -Alan.
Yes - good catch. I hadn't tested the sym link being a relative path. We should always open whatever is pointed to from DEFAULT_ZONEINFO_FILE. This simplifies the code. Tested and looks good. regards, Sean. On 12/03/12 14:34, Alan Bateman wrote:
On 12/03/2012 14:31, Seán Coffey wrote:
Ok - good point on the stat change Alan. I think this is what you're after :
At L295 then I assume itshould open DEFAULT_ZONEINFO_FILE, otherwise if the sym link is a relative path then it would be opened relative to the working directory rather than the link.
-Alan.
On 12/03/2012 15:11, Seán Coffey wrote:
Yes - good catch. I hadn't tested the sym link being a relative path. We should always open whatever is pointed to from DEFAULT_ZONEINFO_FILE. This simplifies the code. Tested and looks good. I assume this is the latest: http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.3/
and it looks fine to me. -Alan.
fd needs to be closed when fstat or malloc failed? Thanks, Masayoshi On 3/13/2012 12:22 AM, Alan Bateman wrote:
On 12/03/2012 15:11, Seán Coffey wrote:
Yes - good catch. I hadn't tested the sym link being a relative path. We should always open whatever is pointed to from DEFAULT_ZONEINFO_FILE. This simplifies the code. Tested and looks good. I assume this is the latest: http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.3/
and it looks fine to me.
-Alan.
Update made. Hopefully the last iteration ;) http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.4/ regards, Sean. On 13/03/2012 05:59, Masayoshi Okutsu wrote:
fd needs to be closed when fstat or malloc failed?
Thanks, Masayoshi
On 3/13/2012 12:22 AM, Alan Bateman wrote:
On 12/03/2012 15:11, Seán Coffey wrote:
Yes - good catch. I hadn't tested the sym link being a relative path. We should always open whatever is pointed to from DEFAULT_ZONEINFO_FILE. This simplifies the code. Tested and looks good. I assume this is the latest: http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.3/
and it looks fine to me.
-Alan.
Looks good to me. Masayoshi On 3/13/2012 6:38 PM, Seán Coffey wrote:
Update made. Hopefully the last iteration ;)
http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.4/
regards, Sean.
On 13/03/2012 05:59, Masayoshi Okutsu wrote:
fd needs to be closed when fstat or malloc failed?
Thanks, Masayoshi
On 3/13/2012 12:22 AM, Alan Bateman wrote:
On 12/03/2012 15:11, Seán Coffey wrote:
Yes - good catch. I hadn't tested the sym link being a relative path. We should always open whatever is pointed to from DEFAULT_ZONEINFO_FILE. This simplifies the code. Tested and looks good. I assume this is the latest: http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.3/
and it looks fine to me.
-Alan.
On 13/03/2012 09:38, Seán Coffey wrote:
Update made. Hopefully the last iteration ;)
http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.4/ Looks okay to me. For bonus points, open, fstat and read should be restarted if interrupted (EINTR).
-Alan.
I'll push changes as are Alan. You've a good point on how we should handle EINTR for such system calls. I think it's something that's relevant to more than just this file and have filed bug 7153347 to follow that. regards, Sean. On 13/03/2012 09:54, Alan Bateman wrote:
On 13/03/2012 09:38, Seán Coffey wrote:
Update made. Hopefully the last iteration ;)
http://cr.openjdk.java.net/~coffeys/webrev.7149608.jdk8.4/ Looks okay to me. For bonus points, open, fstat and read should be restarted if interrupted (EINTR).
-Alan.
participants (3)
-
Alan Bateman
-
Masayoshi Okutsu
-
Seán Coffey