object-file API: split up and simplify check_object_signature()

Split up the check_object_signature() function into that non-streaming version (it accepts an already filled "buf"), and a new stream_object_signature() which will retrieve the object from storage, and hash it on-the-fly. All of the callers of check_object_signature() were effectively calling two different functions, if we go by cyclomatic complexity. I.e. they'd either take the early "if (map)" branch and return early, or not. This has been the case since the "if (map)" condition was added in 090ea12671b (parse_object: avoid putting whole blob in core, 2012-03-07). We can then further simplify the resulting check_object_signature() function since only one caller wanted to pass a non-NULL "buf" and a non-NULL "real_oidp". That "read_loose_object()" codepath used by "git fsck" can instead use hash_object_file() followed by oideq(). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Feb 5, 2022 at 00:48 UTC 0f156dbb04b434d95ce5465e6b07d8869d55e8e0
7 files changed +38 -28
builtin/fast-export.c
+1 -1
@@ -300,7 +300,7 @@ static void export_blob(const struct object_id *oid)
300 if (!buf)
301 die("could not read blob %s", oid_to_hex(oid));
302 if (check_object_signature(the_repository, oid, buf, size,
303 - type_name(type), NULL) < 0)
303 + type_name(type)) < 0)
304 die("oid mismatch in blob %s", oid_to_hex(oid));
305 object = parse_object_buffer(the_repository, oid, type,
306 size, buf, &eaten);
builtin/index-pack.c
+1 -1
@@ -1413,7 +1413,7 @@ static void fix_unresolved_deltas(struct hashfile *f)
1413 continue;
1414
1415 if (check_object_signature(the_repository, &d->oid, data, size,
1416 - type_name(type), NULL) < 0)
1416 + type_name(type)) < 0)
1417 die(_("local object %s is corrupt"), oid_to_hex(&d->oid));
1418
1419 /*
builtin/mktag.c
+2 -3
@@ -61,9 +61,8 @@ static int verify_object_in_tag(struct object_id *tagged_oid, int *tagged_type)
61 type_name(*tagged_type), type_name(type));
62
63 repl = lookup_replace_object(the_repository, tagged_oid);
64 - ret = check_object_signature(the_repository, repl,
65 - buffer, size, type_name(*tagged_type),
66 - NULL);
64 + ret = check_object_signature(the_repository, repl, buffer, size,
65 + type_name(*tagged_type));
66 free(buffer);
67
68 return ret;
cache.h
+8 -4
@@ -1322,15 +1322,19 @@ int parse_loose_header(const char *hdr, struct object_info *oi);
1322 /**
1323 * With in-core object data in "buf", rehash it to make sure the
1324 * object name actually matches "oid" to detect object corruption.
1325 - * With "buf" == NULL, try reading the object named with "oid" using
1326 - * the streaming interface and rehash it to do the same.
1325 *
1326 * A negative value indicates an error, usually that the OID is not
1327 * what we expected, but it might also indicate another error.
1328 */
1329 int check_object_signature(struct repository *r, const struct object_id *oid,
1332 - void *buf, unsigned long size, const char *type,
1333 - struct object_id *real_oidp);
1330 + void *buf, unsigned long size, const char *type);
1331 +
1332 +/**
1333 + * A streaming version of check_object_signature().
1334 + * Try reading the object named with "oid" using
1335 + * the streaming interface and rehash it to do the same.
1336 + */
1337 +int stream_object_signature(struct repository *r, const struct object_id *oid);
1338
1339 int finalize_object_file(const char *tmpfile, const char *filename);
1340
object-file.c
+18 -14
@@ -1067,22 +1067,25 @@ int format_object_header(char *str, size_t size, enum object_type type,
1067 }
1068
1069 int check_object_signature(struct repository *r, const struct object_id *oid,
1070 - void *buf, unsigned long size, const char *type,
1071 - struct object_id *real_oidp)
1070 + void *buf, unsigned long size, const char *type)
1071 {
1073 - struct object_id tmp;
1074 - struct object_id *real_oid = real_oidp ? real_oidp : &tmp;
1072 + struct object_id real_oid;
1073 +
1074 + hash_object_file(r->hash_algo, buf, size, type, &real_oid);
1075 +
1076 + return !oideq(oid, &real_oid) ? -1 : 0;
1077 +}
1078 +
1079 +int stream_object_signature(struct repository *r, const struct object_id *oid)
1080 +{
1081 + struct object_id real_oid;
1082 + unsigned long size;
1083 enum object_type obj_type;
1084 struct git_istream *st;
1085 git_hash_ctx c;
1086 char hdr[MAX_HEADER_LEN];
1087 int hdrlen;
1088
1081 - if (buf) {
1082 - hash_object_file(r->hash_algo, buf, size, type, real_oid);
1083 - return !oideq(oid, real_oid) ? -1 : 0;
1084 - }
1085 -
1089 st = open_istream(r, oid, &obj_type, &size, NULL);
1090 if (!st)
1091 return -1;
@@ -1105,9 +1108,9 @@ int check_object_signature(struct repository *r, const struct object_id *oid,
1108 break;
1109 r->hash_algo->update_fn(&c, buf, readlen);
1110 }
1108 - r->hash_algo->final_oid_fn(real_oid, &c);
1111 + r->hash_algo->final_oid_fn(&real_oid, &c);
1112 close_istream(st);
1110 - return !oideq(oid, real_oid) ? -1 : 0;
1113 + return !oideq(oid, &real_oid) ? -1 : 0;
1114 }
1115
1116 int git_open_cloexec(const char *name, int flags)
@@ -2611,9 +2614,10 @@ int read_loose_object(const char *path,
2614 git_inflate_end(&stream);
2615 goto out;
2616 }
2614 - if (check_object_signature(the_repository, expected_oid,
2615 - *contents, *size,
2616 - oi->type_name->buf, real_oid) < 0)
2617 + hash_object_file(the_repository->hash_algo,
2618 + *contents, *size, oi->type_name->buf,
2619 + real_oid);
2620 + if (!oideq(expected_oid, real_oid))
2621 goto out;
2622 }
2623
object.c
+2 -2
@@ -279,7 +279,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
279 if ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||
280 (!obj && repo_has_object_file(r, oid) &&
281 oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
282 - if (check_object_signature(r, repl, NULL, 0, NULL, NULL) < 0) {
282 + if (stream_object_signature(r, repl) < 0) {
283 error(_("hash mismatch %s"), oid_to_hex(oid));
284 return NULL;
285 }
@@ -290,7 +290,7 @@ struct object *parse_object(struct repository *r, const struct object_id *oid)
290 buffer = repo_read_object_file(r, oid, &type, &size);
291 if (buffer) {
292 if (check_object_signature(r, repl, buffer, size,
293 - type_name(type), NULL) < 0) {
293 + type_name(type)) < 0) {
294 free(buffer);
295 error(_("hash mismatch %s"), oid_to_hex(repl));
296 return NULL;
pack-check.c
+6 -3
@@ -127,7 +127,7 @@ static int verify_packfile(struct repository *r,
127
128 if (type == OBJ_BLOB && big_file_threshold <= size) {
129 /*
130 - * Let check_object_signature() check it with
130 + * Let stream_object_signature() check it with
131 * the streaming interface; no point slurping
132 * the data in-core only to discard.
133 */
@@ -142,8 +142,11 @@ static int verify_packfile(struct repository *r,
142 err = error("cannot unpack %s from %s at offset %"PRIuMAX"",
143 oid_to_hex(&oid), p->pack_name,
144 (uintmax_t)entries[i].offset);
145 - else if (check_object_signature(r, &oid, data, size,
146 - type_name(type), NULL) < 0)
145 + else if (data && check_object_signature(r, &oid, data, size,
146 + type_name(type)) < 0)
147 + err = error("packed %s from %s is corrupt",
148 + oid_to_hex(&oid), p->pack_name);
149 + else if (!data && stream_object_signature(r, &oid) < 0)
150 err = error("packed %s from %s is corrupt",
151 oid_to_hex(&oid), p->pack_name);
152 else if (fn) {