[PATCH 7/8] packfile: fix short-circuiting of empty requests

From: Patrick Steinhardt <ps@pks.im>
Date: Dec 18, 2025, 06:28 UTC
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 <aplattner@nvidia.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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