git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] odb: do not use "blank" substitute for NULL

From
Patrick Steinhardt <ps@pks.im>
Date
Dec 18, 2025, 08:50 UTC
Message-ID
<aUPASOMyxIJjYwTj@pks.im>
In-Reply-To
<xmqqpl8cxy0j.fsf@gitster.g>
On Thu, Dec 18, 2025 at 12:35:40PM +0900, Junio C Hamano wrote:
Show 30 quoted lines
> diff --git a/object-file.c b/object-file.c
> index 12177a7dd7..e0cce3a62a 100644
> --- a/object-file.c
> +++ b/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;
>  	}

Okay, here we know to exit early in case `oi == NULL`. So any subsequent code can assume that `oi` is non-NULL. Good.

Show 51 quoted lines
> diff --git a/odb.c b/odb.c
> index f4cbee4b04..85dc21b104 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -664,34 +664,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;
>  	}

Looks reasonable. We pass down `oi` to both the loose backend and the packfile store, but you teach both of them to handle this alright.

Show 11 quoted lines
> diff --git a/packfile.c b/packfile.c
> index 7a16aaa90d..2aa6135c3a 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -2106,7 +2105,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;
And this here is fixing the actual performance regression.

All of this looks as expected to me, so let's merge this patch down fastish. I'll rebase my bigger patch series at [1] on top of your patch.

Thanks!
Patrick
[1]: <20251218-b4-pks-odb-read-object-info-improvements-v1-0-81c8368492be@pks.im>
Previous: Junio C Hamano
Message 8 of 8 in “odb: do not use "blank" substitute for NULL”
  1. odb: do not use "blank" substitute for NULLJunio C Hamano, Dec 18, 2025
  2. Patrick SteinhardtDec 18, 2025
  3. Aaron PlattnerDec 18, 2025
  4. Kristoffer HaugsbakkDec 18, 2025
  5. Carlo Marcelo Arenas BelónDec 18, 2025
  6. Kristoffer HaugsbakkDec 19, 2025
  7. Junio C HamanoDec 19, 2025
  8. Patrick SteinhardtDec 18, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.