Re: [PATCH 2/9] xdiff: make xrecord_t.ptr a uint8_t instead of char

From: Ezekiel Newren <ezekielnewren@gmail.com>
Date: Oct 22, 2025, 21:12 UTC
Message-ID: <CAH=ZcbDOY2yDQbBJeoKHesZzZCBvscqf7SoqbX4j3oHCBY5p8g@mail.gmail.com>
In-Reply-To: <aPdFZp8GokGoshol@pks.im>

On Tue, Oct 21, 2025 at 2:33 AM Patrick Steinhardt <ps@pks.im> wrote:

Show 25 quoted lines

>
> On Wed, Oct 15, 2025 at 09:18:14PM +0000, Ezekiel Newren via GitGitGadget wrote:
> > diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> > index 6f3998ee54..411a8aa69f 100644
> > --- a/xdiff/xdiffi.c
> > +++ b/xdiff/xdiffi.c
> > @@ -993,11 +993,11 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)
> >
> >               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((const char *)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((const char *)rec[i].ptr, rec[i].size, flags);
> >
> >               xch->ignore = ignore;
> >       }
>
> Okay. Seemingly, we convert the structure itself, but we don't convert
> any of the functions to accept an `uint8_t`. I guess you drew the line
> here so that we don't have to also touch up dozens of function
> signatures?

That is correct. I wanted to avoid _boiling the ocean_ just to change the type of ptr.

> And how did you end up verifying that you added all casts? Does the
> compiler flag those as warnings?

I used CLion to search for all uses of that field and then added casts where the types differ. Another way to do that is to run `make DEVELOPER=1` and address all of the `uint8_t differs in signedness from char` errors that are spat out.

> In any case, it might be nice to explain both of these details in the
> commit message.

I will update it.

Thanks.