Hi Ralf,

On 18/07/2019 10:24 pm, Schmelter, Ralf wrote:
Hi Severin,

thanks for the review.

If I read this right, then the method doesn't do what it claims it
does. It's printing to System.out unconditionally instead of to 'ps'.
What am I missing?

Nothing. The code should of course use ps and not System.out. I've updated the 
webrev:
http://cr.openjdk.java.net/~rschmelter/webrevs/8227868/webrev.1/

General approach seems okay. Using Arrays.copyOf is better than creating Strings just for the purposes of printing. Not sure why you chose a 8K buffer when existing codes uses 256 chars?

I'm not sure why additional tests would be needed, given that the existing ones 
already test the code.

There would ideally have been a regression test written for JDK-8222491 to demonstrate the conversion problem. Such a test would then be modified to extend to this bug. If it is not practical to try and produce a regression test to the demonstrate the problem then the bug reports should have the noreg-hard label added to them. (I doubt we have any current tests using non-ascii UTF8 :( ).

Thanks,
David
-----

Best regards,
Ralf

Reply via email to