Re: [PATCH 3/8] odb: return newly created in-memory sources

[Toon Claes <toon@iotcl.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Callers have no trivial way to obtain the newly created object database
> source when adding it to the in-memory list of alternates. While not yet
> needed anywhere, a subsequent commit will want to obtain that pointer.
>
> Refactor the function to return the source to make it easily accessible.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  odb.c | 37 +++++++++++++++++++++----------------
>  odb.h |  4 ++--
>  2 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 61104b7cb8..7793816f81 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -139,23 +139,22 @@ static void read_info_alternates(struct object_database *odb,
>  				 const char *relative_base,
>  				 int depth);
>  
> -static int link_alt_odb_entry(struct object_database *odb,
> -			      const struct strbuf *entry,
> -			      const char *relative_base,
> -			      int depth,
> -			      const char *normalized_objdir)
> +static struct odb_source *link_alt_odb_entry(struct object_database *odb,
> +					     const char *entry,
> +					     const char *relative_base,
> +					     int depth,
> +					     const char *normalized_objdir)
>  {
> -	struct odb_source *alternate;
> +	struct odb_source *alternate = NULL;
>  	struct strbuf pathbuf = STRBUF_INIT;
>  	struct strbuf tmp = STRBUF_INIT;
>  	khiter_t pos;
> -	int ret = -1;
>  
> -	if (!is_absolute_path(entry->buf) && relative_base) {
> +	if (!is_absolute_path(entry) && relative_base) {
>  		strbuf_realpath(&pathbuf, relative_base, 1);
>  		strbuf_addch(&pathbuf, '/');
>  	}
> -	strbuf_addbuf(&pathbuf, entry);
> +	strbuf_addstr(&pathbuf, entry);
>  
>  	if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
>  		error(_("unable to normalize alternate object path: %s"),
> @@ -189,11 +188,11 @@ static int link_alt_odb_entry(struct object_database *odb,
>  
>  	/* recursively add alternates */
>  	read_info_alternates(odb, alternate->path, depth + 1);
> -	ret = 0;
> +
>   error:
>  	strbuf_release(&tmp);
>  	strbuf_release(&pathbuf);
> -	return ret;
> +	return alternate;
>  }
>  
>  static const char *parse_alt_odb_entry(const char *string,
> @@ -246,7 +245,7 @@ static void link_alt_odb_entries(struct object_database *odb, const char *alt,
>  		alt = parse_alt_odb_entry(alt, sep, &entry);
>  		if (!entry.len)
>  			continue;
> -		link_alt_odb_entry(odb, &entry,
> +		link_alt_odb_entry(odb, entry.buf,
>  				   relative_base, depth, objdirbuf.buf);
>  	}
>  	strbuf_release(&entry);
> @@ -316,17 +315,23 @@ void odb_add_to_alternates_file(struct object_database *odb,
>  	free(alts);
>  }
>  
> -void odb_add_to_alternates_memory(struct object_database *odb,
> -				  const char *reference)
> +struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
> +						const char *reference)
>  {
> +	struct odb_source *alternate;
> +	char *objdir;
> +
>  	/*
>  	 * Make sure alternates are initialized, or else our entry may be
>  	 * overwritten when they are.
>  	 */
>  	odb_prepare_alternates(odb);
>  
> -	link_alt_odb_entries(odb, reference,
> -			     '\n', NULL, 0);
> +	objdir = real_pathdup(odb->sources->path, 1);
> +	alternate = link_alt_odb_entry(odb, reference, NULL, 0, objdir);

If I understand correctly, instead of using real_pathdup() we could
instead call:

    alternate = link_alt_odb_entry(odb, reference, "/", 0, odb->sources->path);

I did not test this, but it would avoid duplicating the path here. I'm
not sure though whether it's easier to read.

> +
> +	free(objdir);
> +	return alternate;
>  }
>  
>  struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
> diff --git a/odb.h b/odb.h
> index 387b117c87..ac7ee05188 100644
> --- a/odb.h
> +++ b/odb.h
> @@ -265,8 +265,8 @@ void odb_add_to_alternates_file(struct object_database *odb,
>   * recursive alternates it points to), but do not modify the on-disk alternates
>   * file.
>   */
> -void odb_add_to_alternates_memory(struct object_database *odb,
> -				  const char *dir);
> +struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
> +						const char *dir);
>  
>  /*
>   * Read an object from the database. Returns the object data and assigns object
>
> -- 
> 2.50.1.619.g074bbf1d35.dirty
>
>

-- 
Cheers,
Toon