Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Mon, 11 Oct 2021 18:18:02 GMT, Naoto Sato wrote: >> Wu Yan has updated the pull request incrementally with one additional commit >> since the last revision: >> >> replace realpath > > src/java.base/unix/native/libjava/path_util.h line 31: > >> 29: int collapsible(char *names); >> 30: void splitNames(char *names, char **ix); >> 31: void joinNames(char *names, int nc, char **ix); > > Are these functions, `collapsible`, `splitNames` and `joinNames` have to be > non-static? You are right, thanks for your suggestions, I'll change these functions to static in next commit. - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Mon, 11 Oct 2021 18:16:28 GMT, Naoto Sato wrote: >> Wu Yan has updated the pull request incrementally with one additional commit >> since the last revision: >> >> replace realpath > > src/java.base/unix/native/libjava/TimeZone_md.c line 113: > >> 111: } >> 112: } >> 113: > > There are a few `*(right + 1)` references in the loops. Is there any > possibility that it would run over the buffer? It wouldn't run over the buffer. Here `end` points to `'\0'`. At line 94 and line 99, `right` is less than `end`, so `right + 1` is at most `end`. At line 103, if `right` equals `end`, `*right != '\0'` will be false and `!(*right == '/' && *(right + 1) == '/')` will not executed. - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Thu, 9 Sep 2021 08:25:44 GMT, Wu Yan wrote: >> Hi, >> Please help me review the change to enhance getting time zone ID from >> /etc/localtime on linux. >> >> We use `realpath` instead of `readlink` to obtain the link name of >> /etc/localtime, because `readlink` can only read the value of a symbolic of >> link, not the canonicalized absolute pathname. >> >> For example, the value of /etc/localtime is >> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by >> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of >> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which >> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you >> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. >> >> Thanks, >> wuyan > > Wu Yan has updated the pull request incrementally with one additional commit > since the last revision: > > replace realpath src/java.base/unix/native/libjava/TimeZone_md.c line 113: > 111: } > 112: } > 113: There are a few `*(right + 1)` references in the loops. Is there any possibility that it would run over the buffer? src/java.base/unix/native/libjava/path_util.h line 31: > 29: int collapsible(char *names); > 30: void splitNames(char *names, char **ix); > 31: void joinNames(char *names, int nc, char **ix); Are these functions, `collapsible`, `splitNames` and `joinNames` have to be non-static? - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
On Thu, 9 Sep 2021 08:25:44 GMT, Wu Yan wrote: >> Hi, >> Please help me review the change to enhance getting time zone ID from >> /etc/localtime on linux. >> >> We use `realpath` instead of `readlink` to obtain the link name of >> /etc/localtime, because `readlink` can only read the value of a symbolic of >> link, not the canonicalized absolute pathname. >> >> For example, the value of /etc/localtime is >> "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by >> `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of >> `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which >> consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you >> can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. >> >> Thanks, >> wuyan > > Wu Yan has updated the pull request incrementally with one additional commit > since the last revision: > > replace realpath Hi, we directly process the characters in `linkbuf` instead of realpath. And we reuse [collapse()](https://github.com/openjdk/jdk/blob/master/src/java.base/unix/native/libjava/canonicalize_md.c#L129) to process the '.' and '..' in `linkbuf`. We tested some cases about /etc/localtime, and it works well. Test cases and results are in the attachment. [testcase.log](https://github.com/openjdk/jdk/files/7135028/testcase.log) [result.log](https://github.com/openjdk/jdk/files/7135030/result.log) - PR: https://git.openjdk.java.net/jdk/pull/5327
Re: RFR: 8273111: Default timezone should return zone ID if /etc/localtime is valid but not canonicalization on linux [v2]
> Hi, > Please help me review the change to enhance getting time zone ID from > /etc/localtime on linux. > > We use `realpath` instead of `readlink` to obtain the link name of > /etc/localtime, because `readlink` can only read the value of a symbolic of > link, not the canonicalized absolute pathname. > > For example, the value of /etc/localtime is > "../usr/share/zoneinfo//Asia/Shanghai", then the linkbuf obtained by > `readlink` is "../usr/share/zoneinfo//Asia/Shanghai", and then the call of > `getZoneName(linkbuf)` will get "/Asia/Shanghai", not "Asia/Shanghai", which > consider as invalid in `ZoneInfoFile.getZoneInfo()`. Using `realpath`, you > can get “/usr/share/zoneinfo/Asia/Shanghai“ directly from “/etc/localtime“. > > Thanks, > wuyan Wu Yan has updated the pull request incrementally with one additional commit since the last revision: replace realpath - Changes: - all: https://git.openjdk.java.net/jdk/pull/5327/files - new: https://git.openjdk.java.net/jdk/pull/5327/files/9b29edfc..19cc634d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=5327=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5327=00-01 Stats: 395 lines in 4 files changed: 241 ins; 145 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/5327.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5327/head:pull/5327 PR: https://git.openjdk.java.net/jdk/pull/5327
