On Thu, 24 Oct 2024 11:29:01 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> Modify `FileInputStream` (FIS) to fall back to the superclass
>> implementations of `readAllBytes()`, `readNBytes(int)`, `skip()`, and
>> `transferTo` when the input source would otherwise fail with "Illegal Seek"
>> in the FIS implementation, such as for the standard input or a named pipe.
>
> src/java.base/share/classes/java/io/FileInputStream.java line 334:
>
>> 332: @Override
>> 333: public byte[] readAllBytes() throws IOException {
>> 334: if (!canSeek())
>
> Since `canSeek()` is implemented as a native call whose result depends on the
> file descriptor of this `FileInputStream`, do you think we should reduce
> these native calls in this change and call `canSeek()` just once (and store
> that state) in the constructors of this class (of course, after the call to
> open())?
> If we do that, we might have to consider whether the
> `FileInputStream(FileDescriptor fdObj)` will end up throwing an exception
> (from the native canSeek() implementation) if the `FileDescriptor` passed to
> the constructor is not `valid()`
> https://docs.oracle.com/en/java/javase/23/docs/api/java.base/java/io/FileDescriptor.html#valid()
readAllBytes reads to EOF so there isn't any benefit to caching. I think the
main thing with this PR is whether canSeek is the right thing to use.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21673#discussion_r1815043405
