Re: [PATCH 2/2] xdi_diff: trim common trailing lines

From: Peter Baumann <waste.manager@gmx.de>
Date: Dec 14, 2007, 09:06 UTC
Message-ID: <20071214090614.GB15610@xp.machine.xx>
In-Reply-To: <7vmysez0oa.fsf@gitster.siamese.dyndns.org>

On Thu, Dec 13, 2007 at 02:31:49PM -0800, Junio C Hamano wrote:

Show 27 quoted lines

> This implements earlier Linus's optimization to trim common lines at the
> end before passing them down to low level xdiff interface for all of our
> xdiff users.
> 
> We could later enhance this to also trim common leading lines, but that
> would need tweaking of the output function to add the number of lines
> trimmed at the beginning to line numbers that appear in the hunk
> headers.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  xdiff-interface.c |   34 +++++++++++++++++++++++++++++++++-
>  1 files changed, 33 insertions(+), 1 deletions(-)
> 
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index 69a022c..f2cd488 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -103,9 +103,41 @@ int xdiff_outf(void *priv_, mmbuffer_t *mb, int nbuf)
>  	return 0;
>  }
>  
> +/*
> + * Trim down common substring at the end of the buffers,
> + * but leave at least ctx lines at the end.
> + */
> +static void trim_common_tail(mmfile_t *a, mmfile_t *b, int ctx)

Should ctx be a long? (see comment below)

Show 29 quoted lines

> +{
> +	const int blk = 1024;
> +	long trimmed = 0, recovered = 0;
> +	int i;
> +	char *ap = a->ptr + a->size;
> +	char *bp = b->ptr + b->size;
> +	long smaller = (a->size < b->size) ? a->size : b->size;
> +
> +	while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {
> +		trimmed += blk;
> +		ap -= blk;
> +		bp -= blk;
> +	}
> +
> +	for (i = 0, recovered = 0; recovered < trimmed && i <= ctx; i++) {
> +		while (recovered < trimmed && ap[recovered] != '\n')
> +			recovered++;
> +	}
> +	a->size -= (trimmed - recovered);
> +	b->size -= (trimmed - recovered);
> +}
> +
>  int xdi_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdemitconf_t const *xecfg, xdemitcb_t *xecb)
>  {
> -	return xdl_diff(mf1, mf2, xpp, xecfg, xecb);
> +	mmfile_t a = *mf1;
> +	mmfile_t b = *mf2;
> +
> +	trim_common_tail(&a, &b, xecfg->ctxlen);

xdemitconf_t has the following definition

	typedef struct s_xdemitconf {
		long ctxlen;
	        unsigned long flags;
		find_func_t find_func;
	        void *find_func_priv;
	} xdemitconf_t;

So you are loosing some values in your trim_common_tail function by making ctx only an int. (Not sure that it matters, but I noticed it while glancing over your code).

Show 5 quoted lines

> +
> +	return xdl_diff(&a, &b, xpp, xecfg, xecb);
>  }
>  
>  int read_mmfile(mmfile_t *ptr, const char *filename)

-Peter