Re: [PATCH JGIT] Method ignores results of InputStream.skip()

From: Shawn O. Pearce <spearce@spearce.org>
Date: Apr 27, 2009, 23:21 UTC
Message-ID: <20090427232112.GN23604@spearce.org>
In-Reply-To: <366BBB1215D0AB4B8A153AF047A2878002FCE7EA@dewdfe18.wdf.sap.corp>

"Sohn, Matthias" <matthias.sohn@sap.com> wrote:

> This method ignores the return value of java.io.InputStream.skip()

Doh.  In theory the skip should always succeed because the buffer
held the entire block we want to skip over due to the mark/reset
usage around this region.  But I agree, a skipFully() pattern is
better here.

Show 17 quoted lines

> @@ -139,12 +140,23 @@ private String readLine(final byte[] hdrbuf)
> throws IOException {
>  		while (lf < cnt && hdrbuf[lf] != '\n')
>  			lf++;
>  		bin.reset();
> -		bin.skip(lf);
> +		skipFully(bin, lf);
>  		if (lf < cnt && hdrbuf[lf] == '\n')
> -			bin.skip(1);
> +			skipFully(bin, 1);
>  		return RawParseUtils.decode(Constants.CHARSET, hdrbuf,
> 0, lf);
>  	}
>  
> +	// skip given number of bytes on InputStream respecting return
> value of InputStream.skip()
> +	static private void skipFully(InputStream in, long nBytes)

We already have this method; see NB.skipFully().

NB also has readFully() and a few other useful functions for dealing with common IO related patterns.

Please respin by calling NB.skipFully above rather than creating a new package level method, and fix the line wrapping issue so we can more easily apply it. :-)

-- 
Shawn.