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