Hi Daniel,
Webrev updated in place.
http://cr.openjdk.java.net/~rriggs/webrev-skip-8155808/index.html
On 6/17/2016 9:41 AM, Daniel Fuchs wrote:
Hi Roger,
Should PipeInputStream be final?
It is extended by DeferredCloseInputStream, used on Solaris, and is
private to the implementation
so being final is not useful.
One improvement to avoid two copies of the source code between Windows
and Linux is
to move PipeInputStream to a package private nested class of Process.
If so I guess you could remove the first part of the sentence in
the @exception clause (should that be @throws?):
655 * @exception IOException if the stream does not
support seek,
656 * or if some other I/O error
occurs.
The javadoc can be removed, none of the other methods have javadoc and
the class javadoc
explains the reason for the subclass.
since you do not use seek() - whether the stream supports it or
not will become irrelevant.
Right, seek is not exposed in the interface.
For stdin, the implementation is BufferedInputStream, for stderr, the
implementation is visible
and could be cast to FileInputStream and seek used. But that's all an
accident of implementation, not spec
so buyer beware.
Otherwise it looks reasonable to me.
The code was copied over from InputStream, right?
yes,
Thanks, Roger
best regards,
-- daniel
On 16/06/16 21:11, Roger Riggs wrote:
Ping?
On 6/13/2016 11:06 AM, Roger Riggs wrote:
A latent issue with Process InputStreams support for the skip method
was reported[1].
Though the InputStream returned is a BufferedInputStream, in some
cases,
the skip method calls FileInputStream.seek on the pipe containing
output
from the process but the pipes do not support seek. The proposed fix
is to override
the skip method to implement skip as reading the bytes instead of
invoking seek.
Please review and comment:
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-skip-8155808/index.html
Issue:
[1] https://bugs.openjdk.java.net/browse/JDK-8155808
Thanks, Roger
