Re: [PATCH v4 0/2] merge-tree: fix segmentation fault in read-only repositories

From: Elijah Newren <newren@gmail.com>
Date: Sep 27, 2022, 08:11 UTC
Message-ID: <CABPp-BGJueKtcdvzGWH_ZK6yeA6r3457ue5Uub9_gdU5M-WmvQ@mail.gmail.com>
In-Reply-To: <pull.1362.v4.git.1664229348.gitgitgadget@gmail.com>

On Mon, Sep 26, 2022 at 2:55 PM Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 13 quoted lines

>
> Turns out that the segmentation fault reported by Taylor
> [https://lore.kernel.org/git/YyopQD+LvPucnz3w@nand.local/] happened while
> testing merge-ort in a read-only repository, and that the upstream version
> of git merge-tree is as affected as GitHub's internal version.
>
> Changes since v3:
>
>  * I now consistently use the pattern int ret = 0; ... if (...) ret = -1.
>  * I added a commit to properly propagate write failures through the
>    handle_content_merge() call path, even if it is not really critical (it
>    just fails sooner, but the merge would have failed just the same without
>    this patch).

I'm a little unsure about the commit message of patch 2, as I commented on, but the code all looks good to me. I don't think the comment in the commit message matters too much; whether your commit message addresses the weird case I mentioned, I feel the code handles it just fine. So I'll leave it up to you to decide whether to tweak the commit message or not. Anyway, after you decide on that, feel free to apply my

Reviewed-by: Elijah Newren <newren@gmail.com>

Thanks for investigating, for tracking down all the return codes and how to propagate them, and getting it all fixed up!