Fix up "git log --follow" a bit..

Fix up "git log --follow" a bit..

[Linus Torvalds]

This fixes "git log --follow" to hopefully not leak memory any more, and 
also cleans it up a bit to look more like some of the other functions that 
use "diff_queued_diff" (by *not* using it directly as a global in the 
code, but by instead just taking a pointer to the diff queue and using 
that).

As to "diff_queued_diff", I think it would be better off not as a global 
at all, but as being just an entry in the "struct diff_options" structure, 
but that's a separate issue, and there may be some subtle reason for why 
it's currently a global.

Anyway, no real changes. Instead of having a magical first entry in the 
diff-queue, we now end up just keeping the diff-queue clean, and keeping 
our "preferred" file pairing in an internal "choice" variable. That makes 
it easy to switch the choice around when we find a better one.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

No changes outside of "try_to_follow_renames()", and most of the diff here 
really is just trivial re-organizations and the addition of "free all the 
filepairs we're not interested in.

Updated the comments a bit to match.

 tree-diff.c |   37 ++++++++++++++++++++++++++++---------
 1 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index 42924e9..26bdbdd 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -305,9 +305,15 @@ static inline int diff_might_be_rename(void)
 static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, const char *base, struct diff_options *opt)
 {
 	struct diff_options diff_opts;
-	const char *paths[2];
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_filepair *choice;
+	const char *paths[1];
 	int i;
 
+	/* Remove the file creation entry from the diff queue, and remember it */
+	choice = q->queue[0];
+	q->nr = 0;
+
 	diff_setup(&diff_opts);
 	diff_opts.recursive = 1;
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
@@ -320,17 +326,21 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	diff_tree(t1, t2, base, &diff_opts);
 	diffcore_std(&diff_opts);
 
-	/* NOTE! Ignore the first diff! That was the old one! */
-	for (i = 1; i < diff_queued_diff.nr; i++) {
-		struct diff_filepair *p = diff_queued_diff.queue[i];
+	/* Go through the new set of filepairing, and see if we find a more interesting one */
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
 
 		/*
 		 * Found a source? Not only do we use that for the new
-		 * diff_queued_diff, we also use that as the path in
+		 * diff_queued_diff, we will also use that as the path in
 		 * the future!
 		 */
 		if ((p->status == 'R' || p->status == 'C') && !strcmp(p->two->path, opt->paths[0])) {
-			diff_queued_diff.queue[0] = p;
+			/* Switch the file-pairs around */
+			q->queue[i] = choice;
+			choice = p;
+
+			/* Update the path we use from now on.. */
 			opt->paths[0] = xstrdup(p->one->path);
 			diff_tree_setup_paths(opt->paths, opt);
 			break;
@@ -338,10 +348,19 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	}
 
 	/*
-	 * Then, ignore any but the first entry! It might be the old one,
-	 * or it might be the rename/copy we found
+	 * Then, discard all the non-relevane file pairs...
+	 */
+	for (i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		diff_free_filepair(p);
+	}
+
+	/*
+	 * .. and re-instate the one we want (which might be either the
+	 * original one, or the rename/copy we found)
 	 */
-	diff_queued_diff.nr = 1;
+	q->queue[0] = choice;
+	q->nr = 1;
 }
 
 int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base, struct diff_options *opt)

Re: Fix up "git log --follow" a bit..

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> As to "diff_queued_diff", I think it would be better off not as a global 
> at all, but as being just an entry in the "struct diff_options" structure, 
> but that's a separate issue, and there may be some subtle reason for why 
> it's currently a global.

There is no subtle issue that I know of, but I have to double
check.  It is just that the code is one of the most ancient part
of the system by now.