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

[Phillip Wood <phillip.wood123@gmail.com>]

On 15/10/2025 22:18, Ezekiel Newren via GitGitGadget wrote:
> From: Ezekiel Newren <ezekielnewren@gmail.com>
> 
> Rust uses u8 to refer to bytes in memory. Since xrecord_t.ptr is also
> referring to bytes in memory, rather than unicode code points, use
> uint8_t instead of char.

It C "char" never refers to a unicode code point so I don't follow the 
reasoning here. Isn't the reason you want to change from "char" to 
"uint8_t" to match rust? Given "char" and "uint8_t" are the same width 
why can't we use "char" in the C struct and "u8" in the rust struct as 
the two structs would still have the same layout?

I agree with Patrick's comments on this patch - it would be nice to know 
how you decided where to add casts. Given that rust is going to be 
optional for at least a year we should take care to leave the C code in 
good shape with a minimum number of casts.

Thanks

Phillip

> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>
> ---
>   xdiff/xdiffi.c    |  8 ++++----
>   xdiff/xemit.c     |  6 +++---
>   xdiff/xmerge.c    | 14 +++++++-------
>   xdiff/xpatience.c |  2 +-
>   xdiff/xprepare.c  |  8 ++++----
>   xdiff/xtypes.h    |  2 +-
>   xdiff/xutils.c    |  4 ++--
>   7 files changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
> index 6f3998ee54..411a8aa69f 100644
> --- a/xdiff/xdiffi.c
> +++ b/xdiff/xdiffi.c
> @@ -407,7 +407,7 @@ static int get_indent(xrecord_t *rec)
>   	int ret = 0;
>   
>   	for (i = 0; i < rec->size; i++) {
> -		char c = rec->ptr[i];
> +		uint8_t c = rec->ptr[i];
>   
>   		if (!XDL_ISSPACE(c))
>   			return ret;
> @@ -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;
>   	}
> @@ -1008,7 +1008,7 @@ static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {
>   	size_t i;
>   
>   	for (i = 0; i < xpp->ignore_regex_nr; i++)
> -		if (!regexec_buf(xpp->ignore_regex[i], rec->ptr, rec->size, 1,
> +		if (!regexec_buf(xpp->ignore_regex[i], (const char *)rec->ptr, rec->size, 1,
>   				 &regmatch, 0))
>   			return 1;
>   
> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> index b2f1f30cd3..ead930088a 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(rec->ptr, rec->size, pre, strlen(pre), ecb) < 0)
> +	if (xdl_emit_diffrec((char const *)rec->ptr, rec->size, pre, strlen(pre), ecb) < 0)
>   		return -1;
>   
>   	return 0;
> @@ -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(rec->ptr, rec->size, buf, sz);
> -	return xecfg->find_func(rec->ptr, rec->size, buf, sz, xecfg->find_func_priv);
> +		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);
>   }
>   
>   static int is_func_rec(xdfile_t *xdf, xdemitconf_t const *xecfg, long ri)
> diff --git a/xdiff/xmerge.c b/xdiff/xmerge.c
> index fd600cbb5d..75cb3e76a2 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(rec1[i].ptr, rec1[i].size,
> -			rec2[i].ptr, rec2[i].size, flags);
> +		int result = xdl_recmatch((const char *)rec1[i].ptr, rec1[i].size,
> +			(const char *)rec2[i].ptr, rec2[i].size, flags);
>   		if (!result)
>   			return -1;
>   	}
> @@ -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(rec1->ptr, rec1->size,
> -			    rec2->ptr, rec2->size, flags);
> +	return xdl_recmatch((const char *)rec1->ptr, rec1->size,
> +			    (const char *)rec2->ptr, rec2->size, flags);
>   }
>   
>   /*
> @@ -382,10 +382,10 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,
>   		 * 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
> +		t1.size = (char *)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
> +		t2.size = (char *)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;
> @@ -440,7 +440,7 @@ 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,
> +		if (line_contains_alnum((const char *)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 669b653580..bb61354f22 100644
> --- a/xdiff/xpatience.c
> +++ b/xdiff/xpatience.c
> @@ -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, (const char *)map->env->xdf1.recs[line - 1].ptr);
>   	if (!map->first)
>   		map->first = map->entries + index;
>   	if (map->last) {
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index 192334f1b7..4cb18b2b88 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(rcrec->rec.ptr, rcrec->rec.size,
> -					rec->ptr, rec->size, cf->flags))
> +				xdl_recmatch((const char *)rcrec->rec.ptr, rcrec->rec.size,
> +					(const char *)rec->ptr, rec->size, cf->flags))
>   			break;
>   
>   	if (!rcrec) {
> @@ -156,8 +156,8 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_
>   			if (XDL_ALLOC_GROW(xdf->recs, xdf->nrec + 1, narec))
>   				goto abort;
>   			crec = &xdf->recs[xdf->nrec++];
> -			crec->ptr = prev;
> -			crec->size = (long) (cur - prev);
> +			crec->ptr = (uint8_t const *)prev;
> +			crec->size =(long) ( cur - prev);
>   			crec->ha = hav;
>   			if (xdl_classify_record(pass, cf, crec) < 0)
>   				goto abort;
> diff --git a/xdiff/xtypes.h b/xdiff/xtypes.h
> index 3514bb1684..57983627f5 100644
> --- a/xdiff/xtypes.h
> +++ b/xdiff/xtypes.h
> @@ -39,7 +39,7 @@ typedef struct s_chastore {
>   } chastore_t;
>   
>   typedef struct s_xrecord {
> -	char const *ptr;
> +	uint8_t const *ptr;
>   	long size;
>   	unsigned long ha;
>   } xrecord_t;
> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index 447e66c719..7be063bfb6 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -465,10 +465,10 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp,
>   	xdfenv_t env;
>   
>   	subfile1.ptr = (char *)diff_env->xdf1.recs[line1 - 1].ptr;
> -	subfile1.size = diff_env->xdf1.recs[line1 + count1 - 2].ptr +
> +	subfile1.size = (char *)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 +
> +	subfile2.size = (char *)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;