packfile: use object_id in find_pack_entry_one()

The main function we use to search a pack index for an object is
find_pack_entry_one(). That function still takes a bare pointer to the
hash, despite the fact that its underlying bsearch_pack() function needs
an object_id struct. And so we end up making an extra copy of the hash
into the struct just to do a lookup.

As it turns out, all callers but one already have such an object_id. So
we can just take a pointer to that struct and use it directly. This
avoids the extra copy and provides a more type-safe interface.

The one exception is get_delta_base() in packfile.c, when we are chasing
a REF_DELTA from inside the pack (and thus we have a pointer directly to
the mmap'd pack memory, not a struct). We can just bump the hashcpy()
from inside find_pack_entry_one() to this one caller that needs it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Taylor Blau <me@ttaylorr.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0fc0680b40252ac8f5e3a8f008684df08d43ef36..080071426710da080464923e0671f0a220ba4ddf 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1556,7 +1556,7 @@ static int want_object_in_pack_one(struct packed_git *p,
        if (p == *found_pack)
                offset = *found_offset;
        else
-               offset = find_pack_entry_one(oid->hash, p);
+               offset = find_pack_entry_one(oid, p);
 
        if (offset) {
                if (!*found_pack) {
@@ -3984,7 +3984,7 @@ static int has_sha1_pack_kept_or_nonlocal(const struct object_id *oid)
        while (p) {
                if ((!p->pack_local || p->pack_keep ||
                                p->pack_keep_in_core) &&
-                       find_pack_entry_one(oid->hash, p)) {
+                       find_pack_entry_one(oid, p)) {
                        last_found = p;
                        return 1;
                }

diff --git a/connected.c b/connected.c
index 87cc4b57a17ce541b044950f937a11bfe39707ec..a9e2e139956f0c0daccb917200448d7d67a7dd6f 100644 (file)
--- a/connected.c
+++ b/connected.c
@@ -78,7 +78,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
                        for (p = get_all_packs(the_repository); p; p = p->next) {
                                if (!p->pack_promisor)
                                        continue;
-                               if (find_pack_entry_one(oid->hash, p))
+                               if (find_pack_entry_one(oid, p))
                                        goto promisor_pack_found;
                        }
                        /*
@@ -144,7 +144,7 @@ no_promisor_pack_found:
                 * are sure the ref is good and not sending it to
                 * rev-list for verification.
                 */
-               if (new_pack && find_pack_entry_one(oid->hash, new_pack))
+               if (new_pack && find_pack_entry_one(oid, new_pack))
                        continue;
 
                if (fprintf(rev_list_in, "%s\n", oid_to_hex(oid)) < 0)

diff --git a/midx.c b/midx.c
index 479812cb9bfddccd5091c9f3ba45ca382819755c..e82d4f2e65496b3e26c6e21c42e274406111aa3b 100644 (file)
--- a/midx.c
+++ b/midx.c
@@ -987,7 +987,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
                }
 
                m_offset = e.offset;
-               p_offset = find_pack_entry_one(oid.hash, e.p);
+               p_offset = find_pack_entry_one(&oid, e.p);
 
                if (m_offset != p_offset)
                        midx_report(_("incorrect object offset for oid[%d] = %s: %"PRIx64" != %"PRIx64),

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 32b222a7afce26082e308ca5e6c24a494e9ab5d1..4fa9dfc771a30545281b8a5e2b7d26dc5682c285 100644 (file)
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -935,7 +935,7 @@ static inline int bitmap_position_packfile(struct bitmap_index *bitmap_git,
                                           const struct object_id *oid)
 {
        uint32_t pos;
-       off_t offset = find_pack_entry_one(oid->hash, bitmap_git->pack);
+       off_t offset = find_pack_entry_one(oid, bitmap_git->pack);
        if (!offset)
                return -1;
 
@@ -1609,7 +1609,7 @@ static int in_bitmapped_pack(struct bitmap_index *bitmap_git,
                        if (bsearch_midx(&object->oid, bitmap_git->midx, NULL))
                                return 1;
                } else {
-                       if (find_pack_entry_one(object->oid.hash, bitmap_git->pack) > 0)
+                       if (find_pack_entry_one(&object->oid, bitmap_git->pack) > 0)
                                return 1;
                }
        }

diff --git a/packfile.c b/packfile.c
index c51eab15a57dc9a83441079de460ebaf8ec5a3a2..005ca670b4b495887e6b591208e0c88229a40af6 100644 (file)
--- a/packfile.c
+++ b/packfile.c
@@ -1239,7 +1239,9 @@ off_t get_delta_base(struct packed_git *p,
                *curpos += used;
        } else if (type == OBJ_REF_DELTA) {
                /* The base entry _must_ be in the same pack */
-               base_offset = find_pack_entry_one(base_info, p);
+               struct object_id oid;
+               hashcpy(oid.hash, base_info, the_repository->hash_algo);
+               base_offset = find_pack_entry_one(&oid, p);
                *curpos += the_hash_algo->rawsz;
        } else
                die("I am totally screwed");
@@ -1971,11 +1973,10 @@ off_t nth_packed_object_offset(const struct packed_git *p, uint32_t n)
        }
 }
 
-off_t find_pack_entry_one(const unsigned char *sha1,
-                                 struct packed_git *p)
+off_t find_pack_entry_one(const struct object_id *oid,
+                         struct packed_git *p)
 {
        const unsigned char *index = p->index_data;
-       struct object_id oid;
        uint32_t result;
 
        if (!index) {
@@ -1983,8 +1984,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
                        return 0;
        }
 
-       hashcpy(oid.hash, sha1, the_repository->hash_algo);
-       if (bsearch_pack(&oid, p, &result))
+       if (bsearch_pack(oid, p, &result))
                return nth_packed_object_offset(p, result);
        return 0;
 }
@@ -2016,7 +2016,7 @@ struct packed_git *find_oid_pack(const struct object_id *oid,
        struct packed_git *p;
 
        for (p = packs; p; p = p->next) {
-               if (find_pack_entry_one(oid->hash, p))
+               if (find_pack_entry_one(oid, p))
                        return p;
        }
        return NULL;
@@ -2033,7 +2033,7 @@ static int fill_pack_entry(const struct object_id *oid,
            oidset_contains(&p->bad_objects, oid))
                return 0;
 
-       offset = find_pack_entry_one(oid->hash, p);
+       offset = find_pack_entry_one(oid, p);
        if (!offset)
                return 0;
 

diff --git a/packfile.h b/packfile.h
index 3baffa940cf2ab65b7c68e8c72a297bdf76a7ad7..08f88a7ff521e9f66b1be44f279a76d1ee5331ec 100644 (file)
--- a/packfile.h
+++ b/packfile.h
@@ -154,10 +154,10 @@ int nth_packed_object_id(struct object_id *, struct packed_git *, uint32_t n);
 off_t nth_packed_object_offset(const struct packed_git *, uint32_t n);
 
 /*
- * If the object named sha1 is present in the specified packfile,
+ * If the object named by oid is present in the specified packfile,
  * return its offset within the packfile; otherwise, return 0.
  */
-off_t find_pack_entry_one(const unsigned char *sha1, struct packed_git *);
+off_t find_pack_entry_one(const struct object_id *oid, struct packed_git *);
 
 int is_pack_valid(struct packed_git *);
 void *unpack_entry(struct repository *r, struct packed_git *, off_t, enum object_type *, unsigned long *);

diff --git a/t/helper/test-find-pack.c b/t/helper/test-find-pack.c
index 14b2b0c12c0da366e78fe91acd9b0ef0045673b8..85a69a4e557694c41e042f10a461587a987de99e 100644 (file)
--- a/t/helper/test-find-pack.c
+++ b/t/helper/test-find-pack.c
@@ -40,7 +40,7 @@ int cmd__find_pack(int argc, const char **argv)
                die("cannot parse %s as an object name", argv[0]);
 
        for (p = get_all_packs(the_repository); p; p = p->next)
-               if (find_pack_entry_one(oid.hash, p)) {
+               if (find_pack_entry_one(&oid, p)) {
                        printf("%s\n", p->pack_name);
                        actual_count++;
                }