Re: [PATCH 09/16] worktree: return allocated string from `get_worktree_git_dir()`

> The `get_worktree_git_dir()` function returns a string constant that
> does not need to be free'd by the caller. For `repo_get_git_dir()` and
> `repo_get_common_dir()` this is because we return strings owned by
> `the_repository`.
>

> But for `git_common_path()` it's a bit less obvious though, because that
> function does end up allocating memory. This doesn't result in a memory
> leak either because we write into a buffer returned by `get_pathname()`,
> which returns one out of four static buffers.
>

>
> We're about to drop `git_common_path()` in favor of `repo_common_path()`,
> which doesn't use the same mechanism but instead returns an allocated
> string owned by the caller. While we could adapt `get_worktree_git_dir()`
> to also use `get_pathname()` and print the derived common path into that
> buffer, the whole schema feels a lot like premature optimization in this
> context. There are some callsites where we call `get_worktree_git_dir()`
> in a loop that iterates through all worktrees. But none of these loops
> seem to be even remotely in the hot path, so saving a single allocation
> there does not feel worth it.
>
> Refactor the function to instead consistently return an allocated path
> so that we can start using `repo_common_path()` in a subsequent commit.
>

> diff --git a/revision.c b/revision.c
> index 474fa1e767..be72f226f3 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1874,15 +1874,20 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
>  	for (p = worktrees; *p; p++) {
>  		struct worktree *wt = *p;
>  		struct index_state istate = INDEX_STATE_INIT(revs->repo);
> +		char *gitdir;
>

>  		if (wt->is_current)
>  			continue; /* current index already taken care of */
>
> +		gitdir = get_worktree_git_dir(wt);
> +
>  		if (read_index_from(&istate,
>  				    worktree_git_path(the_repository, wt, "index"),
> -				    get_worktree_git_dir(wt)) > 0)
> +				    gitdir) > 0)
>  			do_add_index_objects_to_pending(revs, &istate, flags);
> +
>  		discard_index(&istate);
> +		free(gitdir);
>  	}
>  	free_worktrees(worktrees);
>  }

> diff --git a/worktree.c b/worktree.c
> index 8f4fc10c44..3b94535963 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -59,8 +59,9 @@ static void add_head_info(struct worktree *wt)
>  static int is_current_worktree(struct worktree *wt)
>  {
>  	char *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));
> -	const char *wt_git_dir = get_worktree_git_dir(wt);
> +	char *wt_git_dir = get_worktree_git_dir(wt);

>  	int is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));
> +	free(wt_git_dir);
>  	free(git_dir);
>  	return is_current;
>  }
> @@ -175,14 +176,14 @@ struct worktree **get_worktrees(void)
>  	return get_worktrees_internal(0);
>  }
>
> -const char *get_worktree_git_dir(const struct worktree *wt)
> +char *get_worktree_git_dir(const struct worktree *wt)
>  {
>  	if (!wt)
> -		return repo_get_git_dir(the_repository);
> +		return xstrdup(repo_get_git_dir(the_repository));
>  	else if (!wt->id)
> -		return repo_get_common_dir(the_repository);
> +		return xstrdup(repo_get_common_dir(the_repository));
>  	else
> -		return git_common_path("worktrees/%s", wt->id);
> +		return xstrdup(git_common_path("worktrees/%s", wt->id));
>  }
>