On Thu, Jan 19, 2012 at 11:58 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Comments?
Looks conceptually right, but I do have to admit to hating that new variable.
I don't see a better way to do it, though. Sure, you could do it with just
if (revs->first_parent_only && pp != &commit->parents)
break;
and avoid the new variable that way, but that replaces the annoying
variable with a pretty subtle thing.
Or we could re-write that while() loop and move the 'parent' variable
into it. Like the appended untested thing.
But maybe your patch is better, and my dislike for that parent counter
is just irrational.
Linus
revision.c | 16 ++++++++++------
1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/revision.c b/revision.c
index 064e35108478..5e8eb379c369 100644
--- a/revision.c
+++ b/revision.c
@@ -415,7 +415,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
{
- struct commit_list **pp, *parent;
+ struct commit_list **pp;
int tree_changed = 0, tree_same = 0;
/*
@@ -441,8 +441,14 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
return;
pp = &commit->parents;
- while ((parent = *pp) != NULL) {
- struct commit *p = parent->item;
+ do {
+ struct commit_list *parent = *pp;
+ struct commit *p;
+
+ if (!parent)
+ break;
+ pp = &parent->next;
+ p = parent->item;
if (parse_commit(p) < 0)
die("cannot simplify commit %s (because of %s)",
@@ -458,7 +464,6 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
* to lose the other branches of this
* merge, so we just keep going.
*/
- pp = &parent->next;
continue;
}
parent->next = NULL;
@@ -487,11 +492,10 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
case REV_TREE_OLD:
case REV_TREE_DIFFERENT:
tree_changed = 1;
- pp = &parent->next;
continue;
}
die("bad tree compare for commit %s", sha1_to_hex(commit->object.sha1));
- }
+ } while (!revs->first_parent_only);
if (tree_changed && !tree_same)
return;
commit->object.flags |= TREESAME;