On Thu, 14 Oct 2021 02:08:45 GMT, Wu Yan <wu...@openjdk.org> 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 with a new target base due to a merge or 
> a rebase. The incremental webrev excludes the unrelated changes brought in by 
> the merge/rebase. The pull request contains four additional commits since the 
> last revision:
> 
>  - Merge branch 'master' into timezone
>  - change functions to be static
>  - replace realpath
>  - 8273111: Default timezone should return zone ID if /etc/localtime is valid 
> but not canonicalization on linux

src/java.base/unix/native/libjava/TimeZone_md.c line 82:

> 80: 
> 81: /*
> 82:  * remove repeated path separators ('/') in the giving 'path'.

given?

src/java.base/unix/native/libjava/TimeZone_md.c line 89:

> 87:     char *left = path;
> 88:     char *right = path;
> 89:     char *end = path + strlen(path);

"char* end"? better to align with existing code style

src/java.base/unix/native/libjava/TimeZone_md.c line 95:

> 93:     for (; right < end; right++) {
> 94:         if (*right == '/' && *(right + 1) == '/') break;
> 95:     }

Is this for loop necessary? Seems it's ok to merge with the nested loop below.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5327

Reply via email to