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

[Patrick Steinhardt <ps@pks.im>]

On Thu, Dec 18, 2025 at 12:35:40PM +0900, Junio C Hamano wrote:
> 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.

> 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.

> 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>