Re: How to replace a single corrupt, packed object?

From: Shawn O. Pearce <spearce@spearce.org>
Date: Aug 11, 2008, 03:07 UTC
Message-ID: <20080811030744.GD27195@spearce.org>
In-Reply-To: <alpine.LFD.1.10.0808102146050.22892@xanadu.home>

Nicolas Pitre <nico@cam.org> wrote:

> OK, here's what the patch to allow repacking without -f and still using 
> redundant objects in presence of pack corruption might look like.  
> Please tell me if that works for you.

Aside from goto being considered harmful by some really smart people, this patch makes a lot of sense. Its only downside is a backwards goto within this function, but the code is actually still quite clear to me.

If this allows git to magically fix Dscho's bad pack, it may be worth including in the core tree.

Show 57 quoted lines

> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 2dadec1..88e73f3 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -277,6 +277,7 @@ static unsigned long write_object(struct sha1file *f,
>  				 */
>  
>  	if (!to_reuse) {
> +		no_reuse:
>  		if (!usable_delta) {
>  			buf = read_sha1_file(entry->idx.sha1, &type, &size);
>  			if (!buf)
> @@ -364,14 +365,28 @@ static unsigned long write_object(struct sha1file *f,
>  			reused_delta++;
>  		}
>  		hdrlen = encode_header(type, entry->size, header);
> +
>  		offset = entry->in_pack_offset;
>  		revidx = find_pack_revindex(p, offset);
>  		datalen = revidx[1].offset - offset;
>  		if (!pack_to_stdout && p->index_version > 1 &&
> -		    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr))
> -			die("bad packed object CRC for %s", sha1_to_hex(entry->idx.sha1));
> +		    check_pack_crc(p, &w_curs, offset, datalen, revidx->nr)) {
> +			error("bad packed object CRC for %s", sha1_to_hex(entry->idx.sha1));
> +			if (entry->delta)
> +				reused_delta--;
> +			goto no_reuse;
> +		}
> +
>  		offset += entry->in_pack_header_size;
>  		datalen -= entry->in_pack_header_size;
> +		if (!pack_to_stdout && p->index_version == 1 &&
> +		    check_pack_inflate(p, &w_curs, offset, datalen, entry->size)) {
> +			die("corrupt packed object for %s", sha1_to_hex(entry->idx.sha1));
> +			if (entry->delta)
> +				reused_delta--;
> +			goto no_reuse;
> +		}
> +
>  		if (type == OBJ_OFS_DELTA) {
>  			off_t ofs = entry->idx.offset - entry->delta->idx.offset;
>  			unsigned pos = sizeof(dheader) - 1;
> @@ -394,10 +409,6 @@ static unsigned long write_object(struct sha1file *f,
>  				return 0;
>  			sha1write(f, header, hdrlen);
>  		}
> -
> -		if (!pack_to_stdout && p->index_version == 1 &&
> -		    check_pack_inflate(p, &w_curs, offset, datalen, entry->size))
> -			die("corrupt packed object for %s", sha1_to_hex(entry->idx.sha1));
>  		copy_pack_data(f, p, &w_curs, offset, datalen);
>  		unuse_pack(&w_curs);
>  		reused++;
> 
> 
> Nicolas

-- 
Shawn.