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 > >>>> > >
