Re: Segfault: git show-branch --reflog refs/pullreqs/1

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 21, 2024, 10:05 UTC
Message-ID: <ZdXLBIv1vLjhQUgx@tanuki>
In-Reply-To: <20240221084250.GA25385@coredump.intra.peff.net>

On Wed, Feb 21, 2024 at 03:42:50AM -0500, Jeff King wrote:

Show 15 quoted lines

> On Wed, Feb 21, 2024 at 10:48:25AM +0900, Yasushi SHOJI wrote:
> 
> > Does anyone see a segfault on `git show-branch --reflog refs/pullreqs/1`?
> > 
> > It seems files_reflog_path() creates a wrong path with the above command
> > using REF_WORKTREE_SHARED.
> I am still trying to wrap my head around how it can get such wrong
> results for show-branch when asking for "git rev-parse branch@{0}", etc,
> are correct. I think it is that "rev-parse branch@{0}" is only looking
> at the output oid and does not consider the reflog message at all. So I
> think it is subtly broken, but in a way that happens to work for that
> caller. But I'm not sure of the correct fix. At least not at this time
> of night.
> 
> Cc-ing folks involved in 6436a20284.

Ah, our mails crossed, but we came to the same conclusion. Things indeed are subtly broken here and work just by chance because all callers pre initialize the object ID. So in the case where the reflog is missing or empty we'd use that pre-initialized object ID because `read_ref_at()` does not indicate the failure to the callers.

I think that this behaviour is not sensible in the first place. When asking for the reflog, we should only ever return object IDs parsed from the reflog. Falling back to parsing the ref itself does not make much sense. I've thus sent a patch series that changes the behaviour here.

Patrick