Re: [PATCH 07/10] xdiff: replace xdfile_t.dstart with xdfenv_t.delta_start

[Phillip Wood <phillip.wood123@gmail.com>]

On 20/01/2026 16:32, Phillip Wood wrote:
> On 02/01/2026 18:52, Ezekiel Newren via GitGitGadget wrote:
>> From: Ezekiel Newren <ezekielnewren@gmail.com>
>>
>> Placing delta_start in xdfenv_t instead of xdfile_t provides a more
>> appropriate context since this variable only makes sense with a pair
>> of files. View with --color-words.
> 
> So as dstart and dend must be the same for both files we now store the 
> values once in xdfenv_t. 

Except it's only dstart that's the same, dend is different because it 
convinently stores an index, not an offset from the end. Having realized 
that, moving them to xdfenv_t makes less sense as having to calculate 
the dend index from an offset from the end of the array each time is a 
pain and sooner or later we'll make a mistake.

Thanks

Phillip

> That explains why we start passing xdfenv_t 
> around rather than xdfile_t in patch 5.
> 
> Thanks
> 
> Phillip
> 
>> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
>> ---
>>   xdiff/xhistogram.c |  4 ++--
>>   xdiff/xpatience.c  |  4 ++--
>>   xdiff/xprepare.c   | 17 +++++++++--------
>>   xdiff/xtypes.h     |  3 ++-
>>   4 files changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
>> index 5ae1282c27..eb6a52d9ba 100644
>> --- a/xdiff/xhistogram.c
>> +++ b/xdiff/xhistogram.c
>> @@ -365,6 +365,6 @@ out:
>>   int xdl_do_histogram_diff(xpparam_t const *xpp, xdfenv_t *env)
>>   {
>>       return histogram_diff(xpp, env,
>> -        env->xdf1.dstart + 1, env->xdf1.dend - env->xdf1.dstart + 1,
>> -        env->xdf2.dstart + 1, env->xdf2.dend - env->xdf2.dstart + 1);
>> +        env->delta_start + 1, env->xdf1.dend - env->delta_start + 1,
>> +        env->delta_start + 1, env->xdf2.dend - env->delta_start + 1);
>>   }
>> diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c
>> index 2bce07cf48..bd0ffbb417 100644
>> --- a/xdiff/xpatience.c
>> +++ b/xdiff/xpatience.c
>> @@ -374,6 +374,6 @@ static int patience_diff(xpparam_t const *xpp, 
>> xdfenv_t *env,
>>   int xdl_do_patience_diff(xpparam_t const *xpp, xdfenv_t *env)
>>   {
>>       return patience_diff(xpp, env,
>> -        env->xdf1.dstart + 1, env->xdf1.dend - env->xdf1.dstart + 1,
>> -        env->xdf2.dstart + 1, env->xdf2.dend - env->xdf2.dstart + 1);
>> +        env->delta_start + 1, env->xdf1.dend - env->delta_start + 1,
>> +        env->delta_start + 1, env->xdf2.dend - env->delta_start + 1);
>>   }
>> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
>> index 06b6a6f804..e88468e74c 100644
>> --- a/xdiff/xprepare.c
>> +++ b/xdiff/xprepare.c
>> @@ -173,7 +173,6 @@ static int xdl_prepare_ctx(mmfile_t *mf, xdfile_t 
>> *xdf, uint64_t flags) {
>>       xdf->changed += 1;
>>       xdf->nreff = 0;
>> -    xdf->dstart = 0;
>>       xdf->dend = xdf->nrec - 1;
>>       return 0;
>> @@ -287,7 +286,7 @@ static int xdl_cleanup_records(xdlclassifier_t 
>> *cf, xdfenv_t *xe) {
>>        */
>>       if ((mlim = xdl_bogosqrt((long)xe->xdf1.nrec)) > XDL_MAX_EQLIMIT)
>>           mlim = XDL_MAX_EQLIMIT;
>> -    for (i = xe->xdf1.dstart, recs = &xe->xdf1.recs[xe->xdf1.dstart]; 
>> i <= xe->xdf1.dend; i++, recs++) {
>> +    for (i = xe->delta_start, recs = &xe->xdf1.recs[xe->delta_start]; 
>> i <= xe->xdf1.dend; i++, recs++) {
>>           rcrec = cf->rcrecs[recs->minimal_perfect_hash];
>>           nm = rcrec ? rcrec->len2 : 0;
>>           action1[i] = (nm == 0) ? DISCARD: (nm >= mlim && ! 
>> need_min) ? INVESTIGATE: KEEP;
>> @@ -295,7 +294,7 @@ static int xdl_cleanup_records(xdlclassifier_t 
>> *cf, xdfenv_t *xe) {
>>       if ((mlim = xdl_bogosqrt((long)xe->xdf2.nrec)) > XDL_MAX_EQLIMIT)
>>           mlim = XDL_MAX_EQLIMIT;
>> -    for (i = xe->xdf2.dstart, recs = &xe->xdf2.recs[xe->xdf2.dstart]; 
>> i <= xe->xdf2.dend; i++, recs++) {
>> +    for (i = xe->delta_start, recs = &xe->xdf2.recs[xe->delta_start]; 
>> i <= xe->xdf2.dend; i++, recs++) {
>>           rcrec = cf->rcrecs[recs->minimal_perfect_hash];
>>           nm = rcrec ? rcrec->len1 : 0;
>>           action2[i] = (nm == 0) ? DISCARD: (nm >= mlim && ! 
>> need_min) ? INVESTIGATE: KEEP;
>> @@ -306,10 +305,10 @@ static int xdl_cleanup_records(xdlclassifier_t 
>> *cf, xdfenv_t *xe) {
>>        * false, or become true.
>>        */
>>       xe->xdf1.nreff = 0;
>> -    for (i = xe->xdf1.dstart, recs = &xe->xdf1.recs[xe->xdf1.dstart];
>> +    for (i = xe->delta_start, recs = &xe->xdf1.recs[xe->delta_start];
>>            i <= xe->xdf1.dend; i++, recs++) {
>>           if (action1[i] == KEEP ||
>> -            (action1[i] == INVESTIGATE && !xdl_clean_mmatch(action1, 
>> i, xe->xdf1.dstart, xe->xdf1.dend))) {
>> +            (action1[i] == INVESTIGATE && !xdl_clean_mmatch(action1, 
>> i, xe->delta_start, xe->xdf1.dend))) {
>>               xe->xdf1.reference_index[xe->xdf1.nreff++] = i;
>>               /* changed[i] remains false, i.e. keep */
>>           } else
>> @@ -318,10 +317,10 @@ static int xdl_cleanup_records(xdlclassifier_t 
>> *cf, xdfenv_t *xe) {
>>       }
>>       xe->xdf2.nreff = 0;
>> -    for (i = xe->xdf2.dstart, recs = &xe->xdf2.recs[xe->xdf2.dstart];
>> +    for (i = xe->delta_start, recs = &xe->xdf2.recs[xe->delta_start];
>>            i <= xe->xdf2.dend; i++, recs++) {
>>           if (action2[i] == KEEP ||
>> -            (action2[i] == INVESTIGATE && !xdl_clean_mmatch(action2, 
>> i, xe->xdf2.dstart, xe->xdf2.dend))) {
>> +            (action2[i] == INVESTIGATE && !xdl_clean_mmatch(action2, 
>> i, xe->delta_start, xe->xdf2.dend))) {
>>               xe->xdf2.reference_index[xe->xdf2.nreff++] = i;
>>               /* changed[i] remains false, i.e. keep */
>>           } else
>> @@ -348,7 +347,7 @@ static void xdl_trim_ends(xdfenv_t *xe)
>>           size_t mph1 = xe->xdf1.recs[i].minimal_perfect_hash;
>>           size_t mph2 = xe->xdf2.recs[i].minimal_perfect_hash;
>>           if (mph1 != mph2) {
>> -            xe->xdf1.dstart = xe->xdf2.dstart = (ssize_t)i;
>> +            xe->delta_start = (ssize_t)i;
>>               lim -= i;
>>               break;
>>           }
>> @@ -370,6 +369,8 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, 
>> xpparam_t const *xpp,
>>               xdfenv_t *xe) {
>>       xdlclassifier_t cf;
>> +    xe->delta_start = 0;
>> +
>>       if (xdl_prepare_ctx(mf1, &xe->xdf1, xpp->flags) < 0) {
>>           return -1;
>> diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h
>> index 979586f20a..bda1f85eb0 100644
>> --- a/xdiff/xtypes.h
>> +++ b/xdiff/xtypes.h
>> @@ -48,7 +48,7 @@ typedef struct s_xrecord {
>>   typedef struct s_xdfile {
>>       xrecord_t *recs;
>>       size_t nrec;
>> -    ptrdiff_t dstart, dend;
>> +    ptrdiff_t dend;
>>       bool *changed;
>>       size_t *reference_index;
>>       size_t nreff;
>> @@ -56,6 +56,7 @@ typedef struct s_xdfile {
>>   typedef struct s_xdfenv {
>>       xdfile_t xdf1, xdf2;
>> +    size_t delta_start;
>>   } xdfenv_t;
> 
>