RFR 8210285 : CharsetDecoder/Encoder's constructor does not reject NaN

Martin Buchholz martinrb at google.com
Mon Sep 3 15:31:40 UTC 2018


Looks good to me.  I would add a call to

new MyDecoder(ascii, 0.5f, 1.5f)

to make sure all calls to the constructor don't throw (because e.g. for
ASCII we know the correct values are 1.0f).

(In any case it feels like an API design mistake - the Charset itself
should be the source of truth about charsPerByte)

On Mon, Sep 3, 2018 at 3:57 AM, Alan Bateman <Alan.Bateman at oracle.com>
wrote:

> On 03/09/2018 05:54, Ivan Gerasimov wrote:
>
>> 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
>>
> The update to Charset-X-Coder.java.template looks okay. The changes also
> CharsetEncoder so we'll need test coverage for that too.
>
> -Alan
>


More information about the core-libs-dev mailing list