[PATCH v3 00/12] object-file API: pass object enums, tidy up streaming interface
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 4, 2022, 23:48 UTC
- Message-ID
- <cover-v3-00.12-00000000000-20220204T234435Z-avarab@gmail.com>
- In-Reply-To
- <cover-v2-00.11-00000000000-20220204T135005Z-avarab@gmail.com>
See the v2 for a summary and goals: https://lore.kernel.org/git/cover-v2-00.11-00000000000-20220204T135005Z-avarab@gmail.com/
This early re-roll addresses Junio's comments on it so far, i.e. the minor whitespace/typos, and then having the interface retain the "if (f() < 0)" interface.
The reason I'd changed it initially was because the callers used a mixture of "if (f())" and "if (f() < 0)", but let's just keep it as it is, with a couple of new patches to fix the existing callers to use "if (f() < 0)" consistently.
A corresponding re-roll to the topic that depends on this one isn't needed, as this part of the API wasn't used by it. For that topic see: https://lore.kernel.org/git/cover-v10-0.6-00000000000-20220204T135538Z-avarab@gmail.com/
Ævar Arnfjörð Bjarmason (12):
object-file.c: split up declaration of unrelated variables
object-file API: return "void", not "int" from hash_object_file()
object-file API: add a format_object_header() function
object-file API: have write_object_file() take "enum object_type"
object API: correct "buf" v.s. "map" mismatch in *.c and *.h
object API docs: move check_object_signature() docs to cache.h
object API users + docs: check <0, not !0 with
check_object_signature()
object-file API: split up and simplify check_object_signature()
object API: rename hash_object_file_literally() to write_*()
object-file API: have hash_object_file() take "enum object_type"
object-file.c: add a literal version of write_object_file_prepare()
object-file API: pass an enum to read_object_with_reference()apply.c | 12 ++-- builtin/cat-file.c | 11 +-- builtin/checkout.c | 2 +- builtin/fast-export.c | 2 +- builtin/fast-import.c | 12 ++-- builtin/grep.c | 4 +- builtin/hash-object.c | 2 +- builtin/index-pack.c | 10 ++- builtin/mktag.c | 9 ++- builtin/mktree.c | 2 +- builtin/notes.c | 2 +- builtin/pack-objects.c | 2 +- builtin/receive-pack.c | 2 +- builtin/replace.c | 4 +- builtin/tag.c | 2 +- builtin/unpack-objects.c | 8 +-- bulk-checkin.c | 4 +- cache-tree.c | 8 +-- cache.h | 20 +++++- commit.c | 2 +- convert.c | 2 +- diffcore-rename.c | 2 +- dir.c | 2 +- http-push.c | 2 +- log-tree.c | 2 +- match-trees.c | 2 +- merge-ort.c | 4 +- merge-recursive.c | 2 +- notes-cache.c | 2 +- notes.c | 8 +-- object-file.c | 145 +++++++++++++++++++++++++-------------- object-store.h | 24 ++++--- object.c | 5 +- pack-check.c | 9 ++- read-cache.c | 2 +- tree-walk.c | 6 +- 36 files changed, 202 insertions(+), 137 deletions(-)
Range-diff against v2:
-: ----------- > 1: 53334bc970a object-file.c: split up declaration of unrelated variables
-: ----------- > 2: 5ba49778ac1 object-file API: return "void", not "int" from hash_object_file()
-: ----------- > 3: 92fd020d199 object-file API: add a format_object_header() function
1: d259f901114 ! 4: 795ac3dea2a object-file API: have write_object_file() take "enum object_type"
@@ builtin/notes.c: static void prepare_note_data(const struct object_id *object, s
- if (write_object_file(d->buf.buf, d->buf.len, blob_type, oid)) {
+ if (write_object_file(d->buf.buf, d->buf.len, OBJ_BLOB, oid)) {
int status = die_message(_("unable to write note object"));
--
+
if (d->edit_path)
- die_message(_("the note contents have been left in %s"),
- d->edit_path);
## builtin/receive-pack.c ##
@@ builtin/receive-pack.c: static void prepare_push_cert_sha1(struct child_process *proc)
2: 207aec4eb64 ! 5: 6faf46277b5 object API: correct "buf" v.s. "map" mismatch in *.c and *.h
@@ Commit message
Change the name of the second argument to check_object_signature() to
be "buf" in object-file.c, making it consistent with the prototype in
- cache..h
+ cache.h
This fixes an inconsistency that's been with us since 2ade9340262 (Add
"check_sha1_signature()" helper function, 2005-04-08), and makes a
3: 636a647ac51 ! 6: bdddaa3648f object API: make check_object_signature() oideq()-like, move docs
@@ Metadata
Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
## Commit message ##
- object API: make check_object_signature() oideq()-like, move docs
+ object API docs: move check_object_signature() docs to cache.h
- Make the return value of check_object_signature() behave like oideq()
- and memcmp() instead of returning negative values on failure.
-
- This reduces the boilerplate required when calling the function, and
- makes the calling code behave the same is if though we'd called
- oideq(), which is basically what we're doing here. We already had some
- callers using "f() < 0", with others using "!f()". Instead of
- declaring the latter a bug let's convert all callers to it.
-
- It is unfortunate that there's also cases where we "return -1" on
- various errors, and we can't tell those apart from the expected OID
- being less than the real OID, but this was the case already.
-
- This change is rather dangerous stand-alone as we're changing the
- return semantics, but not changing the prototype. Therefore any
- out-of-tree code rebased on this change would start doing the opposite
- of what it was meant to do. In a subsequent commit we'll make that a
- non-issue by changing the signature of the function, ensuring that the
- compiler will catch any such misuse.
-
- While we're at it let's re-flow some of the callers to wrap at 79
- characters, and move the API documentation to cache.h, where the
- prototype of this function lives.
+ Move the API documentation for check_object_signature() to cache.h,
+ where its prototype is declared. This is in preparation for adding a
+ companion function.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- ## builtin/fast-export.c ##
-@@ builtin/fast-export.c: static void export_blob(const struct object_id *oid)
- buf = read_object_file(oid, &type, &size);
- if (!buf)
- die("could not read blob %s", oid_to_hex(oid));
-- if (check_object_signature(the_repository, oid, buf, size,
-- type_name(type), NULL) < 0)
-+ if (!check_object_signature(the_repository, oid, buf, size,
-+ type_name(type), NULL))
- die("oid mismatch in blob %s", oid_to_hex(oid));
- object = parse_object_buffer(the_repository, oid, type,
- size, buf, &eaten);
-
- ## builtin/index-pack.c ##
-@@ builtin/index-pack.c: static void fix_unresolved_deltas(struct hashfile *f)
- if (!data)
- continue;
-
-- if (check_object_signature(the_repository, &d->oid,
-- data, size,
-- type_name(type), NULL))
-+ if (!check_object_signature(the_repository, &d->oid, data,
-+ size, type_name(type), NULL))
- die(_("local object %s is corrupt"), oid_to_hex(&d->oid));
-
- /*
-
- ## builtin/mktag.c ##
-@@ builtin/mktag.c: static int verify_object_in_tag(struct object_id *tagged_oid, int *tagged_type)
- type_name(*tagged_type), type_name(type));
-
- repl = lookup_replace_object(the_repository, tagged_oid);
-- ret = check_object_signature(the_repository, repl,
-- buffer, size, type_name(*tagged_type),
-- NULL);
-+ ret = !check_object_signature(the_repository, repl, buffer, size,
-+ type_name(*tagged_type), NULL);
- free(buffer);
-
- return ret;
-
## cache.h ##
@@ cache.h: enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
struct object_info;
@@ cache.h: enum unpack_loose_header_result unpack_loose_header(git_zstream *stream
+ * object name actually matches "oid" to detect object corruption.
+ * With "buf" == NULL, try reading the object named with "oid" using
+ * the streaming interface and rehash it to do the same.
-+ *
-+ * Treat the return value like oideq() (which is like memcmp()),
-+ * except that negative values might also indicate a generic error.
+ */
int check_object_signature(struct repository *r, const struct object_id *oid,
void *buf, unsigned long size, const char *type,
@@ object-file.c: int format_object_header(char *str, size_t size, enum object_type
int check_object_signature(struct repository *r, const struct object_id *oid,
void *buf, unsigned long size, const char *type,
struct object_id *real_oidp)
-@@ object-file.c: int check_object_signature(struct repository *r, const struct object_id *oid,
-
- if (buf) {
- hash_object_file(r->hash_algo, buf, size, type, real_oid);
-- return !oideq(oid, real_oid) ? -1 : 0;
-+ return oideq(oid, real_oid);
- }
-
- st = open_istream(r, oid, &obj_type, &size, NULL);
-@@ object-file.c: int check_object_signature(struct repository *r, const struct object_id *oid,
- }
- r->hash_algo->final_oid_fn(real_oid, &c);
- close_istream(st);
-- return !oideq(oid, real_oid) ? -1 : 0;
-+ return oideq(oid, real_oid);
- }
-
- int git_open_cloexec(const char *name, int flags)
-@@ object-file.c: int read_loose_object(const char *path,
- git_inflate_end(&stream);
- goto out;
- }
-- if (check_object_signature(the_repository, expected_oid,
-- *contents, *size,
-- oi->type_name->buf, real_oid))
-+ if (!check_object_signature(the_repository, expected_oid,
-+ *contents, *size,
-+ oi->type_name->buf, real_oid))
- goto out;
- }
-
-
- ## object.c ##
-@@ object.c: struct object *parse_object(struct repository *r, const struct object_id *oid)
- if ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||
- (!obj && repo_has_object_file(r, oid) &&
- oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
-- if (check_object_signature(r, repl, NULL, 0, NULL, NULL) < 0) {
-+ if (!check_object_signature(r, repl, NULL, 0, NULL, NULL)) {
- error(_("hash mismatch %s"), oid_to_hex(oid));
- return NULL;
- }
-@@ object.c: struct object *parse_object(struct repository *r, const struct object_id *oid)
-
- buffer = repo_read_object_file(r, oid, &type, &size);
- if (buffer) {
-- if (check_object_signature(r, repl, buffer, size,
-- type_name(type), NULL) < 0) {
-+ if (!check_object_signature(r, repl, buffer, size,
-+ type_name(type), NULL)) {
- free(buffer);
- error(_("hash mismatch %s"), oid_to_hex(repl));
- return NULL;
-
- ## pack-check.c ##
-@@ pack-check.c: static int verify_packfile(struct repository *r,
- err = error("cannot unpack %s from %s at offset %"PRIuMAX"",
- oid_to_hex(&oid), p->pack_name,
- (uintmax_t)entries[i].offset);
-- else if (check_object_signature(r, &oid, data, size,
-- type_name(type), NULL))
-+ else if (!check_object_signature(r, &oid, data, size,
-+ type_name(type), NULL))
- err = error("packed %s from %s is corrupt",
- oid_to_hex(&oid), p->pack_name);
- else if (fn) {
-: ----------- > 7: 75abf75a437 object API users + docs: check <0, not !0 with check_object_signature()
4: c38af53e889 ! 8: 478d2915731 object-file API: split up and simplify check_object_signature()
@@ builtin/fast-export.c
@@ builtin/fast-export.c: static void export_blob(const struct object_id *oid)
if (!buf)
die("could not read blob %s", oid_to_hex(oid));
- if (!check_object_signature(the_repository, oid, buf, size,
-- type_name(type), NULL))
-+ type_name(type)))
+ if (check_object_signature(the_repository, oid, buf, size,
+- type_name(type), NULL) < 0)
++ type_name(type)) < 0)
die("oid mismatch in blob %s", oid_to_hex(oid));
object = parse_object_buffer(the_repository, oid, type,
size, buf, &eaten);
@@ builtin/index-pack.c
@@ builtin/index-pack.c: static void fix_unresolved_deltas(struct hashfile *f)
continue;
- if (!check_object_signature(the_repository, &d->oid, data,
-- size, type_name(type), NULL))
-+ size, type_name(type)))
+ if (check_object_signature(the_repository, &d->oid, data, size,
+- type_name(type), NULL) < 0)
++ type_name(type)) < 0)
die(_("local object %s is corrupt"), oid_to_hex(&d->oid));
/*
## builtin/mktag.c ##
@@ builtin/mktag.c: static int verify_object_in_tag(struct object_id *tagged_oid, int *tagged_type)
+ type_name(*tagged_type), type_name(type));
repl = lookup_replace_object(the_repository, tagged_oid);
- ret = !check_object_signature(the_repository, repl, buffer, size,
-- type_name(*tagged_type), NULL);
-+ type_name(*tagged_type));
+- ret = check_object_signature(the_repository, repl,
+- buffer, size, type_name(*tagged_type),
+- NULL);
++ ret = check_object_signature(the_repository, repl, buffer, size,
++ type_name(*tagged_type));
free(buffer);
return ret;
@@ cache.h: int parse_loose_header(const char *hdr, struct object_info *oi);
- * With "buf" == NULL, try reading the object named with "oid" using
- * the streaming interface and rehash it to do the same.
*
- * Treat the return value like oideq() (which is like memcmp()),
- * except that negative values might also indicate a generic error.
+ * A negative value indicates an error, usually that the OID is not
+ * what we expected, but it might also indicate another error.
*/
int check_object_signature(struct repository *r, const struct object_id *oid,
- void *buf, unsigned long size, const char *type,
@@ object-file.c: int format_object_header(char *str, size_t size, enum object_type
+
+ hash_object_file(r->hash_algo, buf, size, type, &real_oid);
+
-+ return oideq(oid, &real_oid);
++ return !oideq(oid, &real_oid) ? -1 : 0;
+}
+
+int stream_object_signature(struct repository *r, const struct object_id *oid)
@@ object-file.c: int format_object_header(char *str, size_t size, enum object_type
- if (buf) {
- hash_object_file(r->hash_algo, buf, size, type, real_oid);
-- return oideq(oid, real_oid);
+- return !oideq(oid, real_oid) ? -1 : 0;
- }
-
st = open_istream(r, oid, &obj_type, &size, NULL);
@@ object-file.c: int check_object_signature(struct repository *r, const struct obj
- r->hash_algo->final_oid_fn(real_oid, &c);
+ r->hash_algo->final_oid_fn(&real_oid, &c);
close_istream(st);
-- return oideq(oid, real_oid);
-+ return oideq(oid, &real_oid);
+- return !oideq(oid, real_oid) ? -1 : 0;
++ return !oideq(oid, &real_oid) ? -1 : 0;
}
int git_open_cloexec(const char *name, int flags)
@@ object-file.c: int read_loose_object(const char *path,
git_inflate_end(&stream);
goto out;
}
-- if (!check_object_signature(the_repository, expected_oid,
-- *contents, *size,
-- oi->type_name->buf, real_oid))
-+
+- if (check_object_signature(the_repository, expected_oid,
+- *contents, *size,
+- oi->type_name->buf, real_oid) < 0)
+ hash_object_file(the_repository->hash_algo,
+ *contents, *size, oi->type_name->buf,
+ real_oid);
@@ object.c: struct object *parse_object(struct repository *r, const struct object_
if ((obj && obj->type == OBJ_BLOB && repo_has_object_file(r, oid)) ||
(!obj && repo_has_object_file(r, oid) &&
oid_object_info(r, oid, NULL) == OBJ_BLOB)) {
-- if (!check_object_signature(r, repl, NULL, 0, NULL, NULL)) {
-+ if (!stream_object_signature(r, repl)) {
+- if (check_object_signature(r, repl, NULL, 0, NULL, NULL) < 0) {
++ if (stream_object_signature(r, repl) < 0) {
error(_("hash mismatch %s"), oid_to_hex(oid));
return NULL;
}
@@ object.c: struct object *parse_object(struct repository *r, const struct object_id *oid)
buffer = repo_read_object_file(r, oid, &type, &size);
if (buffer) {
- if (!check_object_signature(r, repl, buffer, size,
-- type_name(type), NULL)) {
-+ type_name(type))) {
+ if (check_object_signature(r, repl, buffer, size,
+- type_name(type), NULL) < 0) {
++ type_name(type)) < 0) {
free(buffer);
error(_("hash mismatch %s"), oid_to_hex(repl));
return NULL;
@@ pack-check.c: static int verify_packfile(struct repository *r,
err = error("cannot unpack %s from %s at offset %"PRIuMAX"",
oid_to_hex(&oid), p->pack_name,
(uintmax_t)entries[i].offset);
-- else if (!check_object_signature(r, &oid, data, size,
-- type_name(type), NULL))
-+ else if (data && !check_object_signature(r, &oid, data, size,
-+ type_name(type)))
+- else if (check_object_signature(r, &oid, data, size,
+- type_name(type), NULL) < 0)
++ else if (data && check_object_signature(r, &oid, data, size,
++ type_name(type)) < 0)
+ err = error("packed %s from %s is corrupt",
+ oid_to_hex(&oid), p->pack_name);
-+ else if (!data && !stream_object_signature(r, &oid))
++ else if (!data && stream_object_signature(r, &oid) < 0)
err = error("packed %s from %s is corrupt",
oid_to_hex(&oid), p->pack_name);
else if (fn) {
5: a5ebd04d462 = 9: 1276c2d1ed6 object API: rename hash_object_file_literally() to write_*()
6: 40647be525b ! 10: 2c936c674d4 object-file API: have hash_object_file() take "enum object_type"
@@ builtin/fast-export.c
@@ builtin/fast-export.c: static void export_blob(const struct object_id *oid)
if (!buf)
die("could not read blob %s", oid_to_hex(oid));
- if (!check_object_signature(the_repository, oid, buf, size,
-- type_name(type)))
-+ type))
+ if (check_object_signature(the_repository, oid, buf, size,
+- type_name(type)) < 0)
++ type) < 0)
die("oid mismatch in blob %s", oid_to_hex(oid));
object = parse_object_buffer(the_repository, oid, type,
size, buf, &eaten);
@@ builtin/index-pack.c: static struct base_data *resolve_delta(struct object_entry
@@ builtin/index-pack.c: static void fix_unresolved_deltas(struct hashfile *f)
continue;
- if (!check_object_signature(the_repository, &d->oid, data,
-- size, type_name(type)))
-+ size, type))
+ if (check_object_signature(the_repository, &d->oid, data, size,
+- type_name(type)) < 0)
++ type) < 0)
die(_("local object %s is corrupt"), oid_to_hex(&d->oid));
/*
@@ builtin/mktag.c
@@ builtin/mktag.c: static int verify_object_in_tag(struct object_id *tagged_oid, int *tagged_type)
repl = lookup_replace_object(the_repository, tagged_oid);
- ret = !check_object_signature(the_repository, repl, buffer, size,
-- type_name(*tagged_type));
+ ret = check_object_signature(the_repository, repl, buffer, size,
+- type_name(*tagged_type));
+ *tagged_type);
free(buffer);
@@ cache-tree.c: static int verify_one(struct repository *r,
## cache.h ##
@@ cache.h: int parse_loose_header(const char *hdr, struct object_info *oi);
- * except that negative values might also indicate a generic error.
+ * what we expected, but it might also indicate another error.
*/
int check_object_signature(struct repository *r, const struct object_id *oid,
- void *buf, unsigned long size, const char *type);
@@ object-file.c: int index_path(struct index_state *istate, struct object_id *oid,
rc = error(_("%s: failed to insert into database"), path);
strbuf_release(&sb);
@@ object-file.c: int read_loose_object(const char *path,
+ git_inflate_end(&stream);
goto out;
}
-
- hash_object_file(the_repository->hash_algo,
- *contents, *size, oi->type_name->buf,
- real_oid);
@@ object.c: struct object *parse_object(struct repository *r, const struct object_
buffer = repo_read_object_file(r, oid, &type, &size);
if (buffer) {
-- if (!check_object_signature(r, repl, buffer, size,
-- type_name(type))) {
-+ if (!check_object_signature(r, repl, buffer, size, type)) {
+- if (check_object_signature(r, repl, buffer, size,
+- type_name(type)) < 0) {
++ if (check_object_signature(r, repl, buffer, size, type) < 0) {
free(buffer);
error(_("hash mismatch %s"), oid_to_hex(repl));
return NULL;
@@ pack-check.c
@@ pack-check.c: static int verify_packfile(struct repository *r,
oid_to_hex(&oid), p->pack_name,
(uintmax_t)entries[i].offset);
- else if (data && !check_object_signature(r, &oid, data, size,
-- type_name(type)))
-+ type))
+ else if (data && check_object_signature(r, &oid, data, size,
+- type_name(type)) < 0)
++ type) < 0)
err = error("packed %s from %s is corrupt",
oid_to_hex(&oid), p->pack_name);
- else if (!data && !stream_object_signature(r, &oid))
+ else if (!data && stream_object_signature(r, &oid) < 0)
7: e39edfbce05 = 11: ad1a777f454 object-file.c: add a literal version of write_object_file_prepare()
8: f2b1cb861a0 = 12: d2d355e6bb8 object-file API: pass an enum to read_object_with_reference()-- 2.35.1.940.ge7a5b4b05f2