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

[Patrick Steinhardt <ps@pks.im>]

On Wed, Jun 10, 2026 at 04:52:12PM -0500, Justin Tobler wrote:
> On 26/06/10 08:56AM, Patrick Steinhardt wrote:
> > 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?

This is in fact a required change. Before this patch this here was the
place where we populated `git_work_tree_cfg`, and that variable was then
later on applied to the repository. So the implicit `else` branch was
basically just `git_work_tree_cfg = NULL`, but we didn't have to do that
as it already was set to `NULL`.

With the new code though we're not setting `git_work_tree_cfg` anymore
and instead directly populate from `candidate->work_tree`. But that also
means that we now have to clear that variable to retain previous
semantics.

I'll add an explanation to the commit message.

Patrick