Re: [PATCH v3 04/10] xdiff: use size_t for xrecord_t.size

>
> "Ezekiel Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Ezekiel Newren <ezekielnewren@gmail.com>
> >
> > size_t is the appropriate type because size is describing the number of
> > elements, bytes in this case, in memory.
> >
> > Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
> > ---
> >  xdiff/xdiffi.c   |  7 +++----
> >  xdiff/xemit.c    |  8 ++++----
> >  xdiff/xmerge.c   | 16 ++++++++--------
> >  xdiff/xprepare.c |  6 +++---
> >  xdiff/xtypes.h   |  2 +-
> >  5 files changed, 19 insertions(+), 20 deletions(-)
>
> This step looks mostly OK but it is messy in some places.
>
> > diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> > index ead930088a..2f8007753c 100644
> > --- a/xdiff/xemit.c
> > +++ b/xdiff/xemit.c
> > @@ -27,7 +27,7 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
> >  {
> >       xrecord_t *rec = &xdf->recs[ri];
> >
> > -     if (xdl_emit_diffrec((char const *)rec->ptr, rec->size, pre, strlen(pre), ecb) < 0)
> > +     if (xdl_emit_diffrec((char const *)rec->ptr, (long)rec->size, pre, strlen(pre), ecb) < 0)
>
> On platforms where long is narrower than size_t, we'd tentatively
> leave things broken until we update xdl_emit_diffrec() to take
> size_t, as it would become too noisy to change it in the same patch,
> I guess?
>
> > @@ -113,8 +113,8 @@ static long match_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri,
> >       xrecord_t *rec = &xdf->recs[ri];
> >
> >       if (!xecfg->find_func)
> > -             return def_ff((const char *)rec->ptr, rec->size, buf, sz);
> > -     return xecfg->find_func((const char *)rec->ptr, rec->size, buf, sz, xecfg->find_func_priv);
> > +             return def_ff((const char *)rec->ptr, (long)rec->size, buf, sz);
> > +     return xecfg->find_func((const char *)rec->ptr, (long)rec->size, buf, sz, xecfg->find_func_priv);
>
> Ditto.
>
> > diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> > index 75cb3e76a2..0dd4558a32 100644
> > --- a/xdiff/xmerge.c
> > +++ b/xdiff/xmerge.c
> > @@ -101,8 +101,8 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,
> >       xrecord_t *rec2 = xe2->xdf2.recs + i2;
> >
> >       for (i = 0; i < line_count; i++) {
> > -             int result = xdl_recmatch((const char *)rec1[i].ptr, rec1[i].size,
> > -                     (const char *)rec2[i].ptr, rec2[i].size, flags);
> > +             int result = xdl_recmatch((const char *)rec1[i].ptr, (long)rec1[i].size,
> > +                     (const char *)rec2[i].ptr, (long)rec2[i].size, flags);
>
> Ditto.
>
> > @@ -119,11 +119,11 @@ 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 += (int)recs[i++].size)
> >               if (dest)
> >                       memcpy(dest + size, recs[i].ptr, recs[i].size);
> >       if (add_nl) {
> > -             i = recs[count - 1].size;
> > +             i = (int)recs[count - 1].size;
> >               if (i == 0 || recs[count - 1].ptr[i - 1] != '\n') {
> >                       if (needs_cr) {
> >                               if (dest)
>
> This is messier than I expected.  Before the precontext of this
> hunk, "i" and "count" are both incoming parameters of type "int", so
> the same "what if size_t is wider?" puzzlement applies here.  At
> least, the reason why "i" and "count" is "int" is not because they
> want to be able to express negative values, so it shouldn't involve
> too much hassle if we later want to change them to size_t to lose
> these casts.
>
> > @@ -324,8 +324,8 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,
> >
> >  static int recmatch(xrecord_t *rec1, xrecord_t *rec2, unsigned long flags)
> >  {
> > -     return xdl_recmatch((const char *)rec1->ptr, rec1->size,
> > -                         (const char *)rec2->ptr, rec2->size, flags);
> > +     return xdl_recmatch((const char *)rec1->ptr, (long)rec1->size,
> > +                         (const char *)rec2->ptr, (long)rec2->size, flags);
> >  }
>
> Same "long may not be wide enough, in which case we'd need further
> fixes" applies here.
>
> > @@ -441,7 +441,7 @@ static int lines_contain_alnum(xdfenv_t *xe, int i, int chg)
> >  {
> >       for (; chg; chg--, i++)
> >               if (line_contains_alnum((const char *)xe->xdf2.recs[i].ptr,
> > -                             xe->xdf2.recs[i].size))
> > +                             (long)xe->xdf2.recs[i].size))
> >                       return 1;
> >       return 0;
> >  }
>
> Ditto.
>
> > diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> > index 4c56467076..b3219aed3e 100644
> > --- a/xdiff/xprepare.c
> > +++ b/xdiff/xprepare.c
> > @@ -99,8 +99,8 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t
> >       hi = (long) XDL_HASHLONG(rec->ha, cf->hbits);
> >       for (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)
> >               if (rcrec->rec.ha == rec->ha &&
> > -                             xdl_recmatch((const char *)rcrec->rec.ptr, rcrec->rec.size,
> > -                                     (const char *)rec->ptr, rec->size, cf->flags))
> > +                             xdl_recmatch((const char *)rcrec->rec.ptr, (long)rcrec->rec.size,
> > +                                     (const char *)rec->ptr, (long)rec->size, cf->flags))
>
> Ditto.

> > @@ -157,7 +157,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
> >                               goto abort;
> >                       crec = &xdf->recs[xdf->nrec++];
> >                       crec->ptr = (uint8_t const *)prev;
> > -                     crec->size = (long) (cur - prev);
> > +                     crec->size = cur - prev;
>
> Yay!
>
> > diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h
> > index b1c520a378..88b1fe4649 100644
> > --- a/xdiff/xtypes.h
> > +++ b/xdiff/xtypes.h
> > @@ -40,7 +40,7 @@ typedef struct s_chastore {
> >
> >  typedef struct s_xrecord {
> >       uint8_t const *ptr;
> > -     long size;
> > +     size_t size;
>
> Yay, too!
>
> >       unsigned long ha;
> >  } xrecord_t;