Hi Suenaga-san,
Thank you for the update. I think your changes to the return values of
MultiByteToWideChar look good.
Then I noticed (should have noticed in the first place, sorry) that the
error handling in canonicalize_md.c is incorrect (unrelated to your
change). All error cases "goto finish" which would end up "free(NULL)"
in some cases, e.g., line 340 (in the new file) should not end up
calling "free(wresult)" and "free(wpath)"
java_md.c
- "convert_to_unicode()" has different indentation than others (2 spaces
which should be 4).
- Should "wpath" be set to NULL before returning EINVAL at line 524? I
don't know the contract of "create_unc_path", but the caller would
receive a garbage pointer as a return value.
Nit: please add "noreg-hard" tag to the JIRA issue.
Naoto
On 3/9/20 7:52 PM, Yasumasa Suenaga wrote:
I forgot to free buffer if MultiByteToWideChar() failed in zip_util.c .
So I updated webrev:
http://cr.openjdk.java.net/~ysuenaga/JDK-8240725/webrev.02/
Yasumasa
On 2020/03/10 9:12, Yasumasa Suenaga wrote:
Hi Sato-san,
Thanks for your comment!
I updated webrev. Could you review again?
http://cr.openjdk.java.net/~ysuenaga/JDK-8240725/webrev.01/
Yasumasa
On 2020/03/10 2:24, naoto.s...@oracle.com wrote:
Hi Suenaga-san,
I think the return value from the second MultiByteToWideChar
invocation should be examined (non-zero), and if it was not
successful, appropriate action should be taken.
Naoto
On 3/9/20 3:50 AM, Yasumasa Suenaga wrote:
Hi all,
Please review this change:
JBS: https://bugs.openjdk.java.net/browse/JDK-8240725
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8240725/webrev.00/
We found the issue that HotSpot does not start when it is deployed
on the path which contains CJK character(s), and it has been fixed
in JDK-8240197.
On the review of JDK-8240197 [1], we concern similar issue might
occur in other place, and I found potentially problem in below:
- ZFILE_Open() @ zip_util.c
- JDK_Canonicalize() @ canonicalize_md.c (for Windows)
- create_unc_path() @ java_md.c (for Windows)
- Platform::MultibyteStringToWideString() @ WindowsPlatform.cpp
This change passed tests on submit repo
(mach5-one-ysuenaga-JDK-8240725-20200309-0811-9304139).
Thanks,
Yasumasa
[1]
https://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2020-March/038397.html
