Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Tue, 26 Oct 2021 12:59:11 GMT, Naoto Sato wrote: >> src/java.base/share/classes/java/io/Console.java line 590: >> >>> 588: if (cs == null) { >>> 589: cs = Charset.forName(StaticProperty.nativeEncoding(), >>> 590: Charset.defaultCharset()); >> >> I assume that `StaticProperty.nativeEncoding()` will never be `null`? >> Otherwise an IAE would be thrown here where previously >> `Charset.defaultCharset()` would be used. > > Yes. `StaticProperty.nativeEncoding()` caches the value to `native.encoding` > system property. The value is not optional, so it should be considered an > error if `StaticProperty.nativeEncoding()` returned `null`. > https://download.java.net/java/early_access/jdk18/docs/api/java.base/java/lang/System.html#native.encoding Thanks for the clarification. - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Tue, 26 Oct 2021 10:42:49 GMT, Daniel Fuchs wrote: >> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Reflecting review comments > > src/java.base/share/classes/java/io/Console.java line 590: > >> 588: if (cs == null) { >> 589: cs = Charset.forName(StaticProperty.nativeEncoding(), >> 590: Charset.defaultCharset()); > > I assume that `StaticProperty.nativeEncoding()` will never be `null`? > Otherwise an IAE would be thrown here where previously > `Charset.defaultCharset()` would be used. Yes. `StaticProperty.nativeEncoding()` caches the value to `native.encoding` system property. The value is not optional, so it should be considered an error if `StaticProperty.nativeEncoding()` returned `null`. https://download.java.net/java/early_access/jdk18/docs/api/java.base/java/lang/System.html#native.encoding - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments src/java.base/share/classes/java/io/Console.java line 590: > 588: if (cs == null) { > 589: cs = Charset.forName(StaticProperty.nativeEncoding(), > 590: Charset.defaultCharset()); I assume that `StaticProperty.nativeEncoding()` will never be `null`? Otherwise an IAE would be thrown here where previously `Charset.defaultCharset()` would be used. - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments Thank you for clarification. - Marked as reviewed by serb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
On Sat, 23 Oct 2021 22:13:35 GMT, Naoto Sato wrote: >> During the review of JEP 400, a proposal to provide an overloaded method to >> `Charset.forName()` was suggested >> [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This >> PR is to implement the proposal. A CSR is also drafted as >> https://bugs.openjdk.java.net/browse/JDK-8275348 > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Reflecting review comments Marked as reviewed by rriggs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6045
Re: RFR: 8270490: Charset.forName() taking fallback default value [v4]
> During the review of JEP 400, a proposal to provide an overloaded method to > `Charset.forName()` was suggested > [[1]](https://github.com/openjdk/jdk/pull/4733#discussion_r669693954). This > PR is to implement the proposal. A CSR is also drafted as > https://bugs.openjdk.java.net/browse/JDK-8275348 Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Reflecting review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/6045/files - new: https://git.openjdk.java.net/jdk/pull/6045/files/7c73c5ba..0e787e7a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6045&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6045&range=02-03 Stats: 7 lines in 2 files changed: 2 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/6045.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6045/head:pull/6045 PR: https://git.openjdk.java.net/jdk/pull/6045