Re: [PATCH 3/7] setup: remove global `git_work_tree_cfg` variable

[Justin Tobler <jltobler@gmail.com>]

On 26/06/10 08:56AM, Patrick Steinhardt wrote:
> The global `git_work_tree_cfg` variable used to be modified by both
> "setup.c" and by "builtin/init-db.c". We have refactored the latter user
> to not use that variable at all anymore in a preceding commit, which
> makes "setup.c" the only remaining user.
> 
> Even for "setup.c" it is unnecessary though, as we only ever set it to
> the value we have stored in the discovered repository format. The
> consequence is that we only ever set it in case we already have it set
> to the same value in our discovered repository format, which makes it
> redundant.

Nice. I was wondering in the first patch if there would really be any
need to keep `git_work_tree_cfg` around here at all. Makes sense to me
to just set the repository worktree directly.

> Refactor the code so that we instead use the worktree configuration as
> discovered via the repository format. Drop the global variable.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  setup.c | 28 +++++++++++-----------------
>  1 file changed, 11 insertions(+), 17 deletions(-)
> 
> diff --git a/setup.c b/setup.c
> index 52228b42a1..71fc6b33da 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -31,9 +31,6 @@ enum allowed_bare_repo {
>  	ALLOWED_BARE_REPO_ALL,
>  };
>  
> -/* This is set by setup_git_directory_gently() and/or git_default_config() */
> -static char *git_work_tree_cfg;
> -
>  static struct startup_info the_startup_info;
>  struct startup_info *startup_info = &the_startup_info;
>  const char *tmp_original_cwd;
> @@ -799,13 +796,10 @@ static int check_repository_format_gently(const char *gitdir,
>  	}
>  
>  	if (!has_common) {
> -		if (candidate->is_bare != -1) {
> +		if (candidate->is_bare != -1)
>  			is_bare_repository_cfg = candidate->is_bare;
> -		}
> -		if (candidate->work_tree) {
> -			free(git_work_tree_cfg);
> -			git_work_tree_cfg = xstrdup(candidate->work_tree);

Ok, we no longer set `git_work_tree_cfg` in favor of just relying on the
worktree specified in the repository format.

> -		}
> +	} else {
> +		FREE_AND_NULL(candidate->work_tree);

Huh, we were not previously freeing the worktree here, but I assume this
to avoid a resource leak?

The remainder of this patch replaces `git_work_tree_cfg` usage with the
repository format worktree and looks good.

-Justin