From: Phillip Wood Date: Thu, 06 Nov 2025 11:00:34 GMT Subject: Re: [PATCH v2 06/10] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Message-ID: In-Reply-To: <59054ea0cb65718dbac500d342bc960bdb5066c1.1761776388.git.gitgitgadget@gmail.com> Hi Ezekiel On 29/10/2025 22:19, Ezekiel Newren via GitGitGadget wrote: > From: Ezekiel Newren > > The ha field is serving two different purposes, which makes the code > harder to read. At first glance it looks like many places assume > there could never be hash collisions between lines of the two input > files. In reality, line_hash is used together with xdl_recmatch() to > ensure correct comparisons of lines, even when collisions occur. > > To make this clearer, the old ha field has been split: > * line_hash: The straightforward hash of a line, requiring no > additional context. > * minimal_perfect_hash: Not a new concept, but now a separate > field. It comes from the classifier's general-purpose hash table, > which assigns each line a unique and minimal hash across the two > files. It would be nice to explain the differing types for the two fields in the commit message. > diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c > index 85e56021da..16236bd045 100644 > --- a/xdiff/xprepare.c > +++ b/xdiff/xprepare.c > @@ -96,9 +96,9 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t > long hi; > xdlclass_t *rcrec; > > - hi = (long) XDL_HASHLONG(rec->ha, cf->hbits); > + hi = (long) XDL_HASHLONG(rec->line_hash, cf->hbits); "hi" is only used as an array index so it might be nicer to change it to size_t and avoid this cast instead. Thanks Phillip