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