Re: Infinite loop regression in git-fsck in v2.12.0
- From
Jeff King <peff@peff.net>
- Date
- Oct 30, 2018, 22:56 UTC
- Message-ID
- <20181030225603.GA5889@sigill.intra.peff.net>
- In-Reply-To
- <xmqq4ld3134f.fsf@gitster-ct.c.googlers.com>
On Wed, Oct 31, 2018 at 07:28:00AM +0900, Junio C Hamano wrote:
Show 21 quoted lines
> > So we need to distinguish those cases. I think this is the simplest fix:
> >
> > diff --git a/sha1-file.c b/sha1-file.c
> > index dd0b6aa873..a7ff5fe25d 100644
> > --- a/sha1-file.c
> > +++ b/sha1-file.c
> > @@ -2199,6 +2199,7 @@ static int check_stream_sha1(git_zstream *stream,
> > * see the comment in unpack_sha1_rest for details.
> > */
> > while (total_read <= size &&
> > + stream->avail_in > 0 &&
> > (status == Z_OK || status == Z_BUF_ERROR)) {
> > stream->next_out = buf;
> > stream->avail_out = sizeof(buf);
>
> Hmph. If the last round consumed the final input byte and needed
> output space of N bytes, but only M (< N) bytes of the output space
> was available, then it would have reduced both avail_in and
> avail_out down to zero and yielded Z_BUF_ERROR, no? Or would zlib
> refrain from consuming that final byte (leaving avail_in to at least
> one) and give us Z_BUF_ERROR in such a case?Hmm, yeah, good thinking. I think zlib could consume that final byte into its internal buffer.
As part of my digging, I looked at how the loose streaming code handles this. It checks that when we see Z_BUF_ERROR, we actually did run out of output bytes (so if we didn't, then we know it's not the case we expected to be looping on).
I have some patches almost ready to send; I'll use that technique.
-Peff