All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Aaron Plattner <aplattner@nvidia.com>
Cc: git@vger.kernel.org,  Patrick Steinhardt <ps@pks.im>
Subject: Re: What's cooking in git.git (Dec 2025, #03)
Date: Thu, 18 Dec 2025 11:57:30 +0900	[thread overview]
Message-ID: <xmqqtsxoxzs5.fsf@gitster.g> (raw)
In-Reply-To: <f4ba7e89-4717-4b36-921f-56537131fd69@nvidia.com> (Aaron Plattner's message of "Wed, 17 Dec 2025 12:26:58 -0800")

Aaron Plattner <aplattner@nvidia.com> writes:

> I'm pretty sure the problem is when do_oid_object_info_extended() 
> substitutes the blank oi here:
>
> 	if (!oi)
> 		oi = &blank_oi;
>
> and then packfile_store_read_object_info() compares it to its own local 
> blank oi:
>
> 	static struct object_info blank_oi = OBJECT_INFO_INIT;

Ahh, that's an unusual mistake.

The following was done on top of 'seen', but would it help?  We
shouldn't have to use the stand-in "blank" thing to begin with.

Besides, explicitly handling the NULL case would reduce the
potential chance of errors that somebody accidentally writes into
blank_oi, making its contents dirty.




 object-file.c |  8 ++++----
 odb.c         | 29 +++++++++++++----------------
 packfile.c    |  3 +--
 3 files changed, 18 insertions(+), 22 deletions(-)

diff --git c/object-file.c w/object-file.c
index af1c3f972d..6280e42f34 100644
--- c/object-file.c
+++ w/object-file.c
@@ -426,7 +426,7 @@ int odb_source_loose_read_object_info(struct odb_source *source,
 	unsigned long size_scratch;
 	enum object_type type_scratch;
 
-	if (oi->delta_base_oid)
+	if (oi && oi->delta_base_oid)
 		oidclr(oi->delta_base_oid, source->odb->repo->hash_algo);
 
 	/*
@@ -437,13 +437,13 @@ int odb_source_loose_read_object_info(struct odb_source *source,
 	 * return value implicitly indicates whether the
 	 * object even exists.
 	 */
-	if (!oi->typep && !oi->sizep && !oi->contentp) {
+	if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
 		struct stat st;
-		if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
+		if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))
 			return quick_has_loose(source->loose, oid) ? 0 : -1;
 		if (stat_loose_object(source->loose, oid, &st, &path) < 0)
 			return -1;
-		if (oi->disk_sizep)
+		if (oi && oi->disk_sizep)
 			*oi->disk_sizep = st.st_size;
 		return 0;
 	}
diff --git c/odb.c w/odb.c
index 01a9d2e70f..8278ef39a0 100644
--- c/odb.c
+++ w/odb.c
@@ -680,34 +680,31 @@ static int do_oid_object_info_extended(struct object_database *odb,
 				       const struct object_id *oid,
 				       struct object_info *oi, unsigned flags)
 {
-	static struct object_info blank_oi = OBJECT_INFO_INIT;
 	const struct cached_object *co;
 	const struct object_id *real = oid;
 	int already_retried = 0;
 
-
 	if (flags & OBJECT_INFO_LOOKUP_REPLACE)
 		real = lookup_replace_object(odb->repo, oid);
 
 	if (is_null_oid(real))
 		return -1;
 
-	if (!oi)
-		oi = &blank_oi;
-
 	co = find_cached_object(odb, real);
 	if (co) {
-		if (oi->typep)
-			*(oi->typep) = co->type;
-		if (oi->sizep)
-			*(oi->sizep) = co->size;
-		if (oi->disk_sizep)
-			*(oi->disk_sizep) = 0;
-		if (oi->delta_base_oid)
-			oidclr(oi->delta_base_oid, odb->repo->hash_algo);
-		if (oi->contentp)
-			*oi->contentp = xmemdupz(co->buf, co->size);
-		oi->whence = OI_CACHED;
+		if (oi) {
+			if (oi->typep)
+				*(oi->typep) = co->type;
+			if (oi->sizep)
+				*(oi->sizep) = co->size;
+			if (oi->disk_sizep)
+				*(oi->disk_sizep) = 0;
+			if (oi->delta_base_oid)
+				oidclr(oi->delta_base_oid, odb->repo->hash_algo);
+			if (oi->contentp)
+				*oi->contentp = xmemdupz(co->buf, co->size);
+			oi->whence = OI_CACHED;
+		}
 		return 0;
 	}
 
diff --git c/packfile.c w/packfile.c
index ce6716fbea..3ffd6c7240 100644
--- c/packfile.c
+++ w/packfile.c
@@ -2132,7 +2132,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 rtype;
 
@@ -2143,7 +2142,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 (!oi)
 		return 0;
 
 	rtype = packed_object_info(store->source->odb->repo, e.p, e.offset, oi);



  reply	other threads:[~2025-12-18  2:57 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-12 10:26 What's cooking in git.git (Dec 2025, #03) Junio C Hamano
2025-12-13  7:42 ` Adrian Ratiu
2025-12-16  1:59   ` Junio C Hamano
2025-12-16  8:13     ` Patrick Steinhardt
2025-12-16 10:41       ` Adrian Ratiu
2025-12-16 17:49         ` Emily Shaffer
2025-12-17  5:04           ` Junio C Hamano
2025-12-13 18:45 ` René Scharfe
2025-12-17  0:09 ` Aaron Plattner
2025-12-17  5:02   ` Junio C Hamano
2025-12-17  5:20     ` Aaron Plattner
2025-12-17  6:18       ` Jeff King
2025-12-17 13:46       ` Junio C Hamano
2025-12-17 20:26 ` Aaron Plattner
2025-12-18  2:57   ` Junio C Hamano [this message]
2025-12-18  6:22     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqtsxoxzs5.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=aplattner@nvidia.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.