git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Ezekiel Newren via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Ezekiel Newren <ezekielnewren@gmail.com>
Subject: Re: [PATCH 08/17] xdiff: delete chastore from xdfile_t, view with --color-words
Date: Tue, 9 Sep 2025 14:50:36 +0100	[thread overview]
Message-ID: <aae1ad41-8604-45bc-8ec3-03180e6152ff@gmail.com> (raw)
In-Reply-To: <CABPp-BHdKu2nsWhpfGY4MexfChxfwv_0mqvpgrV3kbYgdCYKEg@mail.gmail.com>

On 09/09/2025 09:58, Elijah Newren wrote:
> On Sun, Sep 7, 2025 at 12:46 PM Ezekiel Newren via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Ezekiel Newren <ezekielnewren@gmail.com>
>> The chastore_t type is very unfriendly to Rust FFI. It's also redundant
>> since 'recs' is a vector type that grows every time an xrecord_t is
>> added.
> 
> The second sentence seems to presume the reader knows what chastore_t
> type is for, and about the confusing dual layering between it and
> recs.its confusing dual layering.  I liked your more extended
> explanation in https://lore.kernel.org/git/7ea2dccd71fc502f20614ce217fc9885d1b17413.1756496539.git.gitgitgadget@gmail.com/;
> could some of that be used here?

I agree that's a better explaination. I also think it would be helpful 
to spell out the implications of this change. If I understand the change 
correctly we now store all the records in a contiguous array, rather 
than having the records in a arena and storing a separate array of 
pointers to those records. As sizeof(xrecord_t) is pretty small the 
change to contiguous storage hopefully wont cause any allocation issues, 
though I guess it does mean we end up copying more data as we grow the 
array compared to using an arena.

Overall these first few patches look like a really nice cleanup.

Thanks

Phillip

>>
>> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
>> ---
>>   xdiff/xdiffi.c     | 24 ++++++++++----------
>>   xdiff/xemit.c      |  6 ++---
>>   xdiff/xhistogram.c |  2 +-
>>   xdiff/xmerge.c     | 56 +++++++++++++++++++++++-----------------------
>>   xdiff/xpatience.c  | 10 ++++-----
>>   xdiff/xprepare.c   | 19 ++++++----------
>>   xdiff/xtypes.h     |  3 +--
>>   xdiff/xutils.c     | 12 +++++-----
>>   8 files changed, 63 insertions(+), 69 deletions(-)
>>
>> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
>> index 11cd090b53..a66125d44a 100644
>> --- a/xdiff/xdiffi.c
>> +++ b/xdiff/xdiffi.c
>> @@ -24,7 +24,7 @@
>>
>>   static unsigned long get_hash(xdfile_t *xdf, long index)
>>   {
>> -       return xdf->recs[xdf->rindex[index]]->ha;
>> +       return xdf->recs[xdf->rindex[index]].ha;
>>   }
>>
>>   #define XDL_MAX_COST_MIN 256
>> @@ -489,13 +489,13 @@ static void measure_split(const xdfile_t *xdf, long split,
>>                  m->indent = -1;
>>          } else {
>>                  m->end_of_file = 0;
>> -               m->indent = get_indent(xdf->recs[split]);
>> +               m->indent = get_indent(&xdf->recs[split]);
>>          }
>>
>>          m->pre_blank = 0;
>>          m->pre_indent = -1;
>>          for (i = split - 1; i >= 0; i--) {
>> -               m->pre_indent = get_indent(xdf->recs[i]);
>> +               m->pre_indent = get_indent(&xdf->recs[i]);
>>                  if (m->pre_indent != -1)
>>                          break;
>>                  m->pre_blank += 1;
>> @@ -508,7 +508,7 @@ static void measure_split(const xdfile_t *xdf, long split,
>>          m->post_blank = 0;
>>          m->post_indent = -1;
>>          for (i = split + 1; i < xdf->nrec; i++) {
>> -               m->post_indent = get_indent(xdf->recs[i]);
>> +               m->post_indent = get_indent(&xdf->recs[i]);
>>                  if (m->post_indent != -1)
>>                          break;
>>                  m->post_blank += 1;
>> @@ -752,7 +752,7 @@ static inline int group_previous(xdfile_t *xdf, struct xdlgroup *g)
>>   static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g)
>>   {
>>          if (g->end < xdf->nrec &&
>> -           recs_match(xdf->recs[g->start], xdf->recs[g->end])) {
>> +           recs_match(&xdf->recs[g->start], &xdf->recs[g->end])) {
>>                  xdf->rchg[g->start++] = 0;
>>                  xdf->rchg[g->end++] = 1;
>>
>> @@ -773,7 +773,7 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g)
>>   static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g)
>>   {
>>          if (g->start > 0 &&
>> -           recs_match(xdf->recs[g->start - 1], xdf->recs[g->end - 1])) {
>> +           recs_match(&xdf->recs[g->start - 1], &xdf->recs[g->end - 1])) {
>>                  xdf->rchg[--g->start] = 1;
>>                  xdf->rchg[--g->end] = 0;
>>
>> @@ -988,16 +988,16 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)
>>
>>          for (xch = xscr; xch; xch = xch->next) {
>>                  int ignore = 1;
>> -               xrecord_t **rec;
>> +               xrecord_t *rec;
>>                  long i;
>>
>>                  rec = &xe->xdf1.recs[xch->i1];
>>                  for (i = 0; i < xch->chg1 && ignore; i++)
>> -                       ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags);
>> +                       ignore = xdl_blankline(rec[i].ptr, rec[i].size, flags);
>>
>>                  rec = &xe->xdf2.recs[xch->i2];
>>                  for (i = 0; i < xch->chg2 && ignore; i++)
>> -                       ignore = xdl_blankline(rec[i]->ptr, rec[i]->size, flags);
>> +                       ignore = xdl_blankline(rec[i].ptr, rec[i].size, flags);
>>
>>                  xch->ignore = ignore;
>>          }
>> @@ -1021,7 +1021,7 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,
>>          xdchange_t *xch;
>>
>>          for (xch = xscr; xch; xch = xch->next) {
>> -               xrecord_t **rec;
>> +               xrecord_t *rec;
>>                  int ignore = 1;
>>                  long i;
>>
>> @@ -1033,11 +1033,11 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,
>>
>>                  rec = &xe->xdf1.recs[xch->i1];
>>                  for (i = 0; i < xch->chg1 && ignore; i++)
>> -                       ignore = record_matches_regex(rec[i], xpp);
>> +                       ignore = record_matches_regex(&rec[i], xpp);
>>
>>                  rec = &xe->xdf2.recs[xch->i2];
>>                  for (i = 0; i < xch->chg2 && ignore; i++)
>> -                       ignore = record_matches_regex(rec[i], xpp);
>> +                       ignore = record_matches_regex(&rec[i], xpp);
>>
>>                  xch->ignore = ignore;
>>          }
>> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
>> index 2161ac3cd0..b2f1f30cd3 100644
>> --- a/xdiff/xemit.c
>> +++ b/xdiff/xemit.c
>> @@ -25,7 +25,7 @@
>>
>>   static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb)
>>   {
>> -       xrecord_t *rec = xdf->recs[ri];
>> +       xrecord_t *rec = &xdf->recs[ri];
>>
>>          if (xdl_emit_diffrec(rec->ptr, rec->size, pre, strlen(pre), ecb) < 0)
>>                  return -1;
>> @@ -110,7 +110,7 @@ static long def_ff(const char *rec, long len, char *buf, long sz)
>>   static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri,
>>                             char *buf, long sz)
>>   {
>> -       xrecord_t *rec = xdf->recs[ri];
>> +       xrecord_t *rec = &xdf->recs[ri];
>>
>>          if (!xecfg->find_func)
>>                  return def_ff(rec->ptr, rec->size, buf, sz);
>> @@ -150,7 +150,7 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
>>
>>   static int is_empty_rec(xdfile_t *xdf, long ri)
>>   {
>> -       xrecord_t *rec = xdf->recs[ri];
>> +       xrecord_t *rec = &xdf->recs[ri];
>>          long i = 0;
>>
>>          for (; i < rec->size && XDL_ISSPACE(rec->ptr[i]); i++);
>> diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
>> index 040d81e0bc..4d857e8ae2 100644
>> --- a/xdiff/xhistogram.c
>> +++ b/xdiff/xhistogram.c
>> @@ -86,7 +86,7 @@ struct region {
>>          ((LINE_MAP(index, ptr))->cnt)
>>
>>   #define REC(env, s, l) \
>> -       (env->xdf##s.recs[l - 1])
>> +       (&env->xdf##s.recs[l - 1])
>>
>>   static int cmp_recs(xrecord_t *r1, xrecord_t *r2)
>>   {
>> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
>> index af40c88a5b..fd600cbb5d 100644
>> --- a/xdiff/xmerge.c
>> +++ b/xdiff/xmerge.c
>> @@ -97,12 +97,12 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,
>>                  int line_count, long flags)
>>   {
>>          int i;
>> -       xrecord_t **rec1 = xe1->xdf2.recs + i1;
>> -       xrecord_t **rec2 = xe2->xdf2.recs + i2;
>> +       xrecord_t *rec1 = xe1->xdf2.recs + i1;
>> +       xrecord_t *rec2 = xe2->xdf2.recs + i2;
>>
>>          for (i = 0; i < line_count; i++) {
>> -               int result = xdl_recmatch(rec1[i]->ptr, rec1[i]->size,
>> -                       rec2[i]->ptr, rec2[i]->size, flags);
>> +               int result = xdl_recmatch(rec1[i].ptr, rec1[i].size,
>> +                       rec2[i].ptr, rec2[i].size, flags);
>>                  if (!result)
>>                          return -1;
>>          }
>> @@ -111,7 +111,7 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,
>>
>>   static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int needs_cr, int add_nl, char *dest)
>>   {
>> -       xrecord_t **recs;
>> +       xrecord_t *recs;
>>          int size = 0;
>>
>>          recs = (use_orig ? xe->xdf1.recs : xe->xdf2.recs) + i;
>> @@ -119,12 +119,12 @@ static int xdl_recs_copy_0(int use_orig, xdfenv_t *xe, int i, int count, int nee
>>          if (count < 1)
>>                  return 0;
>>
>> -       for (i = 0; i < count; size += recs[i++]->size)
>> +       for (i = 0; i < count; size += recs[i++].size)
>>                  if (dest)
>> -                       memcpy(dest + size, recs[i]->ptr, recs[i]->size);
>> +                       memcpy(dest + size, recs[i].ptr, recs[i].size);
>>          if (add_nl) {
>> -               i = recs[count - 1]->size;
>> -               if (i == 0 || recs[count - 1]->ptr[i - 1] != '\n') {
>> +               i = recs[count - 1].size;
>> +               if (i == 0 || recs[count - 1].ptr[i - 1] != '\n') {
>>                          if (needs_cr) {
>>                                  if (dest)
>>                                          dest[size] = '\r';
>> @@ -160,22 +160,22 @@ static int is_eol_crlf(xdfile_t *file, int i)
>>
>>          if (i < file->nrec - 1)
>>                  /* All lines before the last *must* end in LF */
>> -               return (size = file->recs[i]->size) > 1 &&
>> -                       file->recs[i]->ptr[size - 2] == '\r';
>> +               return (size = file->recs[i].size) > 1 &&
>> +                       file->recs[i].ptr[size - 2] == '\r';
>>          if (!file->nrec)
>>                  /* Cannot determine eol style from empty file */
>>                  return -1;
>> -       if ((size = file->recs[i]->size) &&
>> -                       file->recs[i]->ptr[size - 1] == '\n')
>> +       if ((size = file->recs[i].size) &&
>> +                       file->recs[i].ptr[size - 1] == '\n')
>>                  /* Last line; ends in LF; Is it CR/LF? */
>>                  return size > 1 &&
>> -                       file->recs[i]->ptr[size - 2] == '\r';
>> +                       file->recs[i].ptr[size - 2] == '\r';
>>          if (!i)
>>                  /* The only line has no eol */
>>                  return -1;
>>          /* Determine eol from second-to-last line */
>> -       return (size = file->recs[i - 1]->size) > 1 &&
>> -               file->recs[i - 1]->ptr[size - 2] == '\r';
>> +       return (size = file->recs[i - 1].size) > 1 &&
>> +               file->recs[i - 1].ptr[size - 2] == '\r';
>>   }
>>
>>   static int is_cr_needed(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m)
>> @@ -334,22 +334,22 @@ static int recmatch(xrecord_t *rec1, xrecord_t *rec2, unsigned long flags)
>>   static void xdl_refine_zdiff3_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
>>                  xpparam_t const *xpp)
>>   {
>> -       xrecord_t **rec1 = xe1->xdf2.recs, **rec2 = xe2->xdf2.recs;
>> +       xrecord_t *rec1 = xe1->xdf2.recs, *rec2 = xe2->xdf2.recs;
>>          for (; m; m = m->next) {
>>                  /* let's handle just the conflicts */
>>                  if (m->mode)
>>                          continue;
>>
>>                  while(m->chg1 && m->chg2 &&
>> -                     recmatch(rec1[m->i1], rec2[m->i2], xpp->flags)) {
>> +                     recmatch(&rec1[m->i1], &rec2[m->i2], xpp->flags)) {
>>                          m->chg1--;
>>                          m->chg2--;
>>                          m->i1++;
>>                          m->i2++;
>>                  }
>>                  while (m->chg1 && m->chg2 &&
>> -                      recmatch(rec1[m->i1 + m->chg1 - 1],
>> -                               rec2[m->i2 + m->chg2 - 1], xpp->flags)) {
>> +                      recmatch(&rec1[m->i1 + m->chg1 - 1],
>> +                               &rec2[m->i2 + m->chg2 - 1], xpp->flags)) {
>>                          m->chg1--;
>>                          m->chg2--;
>>                  }
>> @@ -381,12 +381,12 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
>>                   * This probably does not work outside git, since
>>                   * we have a very simple mmfile structure.
>>                   */
>> -               t1.ptr = (char *)xe1->xdf2.recs[m->i1]->ptr;
>> -               t1.size = xe1->xdf2.recs[m->i1 + m->chg1 - 1]->ptr
>> -                       + xe1->xdf2.recs[m->i1 + m->chg1 - 1]->size - t1.ptr;
>> -               t2.ptr = (char *)xe2->xdf2.recs[m->i2]->ptr;
>> -               t2.size = xe2->xdf2.recs[m->i2 + m->chg2 - 1]->ptr
>> -                       + xe2->xdf2.recs[m->i2 + m->chg2 - 1]->size - t2.ptr;
>> +               t1.ptr = (char *)xe1->xdf2.recs[m->i1].ptr;
>> +               t1.size = xe1->xdf2.recs[m->i1 + m->chg1 - 1].ptr
>> +                       + xe1->xdf2.recs[m->i1 + m->chg1 - 1].size - t1.ptr;
>> +               t2.ptr = (char *)xe2->xdf2.recs[m->i2].ptr;
>> +               t2.size = xe2->xdf2.recs[m->i2 + m->chg2 - 1].ptr
>> +                       + xe2->xdf2.recs[m->i2 + m->chg2 - 1].size - t2.ptr;
>>                  if (xdl_do_diff(&t1, &t2, xpp, &xe) < 0)
>>                          return -1;
>>                  if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
>> @@ -440,8 +440,8 @@ static int line_contains_alnum(const char *ptr, long size)
>>   static int lines_contain_alnum(xdfenv_t *xe, int i, int chg)
>>   {
>>          for (; chg; chg--, i++)
>> -               if (line_contains_alnum(xe->xdf2.recs[i]->ptr,
>> -                               xe->xdf2.recs[i]->size))
>> +               if (line_contains_alnum(xe->xdf2.recs[i].ptr,
>> +                               xe->xdf2.recs[i].size))
>>                          return 1;
>>          return 0;
>>   }
>> diff --git a/xdiff/xpatience.c b/xdiff/xpatience.c
>> index 77dc411d19..bf69a58527 100644
>> --- a/xdiff/xpatience.c
>> +++ b/xdiff/xpatience.c
>> @@ -88,9 +88,9 @@ static int is_anchor(xpparam_t const *xpp, const char *line)
>>   static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
>>                            int pass)
>>   {
>> -       xrecord_t **records = pass == 1 ?
>> +       xrecord_t *records = pass == 1 ?
>>                  map->env->xdf1.recs : map->env->xdf2.recs;
>> -       xrecord_t *record = records[line - 1];
>> +       xrecord_t *record = &records[line - 1];
>>          /*
>>           * After xdl_prepare_env() (or more precisely, due to
>>           * xdl_classify_record()), the "ha" member of the records (AKA lines)
>> @@ -121,7 +121,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,
>>                  return;
>>          map->entries[index].line1 = line;
>>          map->entries[index].hash = record->ha;
>> -       map->entries[index].anchor = is_anchor(xpp, map->env->xdf1.recs[line - 1]->ptr);
>> +       map->entries[index].anchor = is_anchor(xpp, map->env->xdf1.recs[line - 1].ptr);
>>          if (!map->first)
>>                  map->first = map->entries + index;
>>          if (map->last) {
>> @@ -246,8 +246,8 @@ static int find_longest_common_sequence(struct hashmap *map, struct entry **res)
>>
>>   static int match(struct hashmap *map, int line1, int line2)
>>   {
>> -       xrecord_t *record1 = map->env->xdf1.recs[line1 - 1];
>> -       xrecord_t *record2 = map->env->xdf2.recs[line2 - 1];
>> +       xrecord_t *record1 = &map->env->xdf1.recs[line1 - 1];
>> +       xrecord_t *record2 = &map->env->xdf2.recs[line2 - 1];
>>          return record1->ha == record2->ha;
>>   }
>>
>> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
>> index 6f1d4b4725..92f9845003 100644
>> --- a/xdiff/xprepare.c
>> +++ b/xdiff/xprepare.c
>> @@ -131,7 +131,6 @@ static void xdl_free_ctx(xdfile_t *xdf)
>>          xdl_free(xdf->rindex);
>>          xdl_free(xdf->rchg - 1);
>>          xdl_free(xdf->recs);
>> -       xdl_cha_free(&xdf->rcha);
>>   }
>>
>>
>> @@ -146,8 +145,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
>>          xdf->rchg = NULL;
>>          xdf->recs = NULL;
>>
>> -       if (xdl_cha_init(&xdf->rcha, sizeof(xrecord_t), narec / 4 + 1) < 0)
>> -               goto abort;
>>          if (!XDL_ALLOC_ARRAY(xdf->recs, narec))
>>                  goto abort;
>>
>> @@ -158,12 +155,10 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
>>                          hav = xdl_hash_record(&cur, top, xpp->flags);
>>                          if (XDL_ALLOC_GROW(xdf->recs, xdf->nrec + 1, narec))
>>                                  goto abort;
>> -                       if (!(crec = xdl_cha_alloc(&xdf->rcha)))
>> -                               goto abort;
>> +                       crec = &xdf->recs[xdf->nrec++];
>>                          crec->ptr = prev;
>>                          crec->size = (long) (cur - prev);
>>                          crec->ha = hav;
>> -                       xdf->recs[xdf->nrec++] = crec;
>>                          if (xdl_classify_record(pass, cf, crec) < 0)
>>                                  goto abort;
>>                  }
>> @@ -263,7 +258,7 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) {
>>    */
>>   static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2) {
>>          long i, nm, nreff, mlim;
>> -       xrecord_t **recs;
>> +       xrecord_t *recs;
>>          xdlclass_t *rcrec;
>>          char *dis, *dis1, *dis2;
>>          int need_min = !!(cf->flags & XDF_NEED_MINIMAL);
>> @@ -276,7 +271,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>>          if ((mlim = xdl_bogosqrt(xdf1->nrec)) > XDL_MAX_EQLIMIT)
>>                  mlim = XDL_MAX_EQLIMIT;
>>          for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {
>> -               rcrec = cf->rcrecs[(*recs)->ha];
>> +               rcrec = cf->rcrecs[recs->ha];
>>                  nm = rcrec ? rcrec->len2 : 0;
>>                  dis1[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1;
>>          }
>> @@ -284,7 +279,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>>          if ((mlim = xdl_bogosqrt(xdf2->nrec)) > XDL_MAX_EQLIMIT)
>>                  mlim = XDL_MAX_EQLIMIT;
>>          for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {
>> -               rcrec = cf->rcrecs[(*recs)->ha];
>> +               rcrec = cf->rcrecs[recs->ha];
>>                  nm = rcrec ? rcrec->len1 : 0;
>>                  dis2[i] = (nm == 0) ? 0: (nm >= mlim && !need_min) ? 2: 1;
>>          }
>> @@ -320,13 +315,13 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
>>    */
>>   static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {
>>          long i, lim;
>> -       xrecord_t **recs1, **recs2;
>> +       xrecord_t *recs1, *recs2;
>>
>>          recs1 = xdf1->recs;
>>          recs2 = xdf2->recs;
>>          for (i = 0, lim = XDL_MIN(xdf1->nrec, xdf2->nrec); i < lim;
>>               i++, recs1++, recs2++)
>> -               if ((*recs1)->ha != (*recs2)->ha)
>> +               if (recs1->ha != recs2->ha)
>>                          break;
>>
>>          xdf1->dstart = xdf2->dstart = i;
>> @@ -334,7 +329,7 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) {
>>          recs1 = xdf1->recs + xdf1->nrec - 1;
>>          recs2 = xdf2->recs + xdf2->nrec - 1;
>>          for (lim -= i, i = 0; i < lim; i++, recs1--, recs2--)
>> -               if ((*recs1)->ha != (*recs2)->ha)
>> +               if (recs1->ha != recs2->ha)
>>                          break;
>>
>>          xdf1->dend = xdf1->nrec - i - 1;
>> diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h
>> index 85848f1685..3d26cbf1ec 100644
>> --- a/xdiff/xtypes.h
>> +++ b/xdiff/xtypes.h
>> @@ -45,10 +45,9 @@ typedef struct s_xrecord {
>>   } xrecord_t;
>>
>>   typedef struct s_xdfile {
>> -       chastore_t rcha;
>> +       xrecord_t *recs;
>>          long nrec;
>>          long dstart, dend;
>> -       xrecord_t **recs;
>>          char *rchg;
>>          long *rindex;
>>          long nreff;
>> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
>> index 444a108f87..332982b509 100644
>> --- a/xdiff/xutils.c
>> +++ b/xdiff/xutils.c
>> @@ -416,12 +416,12 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp,
>>          mmfile_t subfile1, subfile2;
>>          xdfenv_t env;
>>
>> -       subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1]->ptr;
>> -       subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2]->ptr +
>> -               diff_env->xdf1.recs[line1 + count1 - 2]->size - subfile1.ptr;
>> -       subfile2.ptr = (char *)diff_env->xdf2.recs[line2 - 1]->ptr;
>> -       subfile2.size = diff_env->xdf2.recs[line2 + count2 - 2]->ptr +
>> -               diff_env->xdf2.recs[line2 + count2 - 2]->size - subfile2.ptr;
>> +       subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1].ptr;
>> +       subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2].ptr +
>> +               diff_env->xdf1.recs[line1 + count1 - 2].size - subfile1.ptr;
>> +       subfile2.ptr = (char *)diff_env->xdf2.recs[line2 - 1].ptr;
>> +       subfile2.size = diff_env->xdf2.recs[line2 + count2 - 2].ptr +
>> +               diff_env->xdf2.recs[line2 + count2 - 2].size - subfile2.ptr;
>>          if (xdl_do_diff(&subfile1, &subfile2, xpp, &env) < 0)
>>                  return -1;
>>
>> --
>> gitgitgadget
> 
> You weren't kidding with the --color-words callout; there's an awful
> lot of places where you only change one or two characters (e.g. '->'
> becoming '.'); that's much easier to see when viewing the diff with
> that flag.
> 
> Anyway, looks good.
> 


  reply	other threads:[~2025-09-09 13:50 UTC|newest]

Thread overview: 158+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-07 19:45 [PATCH 00/17] Use rust types in xdiff Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 01/17] xdiff: delete static forward declarations in xprepare Ezekiel Newren via GitGitGadget
2025-09-09  8:55   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 02/17] xdiff: delete local variables and initialize/free xdfile_t directly Ezekiel Newren via GitGitGadget
2025-09-09  8:56   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 03/17] xdiff: delete unnecessary fields from xrecord_t and xdfile_t Ezekiel Newren via GitGitGadget
2025-09-09  8:56   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 04/17] xdiff: delete xdl_get_rec() in xemit Ezekiel Newren via GitGitGadget
2025-09-09  8:56   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 05/17] xdiff: delete struct diffdata_t Ezekiel Newren via GitGitGadget
2025-09-09  8:56   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 06/17] xdiff: delete redundant array xdfile_t.ha Ezekiel Newren via GitGitGadget
2025-09-09  8:57   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 07/17] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t Ezekiel Newren via GitGitGadget
2025-09-09  8:57   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 08/17] xdiff: delete chastore from xdfile_t, view with --color-words Ezekiel Newren via GitGitGadget
2025-09-09  8:58   ` Elijah Newren
2025-09-09 13:50     ` Phillip Wood [this message]
2025-09-09 20:33     ` Junio C Hamano
2025-09-10 22:02     ` Ben Knoble
2025-09-07 19:45 ` [PATCH 09/17] xdiff: treat xdfile_t.rchg like an enum Ezekiel Newren via GitGitGadget
2025-09-09  8:58   ` Elijah Newren
2025-09-07 19:45 ` [PATCH 10/17] compat/rust_types.h: define rust primitive types Ezekiel Newren via GitGitGadget
2025-09-08 15:08   ` Junio C Hamano
2025-09-08 16:15     ` Ezekiel Newren
2025-09-07 19:45 ` [PATCH 11/17] xdiff: include compat/rust_types.h Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 12/17] xdiff: make xrecord_t.ptr a u8 instead of char Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 13/17] xdiff: make xrecord_t.size a usize instead of long Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 14/17] xdiff: split xrecord_t.ha into line_hash and minimal_perfect_hash Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 15/17] xdiff: make xdfile_t.nrec a usize instead of long Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 16/17] xdiff: make xdfile_t.nreff " Ezekiel Newren via GitGitGadget
2025-09-07 19:45 ` [PATCH 17/17] xdiff: change the types of dstart, dend, rchg, and rindex in xdfile_t Ezekiel Newren via GitGitGadget
2025-09-16 21:56 ` [PATCH 00/17] Use rust types in xdiff Junio C Hamano
2025-09-16 22:01   ` Ezekiel Newren
2025-09-17  2:16     ` Elijah Newren
2025-09-17 13:53       ` Junio C Hamano
2025-09-17  6:22     ` Junio C Hamano
2025-09-18 23:56 ` [PATCH v2 00/10] " Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 01/10] xdiff: delete static forward declarations in xprepare Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 02/10] xdiff: delete local variables and initialize/free xdfile_t directly Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 03/10] xdiff: delete unnecessary fields from xrecord_t and xdfile_t Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 04/10] xdiff: delete xdl_get_rec() in xemit Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 05/10] xdiff: delete struct diffdata_t Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 06/10] xdiff: delete redundant array xdfile_t.ha Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 07/10] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 08/10] xdiff: delete chastore from xdfile_t Ezekiel Newren via GitGitGadget
2025-09-18 23:56   ` [PATCH v2 10/10] xdiff: treat xdfile_t.rchg like an enum Ezekiel Newren via GitGitGadget
2025-09-19  0:33   ` [PATCH v2 00/10] Use rust types in xdiff Junio C Hamano
2025-09-19  0:41     ` Ezekiel Newren
2025-09-19 15:15     ` Ezekiel Newren
2025-09-19 15:16   ` [PATCH v3 00/10] Cleanup xdfile_t and xrecord_t " Ezekiel Newren via GitGitGadget
2025-09-19 15:16     ` [PATCH v3 01/10] xdiff: delete static forward declarations in xprepare Ezekiel Newren via GitGitGadget
2025-09-20 17:16       ` Junio C Hamano
2025-09-20 17:41         ` Ezekiel Newren
2025-09-20 18:31           ` Elijah Newren
2025-09-20 22:25             ` Ben Knoble
2025-09-20 22:43             ` Junio C Hamano
2025-09-20 17:46         ` Ben Knoble
2025-09-20 18:46           ` Jeff King
2025-09-20 22:25             ` Ben Knoble
2025-09-20 22:52             ` Junio C Hamano
2025-09-20 23:15               ` Jeff King
2025-09-19 15:16     ` [PATCH v3 02/10] xdiff: delete local variables and initialize/free xdfile_t directly Ezekiel Newren via GitGitGadget
2025-09-20 17:36       ` Junio C Hamano
2025-09-19 15:16     ` [PATCH v3 03/10] xdiff: delete unnecessary fields from xrecord_t and xdfile_t Ezekiel Newren via GitGitGadget
2025-09-19 15:16     ` [PATCH v3 04/10] xdiff: delete xdl_get_rec() in xemit Ezekiel Newren via GitGitGadget
2025-09-20 17:48       ` Junio C Hamano
2025-09-21 13:06       ` Phillip Wood
2025-09-21 15:07         ` Ezekiel Newren
2025-09-19 15:16     ` [PATCH v3 05/10] xdiff: delete struct diffdata_t Ezekiel Newren via GitGitGadget
2025-09-21 13:06       ` Phillip Wood
2025-09-21 16:03         ` Ezekiel Newren
2025-09-19 15:16     ` [PATCH v3 06/10] xdiff: delete redundant array xdfile_t.ha Ezekiel Newren via GitGitGadget
2025-09-19 15:16     ` [PATCH v3 07/10] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t Ezekiel Newren via GitGitGadget
2025-09-21 13:06       ` Phillip Wood
2025-09-21 16:07         ` Ezekiel Newren
2025-09-19 15:16     ` [PATCH v3 08/10] xdiff: delete chastore from xdfile_t Ezekiel Newren via GitGitGadget
2025-09-19 15:16     ` [PATCH v3 09/10] xdiff: delete rchg aliasing Ezekiel Newren via GitGitGadget
2025-09-21 13:07       ` Phillip Wood
2025-09-21 16:37         ` Ezekiel Newren
2025-09-19 15:16     ` [PATCH v3 10/10] xdiff: treat xdfile_t.rchg like an enum Ezekiel Newren via GitGitGadget
2025-09-21  0:00       ` Junio C Hamano
2025-09-21  0:38         ` Ezekiel Newren
2025-09-21  9:19           ` Phillip Wood
2025-09-21 16:11             ` Ezekiel Newren
2025-09-19 23:30     ` [PATCH v3 00/10] Cleanup xdfile_t and xrecord_t in xdiff Elijah Newren
2025-09-19 23:37       ` Ezekiel Newren
2025-09-22 19:51     ` [PATCH v4 00/12] " Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 01/12] xdiff: delete static forward declarations in xprepare Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 02/12] xdiff: delete local variables and initialize/free xdfile_t directly Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 03/12] xdiff: delete unnecessary fields from xrecord_t and xdfile_t Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 04/12] xdiff: delete superfluous function xdl_get_rec() in xemit Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 05/12] xdiff: delete superfluous local variables that alias fields in xrecord_t Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 06/12] xdiff: delete struct diffdata_t Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 07/12] xdiff: delete redundant array xdfile_t.ha Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 08/12] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 09/12] xdiff: delete chastore from xdfile_t Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 10/12] xdiff: delete rchg aliasing Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 11/12] xdiff: use bool literals for xdfile_t.rchg Ezekiel Newren via GitGitGadget
2025-09-22 19:51       ` [PATCH v4 12/12] xdiff: refactor 'char *rchg' to 'bool *changed' in xdfile_t Ezekiel Newren via GitGitGadget
2025-09-22 22:39       ` [PATCH v4 00/12] Cleanup xdfile_t and xrecord_t in xdiff Junio C Hamano
2025-09-23  0:13         ` Ezekiel Newren
2025-09-23  1:06           ` Junio C Hamano
2025-09-23  1:30             ` Ezekiel Newren
2025-09-23 14:12               ` Junio C Hamano
2025-09-23 16:50                 ` Ezekiel Newren
2025-09-23 21:24       ` [PATCH v5 00/13] " Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 01/13] xdiff: delete static forward declarations in xprepare Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 02/13] xdiff: delete local variables and initialize/free xdfile_t directly Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 03/13] xdiff: delete unnecessary fields from xrecord_t and xdfile_t Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 04/13] xdiff: delete superfluous function xdl_get_rec() in xemit Ezekiel Newren via GitGitGadget
2025-09-30 13:31           ` Kristoffer Haugsbakk
2025-09-30 19:35             ` Ezekiel Newren
2025-09-30 20:05               ` Junio C Hamano
2025-09-23 21:24         ` [PATCH v5 05/13] xdiff: delete superfluous local variables that alias fields in xrecord_t Ezekiel Newren via GitGitGadget
2025-09-24 10:22           ` Phillip Wood
2025-09-24 14:52             ` Ezekiel Newren
2025-09-23 21:24         ` [PATCH v5 06/13] xdiff: delete struct diffdata_t Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 07/13] xdiff: delete redundant array xdfile_t.ha Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 08/13] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 09/13] xdiff: delete chastore from xdfile_t Ezekiel Newren via GitGitGadget
2025-09-23 21:24         ` [PATCH v5 10/13] xdiff: delete rchg aliasing Ezekiel Newren via GitGitGadget
2025-09-24 10:22           ` Phillip Wood
2025-09-24 15:01             ` Ezekiel Newren
2025-09-24 15:34               ` Junio C Hamano
2025-09-24 15:58                 ` Ezekiel Newren
2025-09-24 21:31                   ` Junio C Hamano
2025-09-24 22:46                     ` Ezekiel Newren
2025-09-25  7:09                       ` Junio C Hamano
2025-09-25 22:02                         ` Ezekiel Newren
2025-09-23 21:24         ` [PATCH v5 11/13] xdiff: rename rchg -> changed in xdfile_t Ezekiel Newren via GitGitGadget
2025-09-24 10:22           ` Phillip Wood
2025-09-24 15:10             ` Ezekiel Newren
2025-09-24 15:18               ` Phillip Wood
2025-09-23 21:24         ` [PATCH v5 12/13] xdiff: use enum macros NONE(0), SOME(1), TOO_MANY(2) in xprepare.c Ezekiel Newren via GitGitGadget
2025-09-24 10:21           ` Phillip Wood
2025-09-24 14:46             ` Ezekiel Newren
2025-09-24 15:18               ` Phillip Wood
2025-09-24 17:29                 ` Junio C Hamano
2025-09-25 18:40                 ` Ezekiel Newren
2025-09-26  2:29                   ` Ezekiel Newren
2025-09-23 21:24         ` [PATCH v5 13/13] xdiff: change type of xdfile_t.changed from char to bool Ezekiel Newren via GitGitGadget
2025-09-24 10:21           ` Phillip Wood
2025-09-24 15:14             ` Ezekiel Newren
2025-09-26 22:41         ` [PATCH v6 00/12] Cleanup xdfile_t and xrecord_t in xdiff Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 01/12] xdiff: delete static forward declarations in xprepare Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 02/12] xdiff: delete local variables and initialize/free xdfile_t directly Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 03/12] xdiff: delete unnecessary fields from xrecord_t and xdfile_t Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 04/12] xdiff: delete superfluous function xdl_get_rec() in xemit Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 05/12] xdiff: delete local variables that alias fields in xrecord_t Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 06/12] xdiff: delete struct diffdata_t Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 07/12] xdiff: delete redundant array xdfile_t.ha Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 08/12] xdiff: delete fields ha, line, size in xdlclass_t in favor of an xrecord_t Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 09/12] xdiff: delete chastore from xdfile_t Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 10/12] xdiff: rename rchg -> changed in xdfile_t Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 11/12] xdiff: add macros DISCARD(0), KEEP(1), INVESTIGATE(2) in xprepare.c Ezekiel Newren via GitGitGadget
2025-09-26 22:41           ` [PATCH v6 12/12] xdiff: change type of xdfile_t.changed from char to bool Ezekiel Newren via GitGitGadget
2025-10-03 13:47           ` [PATCH v6 00/12] Cleanup xdfile_t and xrecord_t in xdiff Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aae1ad41-8604-45bc-8ec3-03180e6152ff@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=ezekielnewren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).