@matheusbsbs / matheus-bagliano / commits / 0763671b8e
Fork 0

nth_packed_object_oid(): use customary integer return

Our nth_packed_object_sha1() function returns NULL for error. So when we wrapped it with nth_packed_object_oid(), we kept the same semantics. But it's a bit funny, because the caller actually passes in an out parameter, and the pointer we return is just that same struct they passed to us (or NULL). It's not too terrible, but it does make the interface a little non-idiomatic. Let's switch to our usual "0 for success, negative for error" return value. Most callers either don't check it, or are trivially converted. The one that requires the biggest change is actually improved, as we can ditch an extra aliased pointer variable. Since we are changing the interface in a subtle way that the compiler wouldn't catch, let's also change the name to catch any topics in flight. We can drop the 'o' and make it nth_packed_object_id(). That's slightly shorter, but also less redundant since the 'o' stands for "object" already. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 23, 2020 at 23:27 UTC 0763671b8e0b3ef873df13c741a911b809e6813d
6 files changed +22 -24
builtin/pack-objects.c
+2 -2
@@ -3053,7 +3053,7 @@ static void add_objects_in_unpacked_packs(void)
3053 in_pack.alloc);
3054
3055 for (i = 0; i < p->num_objects; i++) {
3056 - nth_packed_object_oid(&oid, p, i);
3056 + nth_packed_object_id(&oid, p, i);
3057 o = lookup_unknown_object(&oid);
3058 if (!(o->flags & OBJECT_ADDED))
3059 mark_in_pack_object(o, p, &in_pack);
@@ -3157,7 +3157,7 @@ static void loosen_unused_packed_objects(void)
3157 die(_("cannot open pack index"));
3158
3159 for (i = 0; i < p->num_objects; i++) {
3160 - nth_packed_object_oid(&oid, p, i);
3160 + nth_packed_object_id(&oid, p, i);
3161 if (!packlist_find(&to_pack, &oid) &&
3162 !has_sha1_pack_kept_or_nonlocal(&oid) &&
3163 !loosened_object_can_be_discarded(&oid, p->mtime))
midx.c
+1 -1
@@ -534,7 +534,7 @@ static void fill_pack_entry(uint32_t pack_int_id,
534 uint32_t cur_object,
535 struct pack_midx_entry *entry)
536 {
537 - if (!nth_packed_object_oid(&entry->oid, p, cur_object))
537 + if (nth_packed_object_id(&entry->oid, p, cur_object) < 0)
538 die(_("failed to locate object %d in packfile"), cur_object);
539
540 entry->pack_int_id = pack_int_id;
pack-bitmap.c
+2 -2
@@ -658,7 +658,7 @@ static void show_objects_for_type(
658 offset += ewah_bit_ctz64(word >> offset);
659
660 entry = &bitmap_git->pack->revindex[pos + offset];
661 - nth_packed_object_oid(&oid, bitmap_git->pack, entry->nr);
661 + nth_packed_object_id(&oid, bitmap_git->pack, entry->nr);
662
663 if (bitmap_git->hashes)
664 hash = get_be32(bitmap_git->hashes + entry->nr);
@@ -1136,7 +1136,7 @@ int rebuild_existing_bitmaps(struct bitmap_index *bitmap_git,
1136 struct object_entry *oe;
1137
1138 entry = &bitmap_git->pack->revindex[i];
1139 - nth_packed_object_oid(&oid, bitmap_git->pack, entry->nr);
1139 + nth_packed_object_id(&oid, bitmap_git->pack, entry->nr);
1140 oe = packlist_find(mapping, &oid);
1141
1142 if (oe)
packfile.c
+9 -9
@@ -1261,7 +1261,7 @@ static int retry_bad_packed_offset(struct repository *r,
1261 revidx = find_pack_revindex(p, obj_offset);
1262 if (!revidx)
1263 return OBJ_BAD;
1264 - nth_packed_object_oid(&oid, p, revidx->nr);
1264 + nth_packed_object_id(&oid, p, revidx->nr);
1265 mark_bad_packed_object(p, oid.hash);
1266 type = oid_object_info(r, &oid, NULL);
1267 if (type <= OBJ_NONE)
@@ -1693,7 +1693,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
1693 off_t len = revidx[1].offset - obj_offset;
1694 if (check_pack_crc(p, &w_curs, obj_offset, len, revidx->nr)) {
1695 struct object_id oid;
1696 - nth_packed_object_oid(&oid, p, revidx->nr);
1696 + nth_packed_object_id(&oid, p, revidx->nr);
1697 error("bad packed object CRC for %s",
1698 oid_to_hex(&oid));
1699 mark_bad_packed_object(p, oid.hash);
@@ -1782,7 +1782,7 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
1782 struct object_id base_oid;
1783 revidx = find_pack_revindex(p, obj_offset);
1784 if (revidx) {
1785 - nth_packed_object_oid(&base_oid, p, revidx->nr);
1785 + nth_packed_object_id(&base_oid, p, revidx->nr);
1786 error("failed to read delta base object %s"
1787 " at offset %"PRIuMAX" from %s",
1788 oid_to_hex(&base_oid), (uintmax_t)obj_offset,
@@ -1890,15 +1890,15 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
1890 }
1891 }
1892
1893 -const struct object_id *nth_packed_object_oid(struct object_id *oid,
1894 - struct packed_git *p,
1895 - uint32_t n)
1893 +int nth_packed_object_id(struct object_id *oid,
1894 + struct packed_git *p,
1895 + uint32_t n)
1896 {
1897 const unsigned char *hash = nth_packed_object_sha1(p, n);
1898 if (!hash)
1899 - return NULL;
1899 + return -1;
1900 hashcpy(oid->hash, hash);
1901 - return oid;
1901 + return 0;
1902 }
1903
1904 void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
@@ -2077,7 +2077,7 @@ int for_each_object_in_pack(struct packed_git *p,
2077 else
2078 pos = i;
2079
2080 - if (!nth_packed_object_oid(&oid, p, pos))
2080 + if (nth_packed_object_id(&oid, p, pos) < 0)
2081 return error("unable to get sha1 of object %u in %s",
2082 pos, p->pack_name);
2083
packfile.h
+2 -3
@@ -129,10 +129,9 @@ int bsearch_pack(const struct object_id *oid, const struct packed_git *p, uint32
129 const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
130 /*
131 * Like nth_packed_object_sha1, but write the data into the object specified by
132 - * the the first argument. Returns the first argument on success, and NULL on
133 - * error.
132 + * the the first argument. Returns 0 on success, negative otherwise.
133 */
135 -const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
134 +int nth_packed_object_id(struct object_id *, struct packed_git *, uint32_t n);
135
136 /*
137 * Return the offset of the nth object within the specified packfile.
sha1-name.c
+6 -7
@@ -155,7 +155,6 @@ static void unique_in_pack(struct packed_git *p,
155 struct disambiguate_state *ds)
156 {
157 uint32_t num, i, first = 0;
158 - const struct object_id *current = NULL;
158
159 if (p->multi_pack_index)
160 return;
@@ -173,10 +172,10 @@ static void unique_in_pack(struct packed_git *p,
172 */
173 for (i = first; i < num && !ds->ambiguous; i++) {
174 struct object_id oid;
176 - current = nth_packed_object_oid(&oid, p, i);
177 - if (!match_sha(ds->len, ds->bin_pfx.hash, current->hash))
175 + nth_packed_object_id(&oid, p, i);
176 + if (!match_sha(ds->len, ds->bin_pfx.hash, oid.hash))
177 break;
179 - update_candidates(ds, current);
178 + update_candidates(ds, &oid);
179 }
180 }
181
@@ -643,14 +642,14 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
642 */
643 mad->init_len = 0;
644 if (!match) {
646 - if (nth_packed_object_oid(&oid, p, first))
645 + if (!nth_packed_object_id(&oid, p, first))
646 extend_abbrev_len(&oid, mad);
647 } else if (first < num - 1) {
649 - if (nth_packed_object_oid(&oid, p, first + 1))
648 + if (!nth_packed_object_id(&oid, p, first + 1))
649 extend_abbrev_len(&oid, mad);
650 }
651 if (first > 0) {
653 - if (nth_packed_object_oid(&oid, p, first - 1))
652 + if (!nth_packed_object_id(&oid, p, first - 1))
653 extend_abbrev_len(&oid, mad);
654 }
655 mad->init_len = mad->cur_len;