From: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
Date: Fri, 06 Feb 2026 17:06:54 GMT
Subject: Re: [PATCH V2 2/3] wt-status: pass struct repository and wt_status through function parameters
Message-ID: <20260206170747.1231093-1-shreyanshpaliwalcmsmn@gmail.com>
In-Reply-To: <997a4a47-2d00-418f-b0a6-3e4dc2f45bbb@gmail.com>

> On 06/02/2026 12:57, Shreyansh Paliwal wrote:
> > I tried this out below, and it showed no fails in tests.
> > After this we can just directly replace all the_repository with 'r' or 's->repo'
> > without the hassle of checking the worktree is defined or not.
> 
> As we're trying to remove uses of "the_repository" I think you should 
> use "wt->repo" where we know always "wt != NULL". There are not many 
> callers of these functions so is easy to do necessary analysis (see below).
> 
> > diff --git a/branch.c b/branch.c
> > index 243db7d0fc..0a0097dd85 100644
> > --- a/branch.c
> > +++ b/branch.c
> > @@ -412,7 +412,7 @@ static void prepare_checked_out_branches(void)
> >   			free(old);
> >   		}
> >   
> > -		if (wt_status_check_rebase(wt, &state) &&
> > +		if (wt_status_check_rebase(the_repository, wt, &state) &&
> 
> As I said yesterday we know "wt != NULL" here so it is fine to use 
> "wt->repo" rather than introduce a new use of "the_repository", you just 
> need to explain that in the commit message.
> 
> >   		    (state.rebase_in_progress || state.rebase_interactive_in_progress) &&
> >   		    state.branch) {
> >   			struct strbuf ref = STRBUF_INIT;
> > @@ -425,7 +425,7 @@ static void prepare_checked_out_branches(void)
> >   		}
> >   		wt_status_state_free_buffers(&state);
> >   
> > -		if (wt_status_check_bisect(wt, &state) &&
> > +		if (wt_status_check_bisect(the_repository, wt, &state) &&
> 
> The same is true here.
> 
> >   		    state.bisecting_from) {
> >   			struct strbuf ref = STRBUF_INIT;
> >   			strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from);
> > diff --git a/worktree.c b/worktree.c
> > index 9308389cb6..86eff384ae 100644
> > --- a/worktree.c
> > +++ b/worktree.c
> > @@ -443,7 +443,7 @@ int is_worktree_being_rebased(const struct worktree *wt,
> >   	int found_rebase;
> >   
> >   	memset(&state, 0, sizeof(state));
> > -	found_rebase = wt_status_check_rebase(wt, &state) &&
> > +	found_rebase = wt_status_check_rebase(the_repository, wt, &state) &&
> 
> This function is called from 
> builtin/branch.c:reject_rebase_or_bisect_branch() with "wt != NULL". It 
> is also called from worktree.c:is_shared_symref() which dereferences wt 
> before calling this function so we can assume "wt != NULL" there as 
> well. That means we can use "wt->repo" here.
> 
> >   		       (state.rebase_in_progress ||
> >   			state.rebase_interactive_in_progress) &&
> >   		       state.branch &&
> > @@ -460,7 +460,7 @@ int is_worktree_being_bisected(const struct worktree *wt,
> >   	int found_bisect;
> >   
> >   	memset(&state, 0, sizeof(state));
> > -	found_bisect = wt_status_check_bisect(wt, &state) &&
> > +	found_bisect = wt_status_check_bisect(the_repository, wt, &state) &&
> 
> The same analysis for is_worktree_being_rebased() applies here.
> 
> The changes to get_branch() below look sensible

Thank you for explaining this and for each case.
I have understood where wt->repo can be used safely.
Will make changes and send a v3.
Hopefully that will be good to go :)

Best,
Shreyansh