Re: [PATCH 02/10] xdiff: make classic diff explicit by creating xdl_do_classic_diff()

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jan 20, 2026, 15:01 UTC
Message-ID: <1c46f551-0040-481e-9476-bc1b85f92636@gmail.com>
In-Reply-To: <9bd01bce9f0763d9dcc962ff94fcda36346bafc4.1767379944.git.gitgitgadget@gmail.com>

On 02/01/2026 18:52, Ezekiel Newren via GitGitGadget wrote:

> From: Ezekiel Newren <ezekielnewren@gmail.com>
> 
> Later patches will prepare xdl_cleanup_records() to be moved into xdiffi.c
> since only the classic diff uses that function.

I assume that's to make it easier to covert the myers implementation to rust without affecting the rest of the code? If so it would be nice to say that.

> Signed-off-by: Ezekiel Newren <ezekielnewren@gmail.com>

Show 18 quoted lines

> +int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
> +		xdfenv_t *xe) {
> +	int res;
> +
> +	if (xdl_prepare_env(mf1, mf2, xpp, xe) < 0)
> +		return -1;
> +
> +	if (XDF_DIFF_ALG(xpp->flags) == XDF_PATIENCE_DIFF) {
> +		res = xdl_do_patience_diff(xpp, xe);
> +		goto out;
> +	}
> +
> +	if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF) {
> +		res = xdl_do_histogram_diff(xpp, xe);
> +		goto out;
> +	}
> +
> +	res = xdl_do_classic_diff(xe, xpp->flags);

This might be clearer that we're calling only one of the three functions if we wrote this as

	if (XDF_DIFF_ALG(xpp->flags) == XDIF_PATIENCE_DIFF)
		res = xdl_do_patience_diff(xpp, xe);
	else if (XDF_DIFF_ALG(xpp->flags) == XDF_HISTOGRAM_DIFF)
		res = xdl_do_histogram_diff(xpp, xe);
	else
		res = xdl_do_classic_diff(xe, xpp->flags);

and then we can drop the out: label

Thanks

Phillip

Show 15 quoted lines

>    out:
>   	if (res < 0)
>   		xdl_free_env(xe);
> diff --git a/xdiff/xdiffi.h b/xdiff/xdiffi.h
> index 49e52c67f9..8bf4c20373 100644
> --- a/xdiff/xdiffi.h
> +++ b/xdiff/xdiffi.h
> @@ -42,6 +42,7 @@ typedef struct s_xdchange {
>   int xdl_recs_cmp(xdfile_t *xdf1, long off1, long lim1,
>   		 xdfile_t *xdf2, long off2, long lim2,
>   		 long *kvdf, long *kvdb, int need_min, xdalgoenv_t *xenv);
> +int xdl_do_classic_diff(xdfenv_t *xe, uint64_t flags);
>   int xdl_do_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
>   		xdfenv_t *xe);
>   int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags);