From b25562e63fe8afaf0f103362a4e672e9ccdc2d68 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 7 Jan 2023 08:48:55 -0500 Subject: [PATCH 1/6] object-file: inline calls to read_object() Since read_object() is these days just a thin wrapper around oid_object_info_extended(), and since it only has two callers, let's just inline those calls. This has a few positive outcomes: - it's a net reduction in source code lines - even though the callers end up with a few extra lines, they're now more flexible and can use object_info flags directly. So no more need to convert die_if_corrupt between parameter/flag, and we can ask for lookup replacement with a flag rather than doing it ourselves. - there's one fewer function in an already crowded namespace (e.g., the difference between read_object() and read_object_file() was not immediately obvious; now we only have one of them). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- object-file.c | 45 +++++++++++++++++---------------------------- object-store.h | 2 +- 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/object-file.c b/object-file.c index 80a0cd3b35..ed1babbac2 100644 --- a/object-file.c +++ b/object-file.c @@ -1671,23 +1671,6 @@ int oid_object_info(struct repository *r, return type; } -static void *read_object(struct repository *r, - const struct object_id *oid, enum object_type *type, - unsigned long *size, - int die_if_corrupt) -{ - struct object_info oi = OBJECT_INFO_INIT; - void *content; - oi.typep = type; - oi.sizep = size; - oi.contentp = &content; - - if (oid_object_info_extended(r, oid, &oi, die_if_corrupt - ? OBJECT_INFO_DIE_IF_CORRUPT : 0) < 0) - return NULL; - return content; -} - int pretend_object_file(void *buf, unsigned long len, enum object_type type, struct object_id *oid) { @@ -1709,8 +1692,8 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type, /* * This function dies on corrupt objects; the callers who want to - * deal with them should arrange to call read_object() and give error - * messages themselves. + * deal with them should arrange to call oid_object_info_extended() and give + * error messages themselves. */ void *read_object_file_extended(struct repository *r, const struct object_id *oid, @@ -1718,16 +1701,19 @@ void *read_object_file_extended(struct repository *r, unsigned long *size, int lookup_replace) { + struct object_info oi = OBJECT_INFO_INIT; + unsigned flags = OBJECT_INFO_DIE_IF_CORRUPT; void *data; - const struct object_id *repl = lookup_replace ? - lookup_replace_object(r, oid) : oid; - errno = 0; - data = read_object(r, repl, type, size, 1); - if (data) - return data; + oi.typep = type; + oi.sizep = size; + oi.contentp = &data; + if (lookup_replace) + flags |= OBJECT_INFO_LOOKUP_REPLACE; + if (oid_object_info_extended(r, oid, &oi, flags)) + return NULL; - return NULL; + return data; } void *read_object_with_reference(struct repository *r, @@ -2255,6 +2241,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime) { void *buf; unsigned long len; + struct object_info oi = OBJECT_INFO_INIT; enum object_type type; char hdr[MAX_HEADER_LEN]; int hdrlen; @@ -2262,8 +2249,10 @@ int force_object_loose(const struct object_id *oid, time_t mtime) if (has_loose_object(oid)) return 0; - buf = read_object(the_repository, oid, &type, &len, 0); - if (!buf) + oi.typep = &type; + oi.sizep = &len; + oi.contentp = &buf; + if (oid_object_info_extended(the_repository, oid, &oi, 0)) return error(_("cannot read object for %s"), oid_to_hex(oid)); hdrlen = format_object_header(hdr, sizeof(hdr), type, len); ret = write_loose_object(oid, hdr, hdrlen, buf, len, mtime, 0); diff --git a/object-store.h b/object-store.h index 98c1d67946..f0aa03bbb9 100644 --- a/object-store.h +++ b/object-store.h @@ -358,7 +358,7 @@ void assert_oid_type(const struct object_id *oid, enum object_type expect); /* * Enabling the object read lock allows multiple threads to safely call the * following functions in parallel: repo_read_object_file(), read_object_file(), - * read_object_file_extended(), read_object_with_reference(), read_object(), + * read_object_file_extended(), read_object_with_reference(), * oid_object_info() and oid_object_info_extended(). * * obj_read_lock() and obj_read_unlock() may also be used to protect other From 34728d7f30c1af6cf48a72b807796b719ab1c111 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 7 Jan 2023 08:49:15 -0500 Subject: [PATCH 2/6] streaming: inline call to read_object_file_extended() The open_istream_incore() function is the only direct user of read_object_file_extended(), and the only caller which unsets the lookup_replace flag. Since read_object_file_extended() is now just a thin wrapper around oid_object_info_extended(), let's inline the call. That will let us simplify read_object_file_extended() in the next patch. The inlined version here is a few more lines because of the query setup, but it's much more flexible, since we can pass (or omit) any flags we want. Note the updated comment in the istream struct definition. It was already slightly wrong (we never called read_object(); it has been read_object_file_extended() since day one), but should now be accurate. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- streaming.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/streaming.c b/streaming.c index 7b2f8b2b93..27841dc1d9 100644 --- a/streaming.c +++ b/streaming.c @@ -38,7 +38,7 @@ struct git_istream { union { struct { - char *buf; /* from read_object() */ + char *buf; /* from oid_object_info_extended() */ unsigned long read_ptr; } incore; @@ -388,12 +388,17 @@ static ssize_t read_istream_incore(struct git_istream *st, char *buf, size_t sz) static int open_istream_incore(struct git_istream *st, struct repository *r, const struct object_id *oid, enum object_type *type) { - st->u.incore.buf = read_object_file_extended(r, oid, type, &st->size, 0); + struct object_info oi = OBJECT_INFO_INIT; + st->u.incore.read_ptr = 0; st->close = close_istream_incore; st->read = read_istream_incore; - return st->u.incore.buf ? 0 : -1; + oi.typep = type; + oi.sizep = &st->size; + oi.contentp = (void **)&st->u.incore.buf; + return oid_object_info_extended(r, oid, &oi, + OBJECT_INFO_DIE_IF_CORRUPT); } /***************************************************************************** From 7be13f5f743978180ba377e12a312b773ed9af2b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 7 Jan 2023 08:50:19 -0500 Subject: [PATCH 3/6] read_object_file_extended(): drop lookup_replace option Our sole caller always passes in "1", so we can just drop the parameter entirely. Anybody who doesn't want this behavior could easily call oid_object_info_extended() themselves, as we're just a thin wrapper around it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- object-file.c | 7 ++----- object-store.h | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/object-file.c b/object-file.c index ed1babbac2..f472f2d6a0 100644 --- a/object-file.c +++ b/object-file.c @@ -1698,18 +1698,15 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type, void *read_object_file_extended(struct repository *r, const struct object_id *oid, enum object_type *type, - unsigned long *size, - int lookup_replace) + unsigned long *size) { struct object_info oi = OBJECT_INFO_INIT; - unsigned flags = OBJECT_INFO_DIE_IF_CORRUPT; + unsigned flags = OBJECT_INFO_DIE_IF_CORRUPT | OBJECT_INFO_LOOKUP_REPLACE; void *data; oi.typep = type; oi.sizep = size; oi.contentp = &data; - if (lookup_replace) - flags |= OBJECT_INFO_LOOKUP_REPLACE; if (oid_object_info_extended(r, oid, &oi, flags)) return NULL; diff --git a/object-store.h b/object-store.h index f0aa03bbb9..6ccacc947b 100644 --- a/object-store.h +++ b/object-store.h @@ -244,13 +244,13 @@ void *map_loose_object(struct repository *r, const struct object_id *oid, void *read_object_file_extended(struct repository *r, const struct object_id *oid, enum object_type *type, - unsigned long *size, int lookup_replace); + unsigned long *size); static inline void *repo_read_object_file(struct repository *r, const struct object_id *oid, enum object_type *type, unsigned long *size) { - return read_object_file_extended(r, oid, type, size, 1); + return read_object_file_extended(r, oid, type, size); } #ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS #define read_object_file(oid, type, size) repo_read_object_file(the_repository, oid, type, size) From 0ba05cf2e0d077bedbc1ee2521b3e5b5dc883250 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 7 Jan 2023 08:50:33 -0500 Subject: [PATCH 4/6] repo_read_object_file(): stop wrapping read_object_file_extended() The only caller of read_object_file_extended() is the thin wrapper of repo_read_object_file(). Instead of wrapping, let's just rename the inner function and let people call it directly. This cleans up the namespace and reduces confusion. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- object-file.c | 8 ++++---- object-store.h | 18 +++++------------- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/object-file.c b/object-file.c index f472f2d6a0..80b08fc389 100644 --- a/object-file.c +++ b/object-file.c @@ -1695,10 +1695,10 @@ int pretend_object_file(void *buf, unsigned long len, enum object_type type, * deal with them should arrange to call oid_object_info_extended() and give * error messages themselves. */ -void *read_object_file_extended(struct repository *r, - const struct object_id *oid, - enum object_type *type, - unsigned long *size) +void *repo_read_object_file(struct repository *r, + const struct object_id *oid, + enum object_type *type, + unsigned long *size) { struct object_info oi = OBJECT_INFO_INIT; unsigned flags = OBJECT_INFO_DIE_IF_CORRUPT | OBJECT_INFO_LOOKUP_REPLACE; diff --git a/object-store.h b/object-store.h index 6ccacc947b..1a713d89d7 100644 --- a/object-store.h +++ b/object-store.h @@ -241,17 +241,10 @@ const char *loose_object_path(struct repository *r, struct strbuf *buf, void *map_loose_object(struct repository *r, const struct object_id *oid, unsigned long *size); -void *read_object_file_extended(struct repository *r, - const struct object_id *oid, - enum object_type *type, - unsigned long *size); -static inline void *repo_read_object_file(struct repository *r, - const struct object_id *oid, - enum object_type *type, - unsigned long *size) -{ - return read_object_file_extended(r, oid, type, size); -} +void *repo_read_object_file(struct repository *r, + const struct object_id *oid, + enum object_type *type, + unsigned long *size); #ifndef NO_THE_REPOSITORY_COMPATIBILITY_MACROS #define read_object_file(oid, type, size) repo_read_object_file(the_repository, oid, type, size) #endif @@ -358,8 +351,7 @@ void assert_oid_type(const struct object_id *oid, enum object_type expect); /* * Enabling the object read lock allows multiple threads to safely call the * following functions in parallel: repo_read_object_file(), read_object_file(), - * read_object_file_extended(), read_object_with_reference(), - * oid_object_info() and oid_object_info_extended(). + * read_object_with_reference(), oid_object_info() and oid_object_info_extended(). * * obj_read_lock() and obj_read_unlock() may also be used to protect other * section which cannot execute in parallel with object reading. Since the used From c2f32bef9cbed13f21b2308cc3c02158c338f70f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 7 Jan 2023 08:50:53 -0500 Subject: [PATCH 5/6] packfile: inline custom read_object() When the pack code was split into its own file[1], it got a copy of the static read_object() function. But there's only one caller here, so we could just inline it. And it's worth doing so, as the name read_object() invites comparisons to the public read_object_file(), but the two don't behave quite the same. [1] The move happened over several commits, but the relevant one here is f1d8130be0 (pack: move clear_delta_base_cache(), packed_object_info(), unpack_entry(), 2017-08-18). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- packfile.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/packfile.c b/packfile.c index c0d7dd93f4..79e21ab18e 100644 --- a/packfile.c +++ b/packfile.c @@ -1650,22 +1650,6 @@ struct unpack_entry_stack_ent { unsigned long size; }; -static void *read_object(struct repository *r, - const struct object_id *oid, - enum object_type *type, - unsigned long *size) -{ - struct object_info oi = OBJECT_INFO_INIT; - void *content; - oi.typep = type; - oi.sizep = size; - oi.contentp = &content; - - if (oid_object_info_extended(r, oid, &oi, 0) < 0) - return NULL; - return content; -} - void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset, enum object_type *final_type, unsigned long *final_size) { @@ -1798,6 +1782,8 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset, uint32_t pos; struct object_id base_oid; if (!(offset_to_pack_pos(p, obj_offset, &pos))) { + struct object_info oi = OBJECT_INFO_INIT; + nth_packed_object_id(&base_oid, p, pack_pos_to_index(p, pos)); error("failed to read delta base object %s" @@ -1805,7 +1791,13 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset, oid_to_hex(&base_oid), (uintmax_t)obj_offset, p->pack_name); mark_bad_packed_object(p, &base_oid); - base = read_object(r, &base_oid, &type, &base_size); + + oi.typep = &type; + oi.sizep = &base_size; + oi.contentp = &base; + if (oid_object_info_extended(r, &base_oid, &oi, 0) < 0) + base = NULL; + external_base = base; } } From 15b63689a1698f91c6882bbf99f43226033d7cb1 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 12 Jan 2023 11:06:49 -0500 Subject: [PATCH 6/6] object-file: fix indent-with-space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit b25562e63f (object-file: inline calls to read_object(), 2023-01-07) accidentally indented a conditional block with spaces instead of a tab. Reported-by: Ævar Arnfjörð Bjarmason Signed-off-by: Jeff King Acked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- object-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/object-file.c b/object-file.c index 80b08fc389..ce9efae994 100644 --- a/object-file.c +++ b/object-file.c @@ -1708,7 +1708,7 @@ void *repo_read_object_file(struct repository *r, oi.sizep = size; oi.contentp = &data; if (oid_object_info_extended(r, oid, &oi, flags)) - return NULL; + return NULL; return data; }