Re: [PATCH v2] rerere: fix crash during clear

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 20, 2024, 01:22 UTC
Message-ID: <xmqqplwsx730.fsf@gitster.g>
In-Reply-To: <20240218194603.1210895-1-marcel@roethke.info>

Marcel Röthke <marcel@roethke.info> writes:

> When rerere_clear is called, for instance when aborting a rebase, and
> the current conflict does not have a pre or postimage recorded git
> crashes with a SEGFAULT in has_rerere_resolution when accessing the
> status member of struct rerere_dir.

I had to read this twice before realizing the reason why I found it hard to grok was because of a missing comma between "recorded" and "git".

> This happens because scan_rerere_dir
> only allocates the status field in struct rerere_dir when a post or
> preimage was found.

But that is not really the root cause, no? Readers following the above text are probably wondering why the preimage was not recorded, when a conflict resulted in stopping a mergy-command and invoking rerere machinery, before rerere_clear() got called. Is that something that usually happen? How? Do we have a reproduction sequence of such a state that we can make it into a new test in t4200 where we already have tests for "git rerere clear" and its friends?

> In some cases a segfault may happen even if a post
> or preimage was recorded if it was not for the variant of interest and
> the number of the variant that is present is lower than the variant of
> interest.

Ditto.  What sequence of events would lead to such a state?

The answer *can* be ".git/rr-cache being a normal directory, the user can poke around, removing files randomly, which can create such a problematic situation", and the reproduction test *can* also be to simulate such an end-user action, but I am asking primarily because I want to make sure that we are *not* losing or failing to create necessary preimage files, causing this problem to users who do not muck with files in .git/rr-cache themselves.

Thanks.