On Thu, 17 Oct 2024 11:14:17 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Brian Burkhalter has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - 8233451: Remove use of handleAvailable() (Windows)
>> - 8233451: Remove use of handleAvailable() (UNIX)
>
> src/java.base/share/classes/sun/nio/ch/ChannelInputStream.java line 56:
>
>> 54: private byte[] b1;
>> 55:
>> 56: private Boolean isOther = null;
>
> I don't particularly like using a Boolean for tri-states but it's not too bad
> here. No need to initialize to null. It could be Stable but probably not much
> benefit here all usages require file I/O that dominates.
>
> Are you going to add a comment to this field as readers might know now what
> "other" means? In the APIs we say "something other than a regular file,
> directory, or symbolic link" and maybe that could be useful here.
Fixed in a163d8d and 492659f.
> src/java.base/share/classes/sun/nio/ch/FileChannelImpl.java line 533:
>
>> 531: }
>> 532:
>> 533: public int available() throws IOException {
>
> This can be package-private. It would be useful to add a method description
> as FC doesn't define this method, same thing for isOther.
Fixed in a163d8d.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21508#discussion_r1805544190
PR Review Comment: https://git.openjdk.org/jdk/pull/21508#discussion_r1805544463
