Re: [PATCH V2 2/3] wt-status: pass struct repository and wt_status through function parameters
- From
Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
- Date
- Feb 6, 2026, 17:06 UTC
- Message-ID
- <20260206170747.1231093-1-shreyanshpaliwalcmsmn@gmail.com>
- In-Reply-To
- <997a4a47-2d00-418f-b0a6-3e4dc2f45bbb@gmail.com>
Show 69 quoted lines
> 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 sensibleThank 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