Re: [PATCH v2 2/4] sha1_file: move delta base cache code up

[Junio C Hamano]

Jonathan Tan  writes:

> In a subsequent patch, packed_object_info() will be modified to use the
> delta base cache, so move the relevant code to before
> packed_object_info().
>
> Signed-off-by: Jonathan Tan 
> ---
>  sha1_file.c | 226 
> +++-
>  1 file changed, 116 insertions(+), 110 deletions(-)

Hmph, is this meant to be just moving two whole functions?

> diff --git a/sha1_file.c b/sha1_file.c
> index a52b27541..a158907d1 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2239,116 +2239,6 @@ static enum ...
> ...
> -int packed_object_info(struct packed_git *p, off_t obj_offset,
> -struct object_info *oi)
> -{
> -...
> - if (oi->delta_base_sha1) {
> - if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> - const unsigned char *base;
> -
> - base = get_delta_base_sha1(p, _curs, curpos,
> -type, obj_offset);
> - if (!base) {
> - type = OBJ_BAD;
> - goto out;
> - }
> -
> - hashcpy(oi->delta_base_sha1, base);
> - } else
> - hashclr(oi->delta_base_sha1);
> - }
> -
> -out:
> - unuse_pack(_curs);
> - return type;
> -}
> -...

The above is what was removed, while ...

> @@ -2486,6 +2376,122 @@ static void ...
> ...
> +int packed_object_info(struct packed_git *p, off_t obj_offset,
> +struct object_info *oi)
> +{
> +...
> + if (oi->delta_base_sha1) {
> + if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> + const unsigned char *base;
> +
> + base = get_delta_base_sha1(p, _curs, curpos,
> +type, obj_offset);
> + if (!base) {
> + type = OBJ_BAD;
> + goto out;
> + }
> +
> + hashcpy(oi->delta_base_sha1, base);
> + } else
> + hashclr(oi->delta_base_sha1);
> + }
> +
> + oi->whence = OI_PACKED;
> + oi->u.packed.offset = obj_offset;
> + oi->u.packed.pack = p;
> + oi->u.packed.is_delta = (type == OBJ_REF_DELTA ||
> +  type == OBJ_OFS_DELTA);
> +
> +out:
> + unuse_pack(_curs);
> + return type;
> +}

... we somehow gained code to update *oi that used to be (and still
is) done by its sole caller, sha1_object_info_extended().

Perhaps this is a rebase-gotcha?

[PATCH v2 2/4] sha1_file: move delta base cache code up

[Jonathan Tan]

In a subsequent patch, packed_object_info() will be modified to use the
delta base cache, so move the relevant code to before
packed_object_info().

Signed-off-by: Jonathan Tan 
---
 sha1_file.c | 226 +++-
 1 file changed, 116 insertions(+), 110 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index a52b27541..a158907d1 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2239,116 +2239,6 @@ static enum object_type packed_to_object_type(struct 
packed_git *p,
goto out;
 }
 
-int packed_object_info(struct packed_git *p, off_t obj_offset,
-  struct object_info *oi)
-{
-   struct pack_window *w_curs = NULL;
-   unsigned long size;
-   off_t curpos = obj_offset;
-   enum object_type type;
-
-   /*
-* We always get the representation type, but only convert it to
-* a "real" type later if the caller is interested.
-*/
-   type = unpack_object_header(p, _curs, , );
-
-   if (oi->sizep) {
-   if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
-   off_t tmp_pos = curpos;
-   off_t base_offset = get_delta_base(p, _curs, _pos,
-  type, obj_offset);
-   if (!base_offset) {
-   type = OBJ_BAD;
-   goto out;
-   }
-   *oi->sizep = get_size_from_delta(p, _curs, tmp_pos);
-   if (*oi->sizep == 0) {
-   type = OBJ_BAD;
-   goto out;
-   }
-   } else {
-   *oi->sizep = size;
-   }
-   }
-
-   if (oi->disk_sizep) {
-   struct revindex_entry *revidx = find_pack_revindex(p, 
obj_offset);
-   *oi->disk_sizep = revidx[1].offset - obj_offset;
-   }
-
-   if (oi->typep || oi->typename) {
-   enum object_type ptot;
-   ptot = packed_to_object_type(p, obj_offset, type, _curs,
-curpos);
-   if (oi->typep)
-   *oi->typep = ptot;
-   if (oi->typename) {
-   const char *tn = typename(ptot);
-   if (tn)
-   strbuf_addstr(oi->typename, tn);
-   }
-   if (ptot < 0) {
-   type = OBJ_BAD;
-   goto out;
-   }
-   }
-
-   if (oi->delta_base_sha1) {
-   if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
-   const unsigned char *base;
-
-   base = get_delta_base_sha1(p, _curs, curpos,
-  type, obj_offset);
-   if (!base) {
-   type = OBJ_BAD;
-   goto out;
-   }
-
-   hashcpy(oi->delta_base_sha1, base);
-   } else
-   hashclr(oi->delta_base_sha1);
-   }
-
-out:
-   unuse_pack(_curs);
-   return type;
-}
-
-static void *unpack_compressed_entry(struct packed_git *p,
-   struct pack_window **w_curs,
-   off_t curpos,
-   unsigned long size)
-{
-   int st;
-   git_zstream stream;
-   unsigned char *buffer, *in;
-
-   buffer = xmallocz_gently(size);
-   if (!buffer)
-   return NULL;
-   memset(, 0, sizeof(stream));
-   stream.next_out = buffer;
-   stream.avail_out = size + 1;
-
-   git_inflate_init();
-   do {
-   in = use_pack(p, w_curs, curpos, _in);
-   stream.next_in = in;
-   st = git_inflate(, Z_FINISH);
-   if (!stream.avail_out)
-   break; /* the payload is larger than it should be */
-   curpos += stream.next_in - in;
-   } while (st == Z_OK || st == Z_BUF_ERROR);
-   git_inflate_end();
-   if ((st != Z_STREAM_END) || stream.total_out != size) {
-   free(buffer);
-   return NULL;
-   }
-
-   return buffer;
-}
-
 static struct hashmap delta_base_cache;
 static size_t delta_base_cached;
 
@@ -2486,6 +2376,122 @@ static void add_delta_base_cache(struct packed_git *p, 
off_t base_offset,
hashmap_add(_base_cache, ent);
 }
 
+int packed_object_info(struct packed_git *p, off_t obj_offset,
+  struct object_info *oi)
+{
+   struct pack_window *w_curs = NULL;
+   unsigned long size;
+   off_t curpos = obj_offset;
+   enum object_type type;
+
+   /*
+* We always get the representation type, but only convert it to
+* a "real" type later if the