Thanks Sherman and Stuart for the review!

On 9/2/18 2:45 PM, Stuart Marks wrote:
Yes, the fix itself looks fine. Quite subtle, good catch.

But should this have a regression test? I can imagine somebody coming along later and "simplifying" (!(... > ...)) to (... <= ...) which would reintroduce the bug.

Yes, there is a regression test added:
http://cr.openjdk.java.net/~igerasim/8210285/00/webrev/test/jdk/java/nio/charset/CharsetDecoder/NaNinCtor.java.html

With kind regards,
Ivan

s'marks

On 9/2/18 11:14 AM, Xueming Shen wrote:
+1

On 8/31/18, 5:17 PM, Ivan Gerasimov wrote:
Hello!

The javadoc for CharsetDecoder [1] states that an exception is thrown when a non-positive number is passed in as an argument.

However we only reject negative or zero numbers, but not NaN.

And likewise for CharsetEncoder.

Would you please help review a trivial fix?

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8210285
WEBREV: http://cr.openjdk.java.net/~igerasim/8210285/00/webrev/

Thanks in advance!

[1] https://download.java.net/java/early_access/jdk11/docs/api/java.base/java/nio/charset/CharsetDecoder.html#%3Cinit%3E(java.nio.charset.Charset,float,float)




--
With kind regards,
Ivan Gerasimov

Reply via email to