Re: [PATCH 2/6] many cleanups to sha1_file.c

From: Junio C Hamano <junkio@cox.net>
Date: Sep 23, 2006, 08:44 UTC
Message-ID: <7v7izvug3b.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0609210004550.2627@xanadu.home>

Nicolas Pitre <nico@cam.org> writes:

> Those cleanups are mainly to set the table for the support of deltas
> with base objects referenced by offsets instead of sha1.  This means
> that many pack lookup functions are converted to take a pack/offset
> tuple instead of a sha1.

There still remains some "struct pack_entry" and I wonder if it would make sense to get rid of them if only for consistency's sake.

Show 15 quoted lines

> diff --git a/sha1_file.c b/sha1_file.c
> index b89edb9..6fae766 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -926,8 +925,8 @@ static int packed_delta_info(unsigned ch
>  
>  		memset(&stream, 0, sizeof(stream));
>  
> -		data = stream.next_in = base_sha1 + 20;
> -		stream.avail_in = left - 20;
> +		data = stream.next_in = (unsigned char *) p->pack_base + offset;;
> +		stream.avail_in = p->pack_size - offset;
>  		stream.next_out = delta_head;
>  		stream.avail_out = sizeof(delta_head);
>

";;"?

Show 12 quoted lines

> @@ -989,75 +988,60 @@ int check_reuse_pack_delta(struct packed
>  ...
> +static int packed_object_info(struct packed_git *p, unsigned long offset,
>  			      char *type, unsigned long *sizep)
>  {
> +	unsigned long size;
>  	enum object_type kind;
>  
> -	if (use_packed_git(p))
> -		die("cannot map packed file");
> +	offset = unpack_object_header(p, offset, &kind, &size);
>

Do all callers of packed_object_info() call use_packed_git to make sure p is mapped? Ah sha1_object_info() is the only one except packed_delta_info() that calls itself and it protects it with use_packed_git(), so you are safe.

We would need to revamp use/unuse code when we implement the partial mapping anyway, but we still need to get this right for now.