Re: [PATCH 1/2] wt-status: avoid passing NULL worktree
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Feb 17, 2026, 17:46 UTC
- Message-ID
- <CAOLa=ZQKLqFn4w3s7PD87FZ_120gohoqKX5c3uLKo2vASsbxfA@mail.gmail.com>
- In-Reply-To
- <409871a7d521b76c9eb811d3c49747e04de8defc.1771258688.git.phillip.wood@dunelm.org.uk>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 7 quoted lines
> From: Phillip Wood <phillip.wood@dunelm.org.uk> > > In preparation for removing the repository argument from > worktree_git_path() add a function to construct a "struct worktree" > from a "struct repository" and use that to avoid passing a NULL > worktree to wt_status_check_bisect() and wt_status_check_rebase(). >
Okay this makes sense, I'm curious how 'wt->id = NULL' is going to be handled. Let's see
Show 17 quoted lines
> wt_status_check_bisect() and wt_status_check_rebase() have the following > callers: > > - branch.c:prepare_checked_out_branches() which loops over all > worktrees. > > - worktree.c:is_worktree_being_rebased() which is called from > builtin/branch.c:reject_rebase_or_bisect_branch() that loops over all > worktrees and worktree.c:is_shared_symref() which dereferences wt > earlier in the function. > > - wt-status:wt_status_get_state() which is updated to avoid passing a > NULL worktree by this patch. > > This updates the only callers that pass a NULL worktree to > worktree_git_path(). >
I was thinking surely there must be other places where we also pass NULL for worktree, but doesn't seem like there are any such instances.
Show 24 quoted lines
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
> worktree.c | 20 ++++++++++++++++++++
> worktree.h | 5 ++++-
> wt-status.c | 15 ++++++++++++---
> 3 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/worktree.c b/worktree.c
> index 9308389cb6f..fd182c319b7 100644
> --- a/worktree.c
> +++ b/worktree.c
> @@ -66,6 +66,26 @@ static int is_current_worktree(struct worktree *wt)
> return is_current;
> }
>
> +struct worktree *get_worktree_from_repository(struct repository *repo)
> +{
> + struct worktree *wt = xcalloc(1, sizeof(*wt));
> + char *gitdir = absolute_pathdup(repo->gitdir);
> + char *commondir = absolute_pathdup(repo->commondir);
> +
> + wt->repo = repo;
> + if (repo->worktree)
> + wt->path = absolute_pathdup(repo->worktree);Shouldn't this always be set? I guess my question is, will `repo->worktree` ever be NULL?
> + wt->is_bare = !!repo->worktree; > + if (fspathcmp(gitdir, commondir)) > + wt->id = xstrdup(find_last_dir_sep(commondir) + 1);
So here we continue to treat NULL as the main worktree. Okay.
> + wt->is_current = is_current_worktree(wt);
Since we're getting the worktree from the repo, shouldn't this be 'true'?
Show 23 quoted lines
> + add_head_info(wt); > + > + free(gitdir); > + free(commondir); > + return wt; > +} > + > /* > * When in a secondary worktree, and when extensions.worktreeConfig > * is true, only $commondir/config and $commondir/worktrees/<id>/ > diff --git a/worktree.h b/worktree.h > index e4bcccdc0ae..b162bbabd50 100644 > --- a/worktree.h > +++ b/worktree.h > @@ -38,7 +38,10 @@ struct worktree **get_worktrees(void); > */ > struct worktree **get_worktrees_without_reading_head(void); > > -/* > +/* Construct a struct worktree from a struct repository */ > +struct worktree *get_worktree_from_repository(struct repository *repo); > + > + /*
Nit: extra space?
> * Returns 1 if linked worktrees exist, 0 otherwise. > */ > int submodule_uses_worktrees(const char *path);
[snip]