Re: [RFH] xdiff shows trivially redundant diff.

[Davide Libenzi <davidel@xmailserver.org>]

On Mon, 3 Apr 2006, Junio C Hamano wrote:

> Davide Libenzi <davidel@xmailserver.org> writes:
>
>> No problem. That's only an eye-issue though, since the diff is still a
>> valid diff according to its definition where D=A-B => B+D==A && A-D==B
>> From the day I released 0.18, xregression is continuosly running w/out
>> any issue. I'll check it out though ...
>
> There is another to report, when ctxlen == 0.
>
> Between the attached files "diff -u0 8f352aa dd40a03", the
> header for a hunk with only inserted lines misidentify the
> original location.
>
> For example, the first hunk says:
>
> 	@@ -0,0 +6 @@
>        +#include "diff.h"
>
> Which is inconsistent with what GNU diff says:
>
> 	@@ -5,0 +6 @@
>        +#include "diff.h"
>
> I've tried this patch but it is not right; the diff between the
> attached two files show a 47-line hunk that inserts at line 400,
> then the next 6-line hunk inserts at line 401 which is obviously
> bogus.
>
> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index afaada1..3e7f999 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -244,7 +244,7 @@ int xdl_emit_hunk_hdr(long s1, long c1,
> 	memcpy(buf, "@@ -", 4);
> 	nb += 4;
>
> -	nb += xdl_num_out(buf + nb, c1 ? s1: 0);
> +	nb += xdl_num_out(buf + nb, c1 ? s1 : (s1-1));
>
> 	if (c1 != 1) {
> 		memcpy(buf + nb, ",", 1);

The fix is fine, but you should do the same even in the s2 case. The 
correct hunk should have been:

@@ -6,0 +6 @@

because the lines are actually inserted at the 6th position, but patch 
handle its own special 0 count case by adding 1 to the position, so I had 
to change even the xpatchi.c code. The 0-context diffs are pretty 
dangerous though, since in case of purely added hunks, patch has no way to 
verify the orig-file position by matching contexts.
Now I'll take a look at the pre-diff optimization issue ...



- Davide



--- xdiff/xpatchi.c
+++ xdiff/xpatchi.c
@@ -162,9 +162,9 @@
  	 * We start from zero, so decrement by one unless it's the special position
  	 * '0' inside the unified diff (new or deleted file).
  	 */
-	if (hki->s1 > 0)
+	if (hki->s1 > 0 && hki->c1 > 0)
  		hki->s1--;
-	if (hki->s2 > 0)
+	if (hki->s2 > 0 && hki->c2 > 0)
  		hki->s2--;

  	return 0;


--- xdiff/xutils.c
+++ xdiff/xutils.c
@@ -537,7 +537,7 @@
  	memcpy(buf, "@@ -", 4);
  	nb += 4;

-	nb += xdl_num_out(buf + nb, c1 ? s1: 0);
+	nb += xdl_num_out(buf + nb, c1 ? s1: s1 - 1);

  	memcpy(buf + nb, ",", 1);
  	nb += 1;
@@ -547,7 +547,7 @@
  	memcpy(buf + nb, " +", 2);
  	nb += 2;

-	nb += xdl_num_out(buf + nb, c2 ? s2: 0);
+	nb += xdl_num_out(buf + nb, c2 ? s2: s2 - 1);

  	memcpy(buf + nb, ",", 1);
  	nb += 1;