Re: [PATCH 2/3] sha1_file: add the ability to parse objects in "pack file format"

From: Linus Torvalds <torvalds@osdl.org>
Date: Jul 12, 2006, 03:42 UTC
Message-ID: <Pine.LNX.4.64.0607112040050.5623@g5.osdl.org>
In-Reply-To: <7vveq31wgo.fsf@assigned-by-dhcp.cox.net>

On Tue, 11 Jul 2006, Junio C Hamano wrote:

Show 15 quoted lines

> >  	unsigned char *buf = xmalloc(1+size);
> > +	unsigned long n;
> >  
> > -	memcpy(buf, (char *) buffer + bytes, stream->total_out - bytes);
> > -	bytes = stream->total_out - bytes;
> > +	n = stream->total_out - bytes;
> > +	if (n > size)
> > +		n = size;
> > +	memcpy(buf, (char *) buffer + bytes, n);
> > +	bytes = n;
> >  	if (bytes < size) {
> >  		stream->next_out = buf + bytes;
> >  		stream->avail_out = size - bytes;
> 
> This one looks like an independent fix for a well spotted bug.

Yeah, well, the "bug" only happens if you screw something up (which I triggered both times I tried to rewrite this ;)

Or possibly it the object is corrupt.

But yes, it's independent.

Show 11 quoted lines

> > -	size = deflateBound(&stream, len+hdrlen);
> > +	size = 8 + deflateBound(&stream, len+hdrlen);
> >  	compressed = xmalloc(size);
> >  
> >  	/* Compress it */
> 
> I am wondring what this eight is.  You would pack 7 7-bit length
> plus 4-bit totalling 49+4 = 53-bit length (plus 4-bit type).  Is
> it an unwritten decision that the format would not deal with
> objects larger than 2^53 (which is probably fine but looks
> magic)?

8 was just "obviously enough".

The "hdrlen" part should already give us _way_ more padding than we need (the old-fashioned header will deflate to something bigger than the new header, so deflateBound() even _without_ the extra space should be plenty).

But I decided to add a few bytes just because it won't hurt.

		Linus