From: Justin Tobler <jltobler@gmail.com>
Date: Tue, 09 Dec 2025 02:34:25 GMT
Subject: Re: [PATCH 3/8] odb: move computation of normalized objdir into `alt_odb_usable()`
Message-ID: <cqrno3lfvbfrb6ieestagbs5avshs7znoumky2plvtc4tjye2a@onwb5vmtstbx>
In-Reply-To: <20251208-b4-pks-odb-alternates-via-source-v1-3-e7ebb8b18c03@pks.im>

On 25/12/08 09:04AM, Patrick Steinhardt wrote:
> The function `alt_odb_usable()` receives as input the object database,
> the path it's supposed to determine usability for as well as the
> normalized path of the main object directory of the repository. The last
> part is derived by the function's caller from the object database. As we
> already pass the object database to `alt_odb_usable()` it is redundant
> information.
> 
> Drop the extra parameter and compute the normalized object directory in
> the function itself.
> 
> While at it, rename the function to `odb_is_source_usable()` to align it
> with modern terminology.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 27 +++++++++++++++------------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/odb.c b/odb.c
> index 3ffeece567..2513457a31 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -89,17 +89,20 @@ int odb_mkstemp(struct object_database *odb,
>  /*
>   * Return non-zero iff the path is usable as an alternate object database.

While we are here we could fix this typo: s/iff/if/

>   */
> -static int alt_odb_usable(struct object_database *o, const char *path,
> -			  const char *normalized_objdir)
> +static bool odb_is_source_usable(struct object_database *o, const char *path)
>  {
>  	int r;
> +	struct strbuf normalized_objdir = STRBUF_INIT;
> +	bool usable = false;
> +
> +	strbuf_realpath(&normalized_objdir, o->sources->path, 1);
>  
>  	/* Detect cases where alternate disappeared */
>  	if (!is_directory(path)) {
>  		error(_("object directory %s does not exist; "
>  			"check .git/objects/info/alternates"),
>  		      path);
> -		return 0;
> +		goto out;
>  	}
>  
>  	/*
> @@ -116,13 +119,17 @@ static int alt_odb_usable(struct object_database *o, const char *path,
>  		kh_value(o->source_by_path, p) = o->sources;
>  	}
>  
> -	if (fspatheq(path, normalized_objdir))
> -		return 0;
> +	if (fspatheq(path, normalized_objdir.buf))
> +		goto out;
>  
>  	if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))
> -		return 0;
> +		goto out;
> +
> +	usable = true;
>  
> -	return 1;
> +out:
> +	strbuf_release(&normalized_objdir);
> +	return usable;
>  }
>  
>  /*
> @@ -164,13 +171,10 @@ static struct odb_source *odb_add_source(struct object_database *odb,
>  					 int depth)
>  {
>  	struct odb_source *alternate = NULL;
> -	struct strbuf tmp = STRBUF_INIT;
>  	khiter_t pos;
>  	int ret;
>  
> -	strbuf_realpath(&tmp, odb->sources->path, 1);
> -
> -	if (!alt_odb_usable(odb, source, tmp.buf))
> +	if (!odb_is_source_usable(odb, source))

The normalized ODB path is only being used in alt_odb_usable() so
relocating it inside that function make sense. Looks good.

-Justin