RFR 9: 8155808: Process.getInputStream().skip() method is faulty

[Roger Riggs]

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
>>>
>>>
>>
>