On Tue, 13 Dec 2022 18:29:31 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> `Console` class now has a couple of internal subclasses within `java.io` 
>> package. It should be `sealed` and subclasses be declared in the `permits` 
>> clause. The implementation resided in `Console` class is separated into 
>> `ConsoleImpl` class.
>
> Naoto Sato has updated the pull request with a new target base due to a merge 
> or a rebase. The incremental webrev excludes the unrelated changes brought in 
> by the merge/rebase. The pull request contains four additional commits since 
> the last revision:
> 
>  - Making fields final, other clean-ups
>  - Merge branch 'master' into JDK-8298416-SealedConsole
>  - tidying up
>  - sealed Console, impl/if separation

Thank you Naoto for the changes, this looks fine to me.

src/java.base/share/classes/java/io/ConsoleImpl.java line 306:

> 304:                                 if (cbuf == rcb) {
> 305:                                     cbuf = grow();
> 306:                                     end = cbuf.length;

Initially this looked like an unintentionally change of logic to me, but when I 
see the current mainline code in IntelliJ, it does note that this assignment is 
then never used and it's right - after assigning `end` here, it never gets used 
and we ultimately end up with a `return` statement which is independent of this 
`end` value. So this change looks fine.

-------------

Marked as reviewed by jpai (Reviewer).

PR: https://git.openjdk.org/jdk/pull/11615

Reply via email to