Subject: Re: [PATCH 02/18] setup: stop using `the_repository` in `is_inside_worktree()`

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 18 May 2026 09:46:42 +0200

On Mon, May 18, 2026 at 08:41:10AM +0900, Junio C Hamano wrote:
> Tian Yuchen <[email-masked]> writes:
> 
> > On 4/20/26 16:22, Patrick Steinhardt wrote:
> >
> > I've noticed something that does not entirely convince me:
> >
> >> @@ -477,11 +476,13 @@ int is_inside_git_dir(void)
> >>  	return inside_git_dir;
> >>  }
> >>  
> >> -int is_inside_work_tree(void)
> >> +int is_inside_work_tree(struct repository *repo)
> >>  {
> >> -	if (inside_work_tree < 0)
> >> -		inside_work_tree = is_inside_dir(repo_get_work_tree(the_repository));
> >> -	return inside_work_tree;
> >> +	static struct strbuf buf = STRBUF_INIT;
> >> +	const char *worktree = repo_get_work_tree(repo);
> >> +	if (!worktree)
> >> +		return 0;
> >> +	return is_inside_dir(strbuf_realpath(&buf, worktree, 1));
> >>  }
> >> 
> >
> > Is it correct to statically allocate memory for the cache here? Could 
> > this lead
> > to memory overwriting issues in a multi-threaded environment?
> 
> I do not offhand know if other code paths that are called from this
> function are thread-safe, but yeah, this use of file-scope static is
> not a safe thing to do.

In the current status quo this is a safe conversion to do, as we still
have the assumption ingrained that this only works for a single repo.
We were using static variables before, and we're still using static
variables now.

That being said, I wouldn't mind dropping the static variable if this is
something we'd rather want to get rid of. It's a smell that I'm not
particularly happy about myself.

I'll send a revised version in a bit, thanks!

Patrick