Re: [PATCH 2/5] xdiff: replace mmbuffer_t with mmfile_t

From: Patrick Steinhardt <ps@pks.im>
Date: Sep 30, 2026, 15:32 UTC
Message-ID: <ar0roZKCwALv0n_A@pks.im>
In-Reply-To: <20260929065239.GB1697497@coredump.intra.peff.net>

On Tue, Sep 29, 2026 at 02:52:39AM -0400, Jeff King wrote:

Show 17 quoted lines

> Our import of xdiff has two identical buffer structures: mmfile_t and
> mmbuffer_t. In upstream xdiff these were actually different, but the
> import in 3443546f6e (Use a *real* built-in diff generator, 2006-03-24)
> simplified mmfile_t to a simple buffer.
> 
> In xdiff we usually use mmfile_t for input and mmbuffer_t for output,
> but they are really both just a ptr/len pair. I don't think that having
> different types is buying us anything in terms of type safety or
> semantics, and having two makes it awkward to use the same helpers for
> both. In particular, an external merge driver's output is read from a
> file, but we can't easily use read_mmfile(), since we want the result in
> an mmbuffer_t.
> 
> Let's use mmfile_t for both cases and drop mmbuffer_t. The latter is
> probably a more descriptive name, but we have many more uses of
> mmfile_t (and helpers like read_mmfile). So let's consolidate using that
> name; we can always change it to something more sensible later.

Yeah, that was my initial reaction, too. `mmbuffer_t` is indeed a better name as `mmfile_t` indicates that it's coming from... well, a file. And that's not necessarily true.

I do wonder whether we should just aim for gradual improvement and use `mmbuffer_t` regardless or even shoot for something altogether different like `struct xdiff_buf` and then simply not mind the fact that we're being inconsistent. That would at least be an initial step into a better direction in my opinion, and we can then touch up things over some time.

But I won't insist on any change like that, I'm okay with keeping `mmfile_t`.

Patrick