Re: [PATCH v3 00/10] Xdiff cleanup part2

From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Nov 14, 2025, 05:52 UTC
Message-ID: <CAH=ZcbAQ5fCUuL3cpETQmGNXsPE_5UMf4CqVgjj0vvmXmU7-Vg@mail.gmail.com>
In-Reply-To: <xmqqqzu4t9yc.fsf@gitster.g>

On Tue, Nov 11, 2025 at 4:40 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

>
> "Ezekiel Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > The primary goal of this patch series is to convert every field's type in
> > xrecord_t and xdfile_t to be unambiguous, in preparation to make it more
> > Rust FFI friendly. Additionally the ha field in xrecord_t is split into
> > line_hash and minimal_perfect hash.
>
> After having read the series to its end, I am left with this feeling
> that it does only half the things that it needs to do.  It does all
> what the above paragraph claims it does, sure, in that the relevant
> data structures now use not "long" but "size_t", not "char" but
> "uint8_t", etc., and I do find the resulting data structures sensibly
> described.

This patch series is already 10 commits long, and it's been a challenge to chunk cleanups of Xdiff because its code is so tangled. I'm hoping that future maintenance of Xdiff (after my xdiff cleanup series is complete) will be much easier.

Show 11 quoted lines

> But for the code to be truly consistent between the data structures
> and the operations that work on them, types of on-stack variables
> and function parameters would need to be updated to match these
> struct members.  As we convert one structure member at a time, casts
> may need to be sprinkled for assignments to these variables and
> passing these struct members as parameters to functions (which I
> commented on one of these patches) to keep the blast radius of the
> changes in each step manageable, but I would have expected that
> functions that used to take, say, an "int", would be updated to take
> "size_t" if the value coming to the parameter is from these struct
> members.

I had to draw the line somewhere, and I plan on making more changes to delete more idiosyncrasies in Xdiff.

> Perhaps that would be the theme for "Xdiff cleanup part 3" series
> that we will eventually see after the dust settles from this round?

Not just part 3, but the entire xdiff cleanup series will be about correcting types among many other code cleanups. This patch series alone is unsatisfactory, but it is only 1 of many patch series to come.