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

[Ivan Gerasimov]

Thanks everyone for reviewing!

As suggested, I added a test to cover CharsetEncoder constructor and a 
sanity check that the constructor doesn't throw given valid arguments.

http://cr.openjdk.java.net/~igerasim/8210285/01/webrev/

With kind regards,

Ivan


On 9/3/18 8:31 AM, Martin Buchholz wrote:
> 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 
> <mailto: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
>         <http://cr.openjdk.java.net/%7Eigerasim/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
>
>

-- 
With kind regards,
Ivan Gerasimov