From: Patrick Steinhardt <ps@pks.im>
Date: Thu, 23 Oct 2025 05:49:55 GMT
Subject: Re: [PATCH 8/9] xdiff: change rindex from long to size_t in xdfile_t
Message-ID: <aPnCA7lzREhUETKc@pks.im>
In-Reply-To: <CAH=ZcbBbnoiBndEYryMpDzav+-iHFA7_3BPNw8hgOBiaFjCq0A@mail.gmail.com>

On Wed, Oct 22, 2025 at 04:14:42PM -0600, Ezekiel Newren wrote:
> On Tue, Oct 21, 2025 at 2:34 AM Patrick Steinhardt <ps@pks.im> wrote:
> >
> > 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.

Sounds good to me, thanks!

Patrick