Subject: [PATCH 2/4] builtin-blame.c: move prepare_final() into a separate function.

From: Junio C Hamano <[email-masked]>
Date: Thu, 03 Apr 2008 02:13:38 -0700

After parsing the command line, we had a rather long loop to compute the
commit object to start annotating from.  Move the logic to a separate
function, so that later patches become easier to read.

It also makes fill_origin_blob() function to return void; the check on its
return value by its callers is always done on !file->ptr, and nobody looks
at the return value from the function (which is the same as file->ptr).

Signed-off-by: Junio C Hamano <[email-masked]>
---
 builtin-blame.c |   56 ++++++++++++++++++++++++++++++++----------------------
 1 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/builtin-blame.c b/builtin-blame.c
index bfd562d..996f535 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -91,7 +91,7 @@ struct origin {
  * Given an origin, prepare mmfile_t structure to be used by the
  * diff machinery
  */
-static char *fill_origin_blob(struct origin *o, mmfile_t *file)
+static void fill_origin_blob(struct origin *o, mmfile_t *file)
 {
 	if (!o->file.ptr) {
 		enum object_type type;
@@ -106,7 +106,6 @@ static char *fill_origin_blob(struct origin *o, mmfile_t *file)
 	}
 	else
 		*file = o->file;
-	return file->ptr;
 }
 
 /*
@@ -2006,6 +2005,10 @@ static int git_blame_config(const char *var, const char *value)
 	return git_default_config(var, value);
 }
 
+/*
+ * Prepare a dummy commit that represents the work tree (or staged) item.
+ * Note that annotating work tree item never works in the reverse.
+ */
 static struct commit *fake_working_tree_commit(const char *path, const char *contents_from)
 {
 	struct commit *commit;
@@ -2122,6 +2125,33 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 	return commit;
 }
 
+static const char *prepare_final(struct scoreboard *sb, struct rev_info *revs)
+{
+	int i;
+	const char *final_commit_name = NULL;
+
+	/*
+	 * There must be one and only one positive commit in the
+	 * revs->pending array.
+	 */
+	for (i = 0; i < revs->pending.nr; i++) {
+		struct object *obj = revs->pending.objects[i].item;
+		if (obj->flags & UNINTERESTING)
+			continue;
+		while (obj->type == OBJ_TAG)
+			obj = deref_tag(obj, NULL, 0);
+		if (obj->type != OBJ_COMMIT)
+			die("Non commit %s?", revs->pending.objects[i].name);
+		if (sb->final)
+			die("More than one commit to dig from %s and %s?",
+			    revs->pending.objects[i].name,
+			    final_commit_name);
+		sb->final = (struct commit *) obj;
+		final_commit_name = revs->pending.objects[i].name;
+	}
+	return final_commit_name;
+}
+
 int cmd_blame(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info revs;
@@ -2327,27 +2357,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	setup_revisions(unk, argv, &revs, NULL);
 	memset(&sb, 0, sizeof(sb));
 
-	/*
-	 * There must be one and only one positive commit in the
-	 * revs->pending array.
-	 */
-	for (i = 0; i < revs.pending.nr; i++) {
-		struct object *obj = revs.pending.objects[i].item;
-		if (obj->flags & UNINTERESTING)
-			continue;
-		while (obj->type == OBJ_TAG)
-			obj = deref_tag(obj, NULL, 0);
-		if (obj->type != OBJ_COMMIT)
-			die("Non commit %s?",
-			    revs.pending.objects[i].name);
-		if (sb.final)
-			die("More than one commit to dig from %s and %s?",
-			    revs.pending.objects[i].name,
-			    final_commit_name);
-		sb.final = (struct commit *) obj;
-		final_commit_name = revs.pending.objects[i].name;
-	}
-
+	final_commit_name = prepare_final(&sb, &revs);
 	if (!sb.final) {
 		/*
 		 * "--not A B -- path" without anything positive;
-- 
1.5.5.rc3.139.g8b2cf