Hi Chihiro,

Yes, I'll sponsor it.
Thank you for the update.

Thanks,
Serguei


On 3/8/20 06:05, Chihiro Ito wrote:
Hi,

I'm sorry. I included "JDK-" in the changeset title. I removed it and
updated it.

Change set : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.06/changeset

Regards,
Chihiro

2020年3月7日(土) 23:13 Chihiro Ito <chiroito...@gmail.com>:
Hi Serguei and Yasumasa,

I update the copyright year and created the change set.

Could you sponsor this, please?

Webrev : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.06/
Change set : http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.06/changeset

Regards,
Chihiro


2020年3月7日(土) 16:03 Yasumasa Suenaga <suen...@oss.nttdata.com>:


Hi Chihiro,

I'm also ok with webrev.05 after updating copyright year.


Yasumasa


On 2020/03/07 3:32, serguei.spit...@oracle.com wrote:
Hi Chichiro,

I'm okay with the fix.
Could you, please, update the copyright date in || 
src/java.base/share/classes/jdk/internal/vm/VMSupport.java before push?

Thanks,
Serguei


On 3/6/20 07:24, Chihiro Ito wrote:
Hi Serguei,

Could you review this again, please?

Regards,
Chihiro


2020年2月27日(木) 22:11 Chihiro Ito<chiroito...@gmail.com>:
Hi Ralf,

Thank you for your advice.

1.
The comment of serializePropertiesToByteArray in VMSupport is "The stream written to 
the byte array is ISO 8859-1 encoded.".
But the previous implementation does not keep this. I think we need to 
implement encode by ISO 8859-1.

2.
According to help, the feature of VM.system_properties is just "Print system 
properties". The users should not use this output for loading. The users use it when 
they want to see System Properties soon.

Regards,
Chihiro


2020年2月26日(水) 18:53 Schmelter, Ralf<ralf.schmel...@sap.com>:
Hi Chihiro,

I have two remarks:

1. ISO Latin 1 characters which are not ASCII will not work with the code. While the 
Properties.store() method claims to create ISO Latin 1 String, it really only will 
create printable ASCII characters  (apart from the comment, but it is ASCII too in this 
case). See Properties.saveConvert, where the char is checked for < 0x20 or > 0x7e 
and then printed as \uxxxx. This is important, since the bytes of the 
ByteArrayOutputStream are then send to the jcmd. And jcmd expects UTF-8 encoded strings, 
which is OK if we only used ASCII characters. But a ISO Latin 1 character >= 0x80 
will break the encoding. Just try using \u00DC in your test.

2. Your change makes it impossible to load the output with properties.load(). 
The old output could be loaded, since it was a valid properties file. But yours 
is not. For example, consider the filename c:\test\new. Formerly it would be 
encoded as:
C\:\\test\\new
And now it is:
C:\test\new
But the properties code would see "\n" as the newline character in your 
encoding. In fact you cannot differentiate between \n, \t, \f and \r originally being one 
or two characters.

Best regards,
Ralf


From: serviceability-dev<serviceability-dev-boun...@openjdk.java.net>  On 
Behalf Of Chihiro Ito
Sent: Dienstag, 25. Februar 2020 04:45
To:serguei.spit...@oracle.com
Cc:serviceability-dev@openjdk.java.net
Subject: Re: RFR: JDK-8222489: jcmd VM.system_properties gives unusable paths 
on Windows

Hi Serguei,

Thanks for your review and advice.

I modified these.
Could you review this again, please?

Webrev :http://cr.openjdk.java.net/~cito/JDK-8222489/webrev.05/

Regards,
Chihiro


Reply via email to