Rework pretty_print_commit to use strbufs instead of custom buffers.

  Also remove the "len" parameter, as:
  (1) it was used as a max boundary, and every caller used ~0u
  (2) we check for final NUL no matter what, so it doesn't help for speed.

  As a result most of the pp_* function takes 3 arguments less, and we need
a lot less local variables, this makes the code way more readable, and
easier to extend if needed.

  This patch also fixes some spacing and cosmetic issues.

  This patch also fixes (as a side effect) a memory leak intoruced in
builtin-archive.c at commit df4a394 (fmt was xmalloc'ed and not free'd)

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

builtin-archive.c

@@ -84,14 +84,16 @@ static int run_remote_archiver(const char *remote, int argc,
 static void *format_subst(const struct commit *commit, const char *format,
                           unsigned long *sizep)
 {
-	unsigned long len = *sizep, result_len = 0;
+	unsigned long len = *sizep;
 	const char *a = format;
-	char *result = NULL;
+	struct strbuf result;
+	struct strbuf fmt;
+
+	strbuf_init(&result, 0);
+	strbuf_init(&fmt, 0);
 
 	for (;;) {
 		const char *b, *c;
-		char *fmt, *formatted = NULL;
-		unsigned long a_len, fmt_len, formatted_len, allocated = 0;
 
 		b = memmem(a, len, "$Format:", 8);
 		if (!b || a + len < b + 9)
@@ -100,32 +102,23 @@ static void *format_subst(const struct commit *commit, const char *format,
 		if (!c)
 			break;
 
-		a_len = b - a;
-		fmt_len = c - b - 8;
-		fmt = xmalloc(fmt_len + 1);
-		memcpy(fmt, b + 8, fmt_len);
-		fmt[fmt_len] = '\0';
-
-		formatted_len = format_commit_message(commit, fmt, &formatted,
-		                                      &allocated);
-		free(fmt);
-		result = xrealloc(result, result_len + a_len + formatted_len);
-		memcpy(result + result_len, a, a_len);
-		memcpy(result + result_len + a_len, formatted, formatted_len);
-		result_len += a_len + formatted_len;
+		strbuf_reset(&fmt);
+		strbuf_add(&fmt, b + 8, c - b - 8);
+
+		strbuf_add(&result, a, b - a);
+		format_commit_message(commit, fmt.buf, &result);
 		len -= c + 1 - a;
 		a = c + 1;
 	}
 
-	if (result && len) {
-		result = xrealloc(result, result_len + len);
-		memcpy(result + result_len, a, len);
-		result_len += len;
+	if (result.len && len) {
+		strbuf_add(&result, a, len);
 	}
 
-	*sizep = result_len;
+	*sizep = result.len;
 
-	return result;
+	strbuf_release(&fmt);
+	return strbuf_detach(&result);
 }
 
 static void *convert_to_archive(const char *path,

builtin-branch.c

@@ -268,23 +268,22 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
 	}
 
 	if (verbose) {
-		char *subject = NULL;
-		unsigned long subject_len = 0;
+		struct strbuf subject;
 		const char *sub = " **** invalid ref ****";
 
+		strbuf_init(&subject, 0);
+
 		commit = lookup_commit(item->sha1);
 		if (commit && !parse_commit(commit)) {
-			pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
-					    &subject, &subject_len, 0,
-					    NULL, NULL, 0);
-			sub = subject;
+			pretty_print_commit(CMIT_FMT_ONELINE, commit,
+					    &subject, 0, NULL, NULL, 0);
+			sub = subject.buf;
 		}
 		printf("%c %s%-*s%s %s %s\n", c, branch_get_color(color),
 		       maxwidth, item->name,
 		       branch_get_color(COLOR_BRANCH_RESET),
 		       find_unique_abbrev(item->sha1, abbrev), sub);
-		if (subject)
-			free(subject);
+		strbuf_release(&subject);
 	} else {
 		printf("%c %s%s%s\n", c, branch_get_color(color), item->name,
 		       branch_get_color(COLOR_BRANCH_RESET));

builtin-log.c

@@ -763,13 +763,13 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)
 			sign = '-';
 
 		if (verbose) {
-			char *buf = NULL;
-			unsigned long buflen = 0;
-			pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
-			                    &buf, &buflen, 0, NULL, NULL, 0);
+			struct strbuf buf;
+			strbuf_init(&buf, 0);
+			pretty_print_commit(CMIT_FMT_ONELINE, commit,
+			                    &buf, 0, NULL, NULL, 0);
 			printf("%c %s %s\n", sign,
-			       sha1_to_hex(commit->object.sha1), buf);
-			free(buf);
+			       sha1_to_hex(commit->object.sha1), buf.buf);
+			strbuf_release(&buf);
 		}
 		else {
 			printf("%c %s\n", sign,

builtin-rev-list.c

@@ -80,13 +80,12 @@ static void show_commit(struct commit *commit)
 		putchar('\n');
 
 	if (revs.verbose_header) {
-		char *buf = NULL;
-		unsigned long buflen = 0;
-		pretty_print_commit(revs.commit_format, commit, ~0,
-				    &buf, &buflen,
-				    revs.abbrev, NULL, NULL, revs.date_mode);
-		printf("%s%c", buf, hdr_termination);
-		free(buf);
+		struct strbuf buf;
+		strbuf_init(&buf, 0);
+		pretty_print_commit(revs.commit_format, commit,
+					&buf, revs.abbrev, NULL, NULL, revs.date_mode);
+		printf("%s%c", buf.buf, hdr_termination);
+		strbuf_release(&buf);
 	}
 	maybe_flush_or_die(stdout, "stdout");
 	if (commit->parents) {

builtin-show-branch.c

@@ -259,16 +259,15 @@ static void join_revs(struct commit_list **list_p,
 
 static void show_one_commit(struct commit *commit, int no_name)
 {
-	char *pretty = NULL;
+	struct strbuf pretty;
 	const char *pretty_str = "(unavailable)";
-	unsigned long pretty_len = 0;
 	struct commit_name *name = commit->util;
 
+	strbuf_init(&pretty, 0);
 	if (commit->object.parsed) {
-		pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
-				    &pretty, &pretty_len,
-				    0, NULL, NULL, 0);
-		pretty_str = pretty;
+		pretty_print_commit(CMIT_FMT_ONELINE, commit,
+					&pretty, 0, NULL, NULL, 0);
+		pretty_str = pretty.buf;
 	}
 	if (!prefixcmp(pretty_str, "[PATCH] "))
 		pretty_str += 8;
@@ -289,7 +288,7 @@ static void show_one_commit(struct commit *commit, int no_name)
 			       find_unique_abbrev(commit->object.sha1, 7));
 	}
 	puts(pretty_str);
-	free(pretty);
+	strbuf_release(&pretty);
 }
 
 static char *ref_name[MAX_REVS + 1];