From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sun, 21 Sep 2025 13:06:10 GMT
Subject: Re: [PATCH v3 04/10] xdiff: delete xdl_get_rec() in xemit
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

> 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,