6516099: InputStream.skipFully(int k) to skip exactly k bytes

[Daniel Fuchs]

Hi Brian,

I like this version better :-)

But I wonder if skip(n) returning a negative value
(ns < 0) when n has been asserted to be > 0 should be
considered as an error, and if an IOException should
be thrown?

I mean - in which cases can a call to skip(n) returning
a negative value for a positive argument could actually
means that the implementation has rewinded?
Some kind of "looping" InputStream?

Wouldn't it be better to throw an IOException in this case,
thus forcing subclasses that behave in this way to override
skipFully as well?

I am concerned that trying to handle negative values when
we don't know what the logic in the subclass actually is
and what it means by it could hide some underlying issues
and introduce subtle bugs that would be hard to diagnose.

That said - I'm not opposing the change - just wanting
to make sure that all options have been duly considered.

best regards,

-- daniel

On 09/11/2018 22:04, Brian Burkhalter wrote:
> Hi Roger,
> 
>> On Nov 9, 2018, at 11:48 AM, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>
>> Thanks for taking the suggestions.
> 
> Suggestions always welcome!
> 
>> I think some of the statements should be normative for all implementations and
>> we're trying too hard to work with a possibly broken skip() implementation.
>>
>> I would recommend that if skip does not do what is expected, then
>> an IOException is immediately thrown.  There's no need for a second attempt.
>> That will encourage correction of badly written skip() or overrides of skipNbytes().
>>
>> Attached is a patch on top of your webrev with my attempt at
>> separating normative text and reducing the complexity in the @implSpec.
> 
> I like the verbiage the way you modified it. An updated patch is at
> 
> http://cr.openjdk.java.net/~bpb/6516099/webrev.06/ <http://cr.openjdk.java.net/~bpb/6516099/webrev.06/>
> 
> including a revision of the implementation to align with the words. The tests are not updated yet.
> 
> Thanks,
> 
> Brian
>