From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 24 Mar 2026 14:11:13 GMT
Subject: Re: [PATCH v2 2/2] rev-parse: use selected alternate terms too look up refs
Message-ID: <5e7aa1bb-1eb7-4b8c-8bd1-032be6a02a82@gmail.com>
In-Reply-To: <888f8670-856a-4ce7-8177-da78ba4f0c8a@schlaraffenlan.de>

Hi Jonas

On 24/03/2026 12:30, Jonas Rebmann wrote:
> 
> On 24/03/2026 11.49, Phillip Wood wrote:
>> If we fail to read the terms because there is no bisect in progress
>> then term_bad and term_good will be NULL and so the next line will
>> segfault.
> 
> My understanding of read_bisect_terms() was that it never sets the terms
> to NULL, that if no bisect is in progress, .git/BISECT_TERMS does not
> exist, and the terms default to "good"/"bad" here in bisect.c:
> 
>      if (errno == ENOENT) {
>          free(*read_bad);
>          *read_bad = xstrdup("bad");
>          free(*read_good);
>          *read_good = xstrdup("good");
>          return;
>      } else {
>          die_errno(_("could not read file '%s'"), filename);
>      }
> 
> So is a NULL-check really needed on caller end?

Oh, sorry I'd misread it, you're correct that we don't need a NULL 
check. Looking at the history of "git rev-parse --bisect" it was 
introduced in ad3f9a71a82 (Add '--bisect' revision machinery argument, 
2009-10-27) which also added a '--bisect' option to "git rev-list". 
Looking at the implementation of that option I wonder if we should make 
revision.c:for_each_bisect_ref() public and call it from 
builtin/rev-parse.c rather than building the refnames and calling 
refs_for_each_ref_ext().

Thanks

Phillip

> 
> Regards,
> Jonas