Re: [PATCH 8/9] xdiff: change rindex from long to size_t in xdfile_t

From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Oct 22, 2025, 22:14 UTC
Message-ID: <CAH=ZcbBbnoiBndEYryMpDzav+-iHFA7_3BPNw8hgOBiaFjCq0A@mail.gmail.com>
In-Reply-To: <aPdFeHZKEsRw1cTX@pks.im>

On Tue, Oct 21, 2025 at 2:34 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 11 quoted lines

>
> On Wed, Oct 15, 2025 at 09:18:20PM +0000, Ezekiel Newren via GitGitGadget wrote:
> > From: Ezekiel Newren <ezekielnewren@gmail.com>
> >
> > rindex describes a index offset which means it's an index into memory
> > which should use size_t. dstart and dend will be deleted in a future
> > patch series. Move them to the end to help avoid refactor conflicts.
>
> In a patch like this I would appreciate some explanation why we can
> change the type without adapting any of its users. So basically explain
> why this refactoring is safe to do and won't cause any issues.

The values of rindex are only used in 3 places. get_hash() which was created in [1]. and 2 places in xdl_recs_cmp(). All of them use rindex as an index into another array directly so there's no cascading refactor impact. get_hash() was created precisely to reduce refactor churn. How about a commit message like:

Changing the type of rindex from long to size_t has no cascading refactor impact because it is only ever used to directly index other arrays.

[1] create get_hash() https://lore.kernel.org/git/637d1032abbd33b7673d3c101267816fbf1a343c.1758926520.git.gitgitgadget@gmail.com/