From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 29 Sep 2026 18:37:00 GMT
Subject: Re: [PATCH 1/5] xdiff: clean up read_mmfile() allocations on error
Message-ID: <xmqqbj9fhpkj.fsf@gitster.g>
In-Reply-To: <20260929065131.GA1697497@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> When read_mmfile() returns an error, it may or may not have allocated a
> buffer in the passed-in mmfile_t. So callers must initialize the pointer
> to NULL and free it even on error.
>
> Most callers do this already, but rerere's diff_two() does not, and
> would leak the buffer after a read error. We could fix it directly, but
> let's instead try to make the interface less error-prone by freeing the
> memory when returning failure from read_mmfile().
>
> This fixes (part of) the leak in diff_two(). In theory it also lets us
> simplify other callers to skip initializing the mmfile. But in practice
> most still need zero-initialization because they may jump to free()
> before even calling read_mmfile (e.g., in try_merge()). But we can at
> least simplify rerere_forget_one_path() a bit.
>
> I said "part of" earlier. There's a related leak in diff_two(): if
> reading the first file succeeds but reading the second fails, we return
> early and leak the first buffer. We can fix that by checking each
> individually.

Nice.  Thanks for plugging my leaks.