Re: [PATCH v3 04/10] xdiff: delete xdl_get_rec() in xemit
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 21, 2025, 13:06 UTC
- Message-ID
- <cd39a321-a6c4-4126-a716-dc0b0cffa68d@gmail.com>
- In-Reply-To
- <ddfee67e06bb56cfa8f11187dc27135bfce63647.1758294992.git.gitgitgadget@gmail.com>
Hi Ezekiel
On 19/09/2025 16:16, Ezekiel Newren via GitGitGadget wrote:
> From: Ezekiel Newren <ezekielnewren@gmail.com> > > This function aliases the fields of xrecord_t, which makes it harder > to track the usages of those fields. Delete it.
Patch 6 goes the other way and introduces a getter function that hides the field accesses so I'm not sure why this one is so bad that it needs to be removed.
Thanks
Phillip
Show 71 quoted lines
> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
> ---
> xdiff/xemit.c | 38 +++++++++++++-------------------------
> 1 file changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> index 1d40c9cb40..b3793e81e2 100644
> --- a/xdiff/xemit.c
> +++ b/xdiff/xemit.c
> @@ -22,21 +22,11 @@
>
> #include "xinclude.h"
>
> -static long xdl_get_rec(xdfile_t *xdf, long ri, char const **rec) {
> -
> - *rec = xdf->recs[ri]->ptr;
> -
> - return xdf->recs[ri]->size;
> -}
> -
> -
> -static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb) {
> - long size, psize = strlen(pre);
> - char const *rec;
> -
> - size = xdl_get_rec(xdf, ri, &rec);
> - if (xdl_emit_diffrec(rec, size, pre, psize, ecb) < 0) {
> +static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *ecb)
> +{
> + xrecord_t *rec = xdf->recs[ri];
>
> + if (xdl_emit_diffrec(rec->ptr, rec->size, pre, strlen(pre), ecb) < 0) {
> return -1;
> }
>
> @@ -120,11 +110,11 @@ 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)
> {
> - const char *rec;
> - long len = xdl_get_rec(xdf, ri, &rec);
> + xrecord_t *rec = xdf->recs[ri];
> +
> if (!xecfg->find_func)
> - return def_ff(rec, len, buf, sz);
> - return xecfg->find_func(rec, len, buf, sz, xecfg->find_func_priv);
> + return def_ff(rec->ptr, rec->size, buf, sz);
> + return xecfg->find_func(rec->ptr, rec->size, buf, sz, xecfg->find_func_priv);
> }
>
> static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri)
> @@ -160,14 +150,12 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg,
>
> static int is_empty_rec(xdfile_t *xdf, long ri)
> {
> - const char *rec;
> - long len = xdl_get_rec(xdf, ri, &rec);
> + xrecord_t *rec = xdf->recs[ri];
> + long i = 0;
>
> - while (len > 0 && XDL_ISSPACE(*rec)) {
> - rec++;
> - len--;
> - }
> - return !len;
> + for (; i < rec->size && XDL_ISSPACE(rec->ptr[i]); i++);
> +
> + return i == rec->size;
> }
>
> int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,