Re: gitk problems: can't unset "idinlist(...)"

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Jul 21, 2007, 06:11 UTC
Message-ID: <alpine.LFD.0.999.0707202304350.27249@woody.linux-foundation.org>
In-Reply-To: <alpine.LFD.0.999.0707202226130.27249@woody.linux-foundation.org>

On Fri, 20 Jul 2007, Linus Torvalds wrote:

> 
> Junio, didn't we have some parent simplification patches recently?

Yeah. Junio, I think your 11d6596709e04b8d2b429f230b2ed570d013f812 is buggy.

Here's a patch. Not very well tested, but it makes gitk happy and passes all the tests. And I really think Junio's code was very dubious (it did that "p = p->next" *every* time, and then did "pp = &p->next", so "pp" would end up jumping by multiple entries.

The new code only ever changes "pp" - either by moving p->next into it (delete the current entry) or by moving pp forward by one (keep the current entry).

That's much more logical, but somebody should double-check me anyway.

		Linus

---
Subject: Fix up duplicate parents removal

This removes duplicate parents properly, making gitk happy again.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 revision.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/revision.c b/revision.c
index 28b5f2e..7036cf2 100644
--- a/revision.c
+++ b/revision.c
@@ -1323,16 +1323,17 @@ static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp
 
 static void remove_duplicate_parents(struct commit *commit)
 {
-	struct commit_list *p;
-	struct commit_list **pp = &commit->parents;
+	struct commit_list **pp, *p;
 
 	/* Examine existing parents while marking ones we have seen... */
-	for (p = commit->parents; p; p = p->next) {
+	pp = &commit->parents;
+	while ((p = *pp) != NULL) {
 		struct commit *parent = p->item;
-		if (parent->object.flags & TMP_MARK)
+		if (parent->object.flags & TMP_MARK) {
+			*pp = p->next;
 			continue;
+		}
 		parent->object.flags |= TMP_MARK;
-		*pp = p;
 		pp = &p->next;
 	}
 	/* ... and clear the temporary mark */