On Sat, 23 Sep 2006, Junio C Hamano wrote:
> Nicolas Pitre <nico@xxxxxxx> 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.
Maybe. But since those are the minimum needed for base offset support
I'd prefer if the extra cleanup is made separate not to mix too many
issues at once.
> > 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);
> >
>
> ";;"?
Typo.
> > @@ -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?
Easy since the function is static.
> 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.
Right. The only entry point to those functions is sha1_object_info()
where I moved the use/unuse and all the other call instances are due to
recursion. > It seemed to me a bit wasteful to call use() so many times
to unuse() the same amount afterwards while only once is needed.
> 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.
Probably moving them near the actual pack content access.
May I assume you'll take care of the extra ; typo?
Nicolas
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html