From 91de344d7696ba8d7b92b7c503fdc0d3961e8cd2 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 25 May 2013 11:08:03 +0200 Subject: builtin_diff_tree(): make it obvious that function wants two entries Instead of accepting an array and using exactly two elements from the array, take two single (struct object_array_entry *) arguments. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/diff.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'builtin/diff.c') diff --git a/builtin/diff.c b/builtin/diff.c index 8c2af6cb43..abdd613a83 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -153,7 +153,8 @@ static int builtin_diff_index(struct rev_info *revs, static int builtin_diff_tree(struct rev_info *revs, int argc, const char **argv, - struct object_array_entry *ent) + struct object_array_entry *ent0, + struct object_array_entry *ent1) { const unsigned char *(sha1[2]); int swap = 0; @@ -161,13 +162,14 @@ static int builtin_diff_tree(struct rev_info *revs, if (argc > 1) usage(builtin_diff_usage); - /* We saw two trees, ent[0] and ent[1]. - * if ent[1] is uninteresting, they are swapped + /* + * We saw two trees, ent0 and ent1. If ent1 is uninteresting, + * swap them. */ - if (ent[1].item->flags & UNINTERESTING) + if (ent1->item->flags & UNINTERESTING) swap = 1; - sha1[swap] = ent[0].item->sha1; - sha1[1-swap] = ent[1].item->sha1; + sha1[swap] = ent0->item->sha1; + sha1[1-swap] = ent1->item->sha1; diff_tree_sha1(sha1[0], sha1[1], "", &revs->diffopt); log_tree_diff_flush(revs); return 0; @@ -403,7 +405,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) else if (ents == 1) result = builtin_diff_index(&rev, argc, argv); else if (ents == 2) - result = builtin_diff_tree(&rev, argc, argv, ent); + result = builtin_diff_tree(&rev, argc, argv, &ent[0], &ent[1]); else if (ent[0].item->flags & UNINTERESTING) { /* * diff A...B where there is at least one merge base @@ -412,8 +414,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) * between the base and B. Note that we pick one * merge base at random if there are more than one. */ - ent[1] = ent[ents-1]; - result = builtin_diff_tree(&rev, argc, argv, ent); + result = builtin_diff_tree(&rev, argc, argv, &ent[0], &ent[ents-1]); } else result = builtin_diff_combined(&rev, argc, argv, ent, ents); -- cgit v1.3 From 33055fa823eff9f4fddb858856f9b9a8d85316fc Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 25 May 2013 11:08:04 +0200 Subject: cmd_diff(): use an object_array for holding trees Change cmd_diff() to use a (struct object_array) for holding the trees that it accumulates, rather than rolling its own equivalent. Incidentally, this change removes a hard-coded limit of 100 trees in combined diff, not that it matters in practice. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/diff.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) (limited to 'builtin/diff.c') diff --git a/builtin/diff.c b/builtin/diff.c index abdd613a83..661fdde4d5 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -253,8 +253,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix) { int i; struct rev_info rev; - struct object_array_entry ent[100]; - int ents = 0, blobs = 0, paths = 0; + struct object_array ent = OBJECT_ARRAY_INIT; + int blobs = 0, paths = 0; const char *path = NULL; struct blobinfo blob[2]; int nongit; @@ -351,13 +351,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix) if (obj->type == OBJ_COMMIT) obj = &((struct commit *)obj)->tree->object; if (obj->type == OBJ_TREE) { - if (ARRAY_SIZE(ent) <= ents) - die(_("more than %d trees given: '%s'"), - (int) ARRAY_SIZE(ent), name); obj->flags |= flags; - ent[ents].item = obj; - ent[ents].name = name; - ents++; + add_object_array(obj, name, &ent); continue; } if (obj->type == OBJ_BLOB) { @@ -381,7 +376,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) /* * Now, do the arguments look reasonable? */ - if (!ents) { + if (!ent.nr) { switch (blobs) { case 0: result = builtin_diff_files(&rev, argc, argv); @@ -402,22 +397,26 @@ int cmd_diff(int argc, const char **argv, const char *prefix) } else if (blobs) usage(builtin_diff_usage); - else if (ents == 1) + else if (ent.nr == 1) result = builtin_diff_index(&rev, argc, argv); - else if (ents == 2) - result = builtin_diff_tree(&rev, argc, argv, &ent[0], &ent[1]); - else if (ent[0].item->flags & UNINTERESTING) { + else if (ent.nr == 2) + result = builtin_diff_tree(&rev, argc, argv, + &ent.objects[0], &ent.objects[1]); + else if (ent.objects[0].item->flags & UNINTERESTING) { /* * diff A...B where there is at least one merge base - * between A and B. We have ent[0] == merge-base, - * ent[ents-2] == A, and ent[ents-1] == B. Show diff - * between the base and B. Note that we pick one - * merge base at random if there are more than one. + * between A and B. We have ent.objects[0] == + * merge-base, ent.objects[ents-2] == A, and + * ent.objects[ents-1] == B. Show diff between the + * base and B. Note that we pick one merge base at + * random if there are more than one. */ - result = builtin_diff_tree(&rev, argc, argv, &ent[0], &ent[ents-1]); + result = builtin_diff_tree(&rev, argc, argv, + &ent.objects[0], + &ent.objects[ent.nr-1]); } else result = builtin_diff_combined(&rev, argc, argv, - ent, ents); + ent.objects, ent.nr); result = diff_result_code(&rev.diffopt, result); if (1 < rev.diffopt.skip_stat_unmatch) refresh_index_quietly(); -- cgit v1.3 From 026f09e79689f99f573b8399443e8c4ffa84a794 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 25 May 2013 11:08:05 +0200 Subject: cmd_diff(): rename local variable "list" -> "entry" It's not a list, it's an array entry. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/diff.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'builtin/diff.c') diff --git a/builtin/diff.c b/builtin/diff.c index 661fdde4d5..84243d9956 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -339,9 +339,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix) } for (i = 0; i < rev.pending.nr; i++) { - struct object_array_entry *list = rev.pending.objects+i; - struct object *obj = list->item; - const char *name = list->name; + struct object_array_entry *entry = &rev.pending.objects[i]; + struct object *obj = entry->item; + const char *name = entry->name; int flags = (obj->flags & UNINTERESTING); if (!obj->parsed) obj = parse_object(obj->sha1); @@ -360,7 +360,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix) die(_("more than two blobs given: '%s'"), name); hashcpy(blob[blobs].sha1, obj->sha1); blob[blobs].name = name; - blob[blobs].mode = list->mode; + blob[blobs].mode = entry->mode; blobs++; continue; -- cgit v1.3 From 5b1e14eab3109c1d33e27b00ee18f8f9f60e779c Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sat, 25 May 2013 11:08:06 +0200 Subject: cmd_diff(): make it obvious which cases are exclusive of each other At first glance the OBJ_COMMIT, OBJ_TREE, and OBJ_BLOB cases look like they might be mutually exclusive. But the OBJ_COMMIT case doesn't end the loop iteration with "continue" like the other two cases, but rather falls through. So use if...else if...else construct to make it more obvious that only the last two cases are mutually exclusive. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/diff.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'builtin/diff.c') diff --git a/builtin/diff.c b/builtin/diff.c index 84243d9956..9fc273d8cd 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -350,22 +350,21 @@ int cmd_diff(int argc, const char **argv, const char *prefix) die(_("invalid object '%s' given."), name); if (obj->type == OBJ_COMMIT) obj = &((struct commit *)obj)->tree->object; + if (obj->type == OBJ_TREE) { obj->flags |= flags; add_object_array(obj, name, &ent); - continue; - } - if (obj->type == OBJ_BLOB) { + } else if (obj->type == OBJ_BLOB) { if (2 <= blobs) die(_("more than two blobs given: '%s'"), name); hashcpy(blob[blobs].sha1, obj->sha1); blob[blobs].name = name; blob[blobs].mode = entry->mode; blobs++; - continue; + } else { + die(_("unhandled object '%s' given."), name); } - die(_("unhandled object '%s' given."), name); } if (rev.prune_data.nr) { if (!path) -- cgit v1.3