From: Patrick Steinhardt Date: Thu, 18 Dec 2025 06:28:17 GMT Subject: [PATCH 7/8] packfile: fix short-circuiting of empty requests Message-ID: <20251218-b4-pks-odb-read-object-info-improvements-v1-7-81c8368492be@pks.im> In-Reply-To: <20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im> When reading object information from the packfile store we have logic that tries to bail out early on empty requests. This is supposed to be a performance optimization so that we don't even have to unpack the object header stored in the packfile. This optimization doesn't work though: we compare the passed-in object info pointer with the pointer of an on-stack variable, which of course cannot ever become true. This issue was introduced via d9f517d051 (object-file: split out functions relating to object store subsystem, 2025-04-15): before this commit, we checked whether the passed-in object info was a `NULL` pointer, and if so, we set it to point to `blank_oi` instead. The commit then split up these the logic so that we continue to set up `blank_oi` in `do_oid_object_info_extended()`, but then do the check in `packfile_store_read_object_info()`. But even before that commit the logic was only partially working, as it could very well be that callers pass a blank object info themselves. Fix this bug by introducing a new `object_info_is_blank_request()` helper, which simply verifies that none of the contained request pointers are populated. Reported-by: Aaron Plattner Helped-by: Junio C Hamano Signed-off-by: Patrick Steinhardt --- odb.h | 10 ++++++++++ packfile.c | 3 +-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/odb.h b/odb.h index afae5e5c01..b869c054c1 100644 --- a/odb.h +++ b/odb.h @@ -353,6 +353,16 @@ struct object_info { } u; }; +/* + * Given an object info structure, figure out whether any of its request + * pointers are populated. + */ +static inline bool object_info_is_blank_request(struct object_info *oi) +{ + return !oi->typep && !oi->sizep && !oi->disk_sizep && + !oi->delta_base_oid && !oi->contentp; +} + /* * Initializer for a "struct object_info" that wants no items. You may * also memset() the memory to all-zeroes. diff --git a/packfile.c b/packfile.c index d2ae2432eb..ce83e77899 100644 --- a/packfile.c +++ b/packfile.c @@ -2157,7 +2157,6 @@ int packfile_store_read_object_info(struct packfile_store *store, struct object_info *oi, unsigned flags UNUSED) { - static struct object_info blank_oi = OBJECT_INFO_INIT; struct pack_entry e; int ret; @@ -2168,7 +2167,7 @@ int packfile_store_read_object_info(struct packfile_store *store, * We know that the caller doesn't actually need the * information below, so return early. */ - if (oi == &blank_oi) { + if (object_info_is_blank_request(oi)) { oi->whence = OI_PACKED; oi->u.packed.offset = e.offset; oi->u.packed.pack = e.p; -- 2.52.0.351.gbe84eed79e.dirty