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

["Shawn O. Pearce" <spearce@spearce.org>]

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.

 
> 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.