Re: [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers

>
> On Thu, Apr 02, 2026 at 08:03:45PM +0300, Burak Kaan Karaçay wrote:
> > Hi,
> >
> > On Thu, Apr 02, 2026 at 09:27:33AM +0200, Patrick Steinhardt wrote:
> > > On Sun, Mar 29, 2026 at 03:46:39PM +0530, Shreyansh Paliwal wrote:
> > > > diff --git a/refs.c b/refs.c
> > > > index 685a0c247b..5cdc8858c5 100644
> > > > --- a/refs.c
> > > > +++ b/refs.c
> > > > @@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
> > > >   strbuf_add(sb, name + used, len - used);
> > > >  }
> > > >
> > > > -int check_branch_ref(struct strbuf *sb, const char *name)
> > > > +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> > > >  {
> > > >   if (startup_info->have_repository)
> > > > -         copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> > > > +         copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> > > >   else
> > > >           strbuf_addstr(sb, name);
> > > >
> > >
> > > I have to agree with Tian's comment on v2, this part here looks wrong. I
> > > don't think we should depend on `startup_info` here, but we should
> > > exclusively rely on whether or not the caller has passed in a
> > > repository. And that will likely require a bit more scrutiny to figure
> > > out whether there are any callers that shouldn't pass in a repository
> > > because it's not initialized.
> > >
> > > Alternatively, we could go with Tian's suggestion of checking for `repo
> > > && repo->gitdir`.
> > >
> > > Patrick
> >
> > This approach actually leads to a bug and segfault in a specific edge
> > case when running 'git check-ref-format'. The current tests don't cover
> > this scenario, but they can be extended to catch it.
>
> > If GIT_DIR is set to a non-existent path,
> > 'startup_info->have_repository' becomes '0' but 'repo->gitdir' still
> > holds the invalid path. As a result, the code enters the first condition
> > and crashes. The case can be tested with this command:
> >
> > $ git --git-dir='non-existing' check-ref-format --branch @{-1}
> >
> > Modifying the behavior of 'repo->gitdir' might solve the issue, but I
> > belive that falls outside the scope of this patch. After a quick search,
> > I found a prophecy from Peff about the 'startup_info->have_repository':
> >
> > [1] https://lore.kernel.org/git/20190806124954.GA13649@sigill.intra.peff.net/
>
> If we cannot make it work in this patch series, the next question is
> whether we actually want to give the false sense of `check_branch_ref()`
> being independent of global state, or whether we want to leave it as-is
> for now and then do a follow-up patch series where we fix the issue and
> adapt the interface.