Re: [PATCH 2/2] diff.c: get rid of duplicate implementation

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 26, 2017, 02:23 UTC
Message-ID: <xmqqy3nyfy0h.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171025184912.21657-3-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 19 quoted lines

> The implementations in diff.c to detect moved lines needs to compare
> strings and hash strings, which is implemented in that file, as well
> as in the xdiff library.
>
> Remove the rather recent implementation in diff.c and rely on the well
> exercised code in the xdiff lib.
> ...
>  static int moved_entry_cmp(const struct diff_options *diffopt,
>  			   const struct moved_entry *a,
>  			   const struct moved_entry *b,
>  			   const void *keydata)
>  {
> -	const char *ap = a->es->line, *ae = a->es->line + a->es->len;
> -	const char *bp = b->es->line, *be = b->es->line + b->es->len;
> - ...
> +	return !xdiff_compare_lines(a->es->line, a->es->len,
> +				    b->es->line, b->es->len,
> +				    diffopt->xdl_opts);
>  }

OK, xdiff's xdl_recmatch() takes counted strings, and line[len] is one byte beyond the end of the line (where LF typically sits), which is the same convention as how "emitted_symbol" represents a line, so not just the implementation replaced with a known-working one, but also the code calls the helper with correct calling convention ;-)

> -	ret->ent.hash = get_string_hash(l, o);
> +	ret->ent.hash = xdiff_hash_string(l->line, l->len, o->xdl_opts);

Likewise.  Looks good.

Will queue.