Re: [PATCH v3 5/5] builtin/stash: merge index in-core

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 28, 2026, 15:32 UTC
Message-ID: <xmqq7bk5o0hs.fsf@gitster.g>
In-Reply-To: <CALnO6CCL6-7Ze0az68NRs2PAr+VJJ=ihU0s+C+DK-bsMB+XGww@mail.gmail.com>

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

Show 17 quoted lines

> On Mon, Sep 28, 2026 at 5:40 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> "D. Ben Knoble" <ben.knoble@gmail.com> writes:
>>
>> > +                     merge_incore_nonrecursive(&o, merge_base, head, merge,
>> > +                                               &result);
>> > +
>> > +                     oidcpy(&index_tree, &result.tree->object.oid);
>>
>> This is risky, isn't it?
>>
>> If there were catastrophic failure (e.g., missing object that were
>> involved in the merge), merge_incore_nonrecursive() may stuff -1 to
>> result.clean and return without populating result.tree, and when
>> that happens, result.tree->object.oid would be dereferencing NULL.
>
> Indeed… unfortunate. Thanks for spotting.

I did

	$ git grep merge_incore_nonrecursive \*.c

and read all the current callers.

They all have code to specifically check for the (result.clean < 0) condition and error out before touching any of the other members of the result structure, so they seem to be safe.