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 df4a394f (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;
+		strbuf_reset(&fmt);
-		fmt_len = c - b - 8;
+		strbuf_add(&fmt, b + 8, 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,
+		strbuf_add(&result, a, b - a);
-		                                      &allocated);
+		format_commit_message(commit, fmt.buf, &result);
-		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;
 		len -= c + 1 - a;
 		a = c + 1;
 	}
 
-	if (result && len) {
+	if (result.len && len) {
-		result = xrealloc(result, result_len + len);
+		strbuf_add(&result, a, len);
-		memcpy(result + result_len, a, len);
-		result_len += 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;
+		struct strbuf subject;
-		unsigned long subject_len = 0;
 		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,
+			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-					    &subject, &subject_len, 0,
+					    &subject, 0, NULL, NULL, 0);
-					    NULL, NULL, 0);
+			sub = subject.buf;
-			sub = subject;
 		}
 		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)
+		strbuf_release(&subject);
-			free(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;
+			struct strbuf buf;
-			unsigned long buflen = 0;
+			strbuf_init(&buf, 0);
-			pretty_print_commit(CMIT_FMT_ONELINE, commit, ~0,
+			pretty_print_commit(CMIT_FMT_ONELINE, commit,
-			                    &buf, &buflen, 0, NULL, NULL, 0);
+			                    &buf, 0, NULL, NULL, 0);
 			printf("%c %s %s\n", sign,
-			       sha1_to_hex(commit->object.sha1), buf);
+			       sha1_to_hex(commit->object.sha1), buf.buf);
-			free(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;
+		struct strbuf buf;
-		unsigned long buflen = 0;
+		strbuf_init(&buf, 0);
-		pretty_print_commit(revs.commit_format, commit, ~0,
+		pretty_print_commit(revs.commit_format, commit,
-				    &buf, &buflen,
+					&buf, revs.abbrev, NULL, NULL, revs.date_mode);
-				    revs.abbrev, NULL, NULL, revs.date_mode);
+		printf("%s%c", buf.buf, hdr_termination);
-		printf("%s%c", buf, hdr_termination);
+		strbuf_release(&buf);
-		free(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_print_commit(CMIT_FMT_ONELINE, commit,
-				    &pretty, &pretty_len,
+					&pretty, 0, NULL, NULL, 0);
-				    0, NULL, NULL, 0);
+		pretty_str = pretty.buf;
-		pretty_str = pretty;
 	}
 	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];

commit.c

@@ -458,11 +458,11 @@ void clear_commit_marks(struct commit *commit, unsigned int mark)
 /*
  * Generic support for pretty-printing the header
  */
-static int get_one_line(const char *msg, unsigned long len)
+static int get_one_line(const char *msg)
 {
 	int ret = 0;
 
-	while (len--) {
+	for (;;) {
 		char c = *msg++;
 		if (!c)
 			break;
@@ -485,31 +485,24 @@ static int is_rfc2047_special(char ch)
 	return (non_ascii(ch) || (ch == '=') || (ch == '?') || (ch == '_'));
 }
 
-static int add_rfc2047(char *buf, const char *line, int len,
+static void add_rfc2047(struct strbuf *sb, const char *line, int len,
 		       const char *encoding)
 {
-	char *bp = buf;
+	int i, last;
-	int i, needquote;
-	char q_encoding[128];
-	const char *q_encoding_fmt = "=?%s?q?";
 
-	for (i = needquote = 0; !needquote && i < len; i++) {
+	for (i = 0; i < len; i++) {
 		int ch = line[i];
 		if (non_ascii(ch))
-			needquote++;
+			goto needquote;
-		if ((i + 1 < len) &&
+		if ((i + 1 < len) && (ch == '=' && line[i+1] == '?'))
-		    (ch == '=' && line[i+1] == '?'))
+			goto needquote;
-			needquote++;
 	}
-	if (!needquote)
+	strbuf_add(sb, line, len);
-		return sprintf(buf, "%.*s", len, line);
+	return;
 
-	i = snprintf(q_encoding, sizeof(q_encoding), q_encoding_fmt, encoding);
+needquote:
-	if (sizeof(q_encoding) < i)
+	strbuf_addf(sb, "=?%s?q?", encoding);
-		die("Insanely long encoding name %s", encoding);
+	for (i = last = 0; i < len; i++) {
-	memcpy(bp, q_encoding, i);
-	bp += i;
-	for (i = 0; i < len; i++) {
 		unsigned ch = line[i] & 0xFF;
 		/*
 		 * We encode ' ' using '=20' even though rfc2047
@@ -518,15 +511,13 @@ static int add_rfc2047(char *buf, const char *line, int len,
 		 * leave the underscore in place.
 		 */
 		if (is_rfc2047_special(ch) || ch == ' ') {
-			sprintf(bp, "=%02X", ch);
+			strbuf_add(sb, line + last, i - last);
-			bp += 3;
+			strbuf_addf(sb, "=%02X", ch);
+			last = i + 1;
 		}
-		else
-			*bp++ = ch;
 	}
-	memcpy(bp, "?=", 2);
+	strbuf_add(sb, line + last, len - last);
-	bp += 2;
+	strbuf_addstr(sb, "?=");
-	return bp - buf;
 }
 
 static unsigned long bound_rfc2047(unsigned long len, const char *encoding)
@@ -537,21 +528,21 @@ static unsigned long bound_rfc2047(unsigned long len, const char *encoding)
 	return len * 3 + elen + 100;
 }
 
-static int add_user_info(const char *what, enum cmit_fmt fmt, char *buf,
+static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,
 			 const char *line, enum date_mode dmode,
 			 const char *encoding)
 {
 	char *date;
 	int namelen;
 	unsigned long time;
-	int tz, ret;
+	int tz;
 	const char *filler = "    ";
 
 	if (fmt == CMIT_FMT_ONELINE)
-		return 0;
+		return;
 	date = strchr(line, '>');
 	if (!date)
-		return 0;
+		return;
 	namelen = ++date - line;
 	time = strtoul(date, &date, 10);
 	tz = strtol(date, NULL, 10);
@@ -560,42 +551,35 @@ static int add_user_info(const char *what, enum cmit_fmt fmt, char *buf,
 		char *name_tail = strchr(line, '<');
 		int display_name_length;
 		if (!name_tail)
-			return 0;
+			return;
 		while (line < name_tail && isspace(name_tail[-1]))
 			name_tail--;
 		display_name_length = name_tail - line;
 		filler = "";
-		strcpy(buf, "From: ");
+		strbuf_addstr(sb, "From: ");
-		ret = strlen(buf);
+		add_rfc2047(sb, line, display_name_length, encoding);
-		ret += add_rfc2047(buf + ret, line, display_name_length,
+		strbuf_add(sb, name_tail, namelen - display_name_length);
-				   encoding);
+		strbuf_addch(sb, '\n');
-		memcpy(buf + ret, name_tail, namelen - display_name_length);
-		ret += namelen - display_name_length;
-		buf[ret++] = '\n';
 	}
 	else {
-		ret = sprintf(buf, "%s: %.*s%.*s\n", what,
+		strbuf_addf(sb, "%s: %.*s%.*s\n", what,
 			      (fmt == CMIT_FMT_FULLER) ? 4 : 0,
 			      filler, namelen, line);
 	}
 	switch (fmt) {
 	case CMIT_FMT_MEDIUM:
-		ret += sprintf(buf + ret, "Date:   %s\n",
+		strbuf_addf(sb, "Date:   %s\n", show_date(time, tz, dmode));
-			       show_date(time, tz, dmode));
 		break;
 	case CMIT_FMT_EMAIL:
-		ret += sprintf(buf + ret, "Date: %s\n",
+		strbuf_addf(sb, "Date: %s\n", show_date(time, tz, DATE_RFC2822));
-			       show_date(time, tz, DATE_RFC2822));
 		break;
 	case CMIT_FMT_FULLER:
-		ret += sprintf(buf + ret, "%sDate: %s\n", what,
+		strbuf_addf(sb, "%sDate: %s\n", what, show_date(time, tz, dmode));
-			       show_date(time, tz, dmode));
 		break;
 	default:
 		/* notin' */
 		break;
 	}
-	return ret;
 }
 
 static int is_empty_line(const char *line, int *len_p)
@@ -607,16 +591,16 @@ static int is_empty_line(const char *line, int *len_p)
 	return !len;
 }
 
-static int add_merge_info(enum cmit_fmt fmt, char *buf, const struct commit *commit, int abbrev)
+static void add_merge_info(enum cmit_fmt fmt, struct strbuf *sb,
+			const struct commit *commit, int abbrev)
 {
 	struct commit_list *parent = commit->parents;
-	int offset;
 
 	if ((fmt == CMIT_FMT_ONELINE) || (fmt == CMIT_FMT_EMAIL) ||
 	    !parent || !parent->next)
-		return 0;
+		return;
 
-	offset = sprintf(buf, "Merge:");
+	strbuf_addstr(sb, "Merge:");
 
 	while (parent) {
 		struct commit *p = parent->item;
@@ -629,10 +613,9 @@ static int add_merge_info(enum cmit_fmt fmt, char *buf, const struct commit *com
 		dots = (abbrev && strlen(hex) != 40) ?  "..." : "";
 		parent = parent->next;
 
-		offset += sprintf(buf + offset, " %s%s", hex, dots);
+		strbuf_addf(sb, " %s%s", hex, dots);
 	}
-	buf[offset++] = '\n';
+	strbuf_addch(sb, '\n');
-	return offset;
 }
 
 static char *get_header(const struct commit *commit, const char *key)
@@ -787,8 +770,8 @@ static void fill_person(struct interp *table, const char *msg, int len)
 	interp_set_entry(table, 6, show_date(date, tz, DATE_ISO8601));
 }
 
-long format_commit_message(const struct commit *commit, const void *format,
+void format_commit_message(const struct commit *commit,
-                           char **buf_p, unsigned long *space_p)
+                           const void *format, struct strbuf *sb)
 {
 	struct interp table[] = {
 		{ "%H" },	/* commit hash */
@@ -841,6 +824,7 @@ long format_commit_message(const struct commit *commit, const void *format,
 	};
 	struct commit_list *p;
 	char parents[1024];
+	unsigned long len;
 	int i;
 	enum { HEADER, SUBJECT, BODY } state;
 	const char *msg = commit->buffer;
@@ -921,20 +905,15 @@ long format_commit_message(const struct commit *commit, const void *format,
 		if (!table[i].value)
 			interp_set_entry(table, i, "<unknown>");
 
-	do {
+	len = interpolate(sb->buf + sb->len, strbuf_avail(sb),
-		char *buf = *buf_p;
+				format, table, ARRAY_SIZE(table));
-		unsigned long len;
+	if (len > strbuf_avail(sb)) {
-
+		strbuf_grow(sb, len);
-		len = interpolate(buf, *space_p, format,
+		interpolate(sb->buf + sb->len, strbuf_avail(sb) + 1,
-				    table, ARRAY_SIZE(table));
+					format, table, ARRAY_SIZE(table));
-		if (len < *space_p)
+	}
-			break;
+	strbuf_setlen(sb, sb->len + len);
-		ALLOC_GROW(buf, len + 1, *space_p);
-		*buf_p = buf;
-	} while (1);
 	interp_clear_table(table, ARRAY_SIZE(table));
-
-	return strlen(*buf_p);
 }
 
 static void pp_header(enum cmit_fmt fmt,
@@ -943,34 +922,24 @@ static void pp_header(enum cmit_fmt fmt,
 		      const char *encoding,
 		      const struct commit *commit,
 		      const char **msg_p,
-		      unsigned long *len_p,
+		      struct strbuf *sb)
-		      unsigned long *ofs_p,
-		      char **buf_p,
-		      unsigned long *space_p)
 {
 	int parents_shown = 0;
 
 	for (;;) {
 		const char *line = *msg_p;
-		char *dst;
+		int linelen = get_one_line(*msg_p);
-		int linelen = get_one_line(*msg_p, *len_p);
-		unsigned long len;
 
 		if (!linelen)
 			return;
 		*msg_p += linelen;
-		*len_p -= linelen;
 
 		if (linelen == 1)
 			/* End of header */
 			return;
 
-		ALLOC_GROW(*buf_p, linelen + *ofs_p + 20, *space_p);
-		dst = *buf_p + *ofs_p;
-
 		if (fmt == CMIT_FMT_RAW) {
-			memcpy(dst, line, linelen);
+			strbuf_add(sb, line, linelen);
-			*ofs_p += linelen;
 			continue;
 		}
 
@@ -988,10 +957,8 @@ static void pp_header(enum cmit_fmt fmt,
 			     parent = parent->next, num++)
 				;
 			/* with enough slop */
-			num = *ofs_p + num * 50 + 20;
+			strbuf_grow(sb, num * 50 + 20);
-			ALLOC_GROW(*buf_p, num, *space_p);
+			add_merge_info(fmt, sb, commit, abbrev);
-			dst = *buf_p + *ofs_p;
-			*ofs_p += add_merge_info(fmt, dst, commit, abbrev);
 			parents_shown = 1;
 		}
 
@@ -1001,129 +968,99 @@ static void pp_header(enum cmit_fmt fmt,
 		 * FULLER shows both authors and dates.
 		 */
 		if (!memcmp(line, "author ", 7)) {
-			len = linelen;
+			unsigned long len = linelen;
 			if (fmt == CMIT_FMT_EMAIL)
 				len = bound_rfc2047(linelen, encoding);
-			ALLOC_GROW(*buf_p, *ofs_p + len + 80, *space_p);
+			strbuf_grow(sb, len + 80);
-			dst = *buf_p + *ofs_p;
+			add_user_info("Author", fmt, sb, line + 7, dmode, encoding);
-			*ofs_p += add_user_info("Author", fmt, dst,
-						line + 7, dmode, encoding);
 		}
 
 		if (!memcmp(line, "committer ", 10) &&
 		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
-			len = linelen;
+			unsigned long len = linelen;
 			if (fmt == CMIT_FMT_EMAIL)
 				len = bound_rfc2047(linelen, encoding);
-			ALLOC_GROW(*buf_p, *ofs_p + len + 80, *space_p);
+			strbuf_grow(sb, len + 80);
-			dst = *buf_p + *ofs_p;
+			add_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
-			*ofs_p += add_user_info("Commit", fmt, dst,
-						line + 10, dmode, encoding);
 		}
 	}
 }
 
 static void pp_title_line(enum cmit_fmt fmt,
 			  const char **msg_p,
-			  unsigned long *len_p,
+			  struct strbuf *sb,
-			  unsigned long *ofs_p,
-			  char **buf_p,
-			  unsigned long *space_p,
-			  int indent,
 			  const char *subject,
 			  const char *after_subject,
 			  const char *encoding,
 			  int plain_non_ascii)
 {
-	char *title;
+	struct strbuf title;
-	unsigned long title_alloc, title_len;
 	unsigned long len;
 
-	title_len = 0;
+	strbuf_init(&title, 80);
-	title_alloc = 80;
+
-	title = xmalloc(title_alloc);
 	for (;;) {
 		const char *line = *msg_p;
-		int linelen = get_one_line(line, *len_p);
+		int linelen = get_one_line(line);
-		*msg_p += linelen;
-		*len_p -= linelen;
 
+		*msg_p += linelen;
 		if (!linelen || is_empty_line(line, &linelen))
 			break;
 
-		if (title_alloc <= title_len + linelen + 2) {
+		strbuf_grow(&title, linelen + 2);
-			title_alloc = title_len + linelen + 80;
+		if (title.len) {
-			title = xrealloc(title, title_alloc);
-		}
-		len = 0;
-		if (title_len) {
 			if (fmt == CMIT_FMT_EMAIL) {
-				len++;
+				strbuf_addch(&title, '\n');
-				title[title_len++] = '\n';
 			}
-			len++;
+			strbuf_addch(&title, ' ');
-			title[title_len++] = ' ';
 		}
-		memcpy(title + title_len, line, linelen);
+		strbuf_add(&title, line, linelen);
-		title_len += linelen;
 	}
 
 	/* Enough slop for the MIME header and rfc2047 */
-	len = bound_rfc2047(title_len, encoding)+ 1000;
+	len = bound_rfc2047(title.len, encoding) + 1000;
 	if (subject)
 		len += strlen(subject);
 	if (after_subject)
 		len += strlen(after_subject);
 	if (encoding)
 		len += strlen(encoding);
-	ALLOC_GROW(*buf_p, title_len + *ofs_p + len, *space_p);
 
+	strbuf_grow(sb, title.len + len);
 	if (subject) {
-		len = strlen(subject);
+		strbuf_addstr(sb, subject);
-		memcpy(*buf_p + *ofs_p, subject, len);
+		add_rfc2047(sb, title.buf, title.len, encoding);
-		*ofs_p += len;
-		*ofs_p += add_rfc2047(*buf_p + *ofs_p,
-				      title, title_len, encoding);
 	} else {
-		memcpy(*buf_p + *ofs_p, title, title_len);
+		strbuf_addbuf(sb, &title);
-		*ofs_p += title_len;
 	}
-	(*buf_p)[(*ofs_p)++] = '\n';
+	strbuf_addch(sb, '\n');
+
 	if (plain_non_ascii) {
 		const char *header_fmt =
 			"MIME-Version: 1.0\n"
 			"Content-Type: text/plain; charset=%s\n"
 			"Content-Transfer-Encoding: 8bit\n";
-		*ofs_p += snprintf(*buf_p + *ofs_p,
+		strbuf_addf(sb, header_fmt, encoding);
-				   *space_p - *ofs_p,
-				   header_fmt, encoding);
 	}
 	if (after_subject) {
-		len = strlen(after_subject);
+		strbuf_addstr(sb, after_subject);
-		memcpy(*buf_p + *ofs_p, after_subject, len);
-		*ofs_p += len;
 	}
-	free(title);
 	if (fmt == CMIT_FMT_EMAIL) {
-		ALLOC_GROW(*buf_p, *ofs_p + 20, *space_p);
+		strbuf_addch(sb, '\n');
-		(*buf_p)[(*ofs_p)++] = '\n';
 	}
+	strbuf_release(&title);
 }
 
 static void pp_remainder(enum cmit_fmt fmt,
 			 const char **msg_p,
-			 unsigned long *len_p,
+			 struct strbuf *sb,
-			 unsigned long *ofs_p,
-			 char **buf_p,
-			 unsigned long *space_p,
 			 int indent)
 {
 	int first = 1;
 	for (;;) {
 		const char *line = *msg_p;
-		int linelen = get_one_line(line, *len_p);
+		int linelen = get_one_line(line);
 		*msg_p += linelen;
-		*len_p -= linelen;
 
 		if (!linelen)
 			break;
@@ -1136,36 +1073,32 @@ static void pp_remainder(enum cmit_fmt fmt,
 		}
 		first = 0;
 
-		ALLOC_GROW(*buf_p, *ofs_p + linelen + indent + 20, *space_p);
+		strbuf_grow(sb, linelen + indent + 20);
 		if (indent) {
-			memset(*buf_p + *ofs_p, ' ', indent);
+			memset(sb->buf + sb->len, ' ', indent);
-			*ofs_p += indent;
+			strbuf_setlen(sb, sb->len + indent);
 		}
-		memcpy(*buf_p + *ofs_p, line, linelen);
+		strbuf_add(sb, line, linelen);
-		*ofs_p += linelen;
+		strbuf_addch(sb, '\n');
-		(*buf_p)[(*ofs_p)++] = '\n';
 	}
 }
 
-unsigned long pretty_print_commit(enum cmit_fmt fmt,
+void pretty_print_commit(enum cmit_fmt fmt, const struct commit *commit,
-				  const struct commit *commit,
+				  struct strbuf *sb, int abbrev,
-				  unsigned long len,
+				  const char *subject, const char *after_subject,
-				  char **buf_p, unsigned long *space_p,
-				  int abbrev, const char *subject,
-				  const char *after_subject,
 				  enum date_mode dmode)
 {
-	unsigned long offset = 0;
 	unsigned long beginning_of_body;
 	int indent = 4;
 	const char *msg = commit->buffer;
 	int plain_non_ascii = 0;
 	char *reencoded;
 	const char *encoding;
-	char *buf;
 
-	if (fmt == CMIT_FMT_USERFORMAT)
+	if (fmt == CMIT_FMT_USERFORMAT) {
-		return format_commit_message(commit, user_format, buf_p, space_p);
+		format_commit_message(commit, user_format, sb);
+		return;
+	}
 
 	encoding = (git_log_output_encoding
 		    ? git_log_output_encoding
@@ -1175,7 +1108,6 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
 	reencoded = logmsg_reencode(commit, encoding);
 	if (reencoded) {
 		msg = reencoded;
-		len = strlen(reencoded);
 	}
 
 	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
@@ -1190,14 +1122,13 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
 	if (fmt == CMIT_FMT_EMAIL && !after_subject) {
 		int i, ch, in_body;
 
-		for (in_body = i = 0; (ch = msg[i]) && i < len; i++) {
+		for (in_body = i = 0; (ch = msg[i]); i++) {
 			if (!in_body) {
 				/* author could be non 7-bit ASCII but
 				 * the log may be so; skip over the
 				 * header part first.
 				 */
-				if (ch == '\n' &&
+				if (ch == '\n' && msg[i+1] == '\n')
-				    i + 1 < len && msg[i+1] == '\n')
 					in_body = 1;
 			}
 			else if (non_ascii(ch)) {
@@ -1207,59 +1138,44 @@ unsigned long pretty_print_commit(enum cmit_fmt fmt,
 		}
 	}
 
-	pp_header(fmt, abbrev, dmode, encoding,
+	pp_header(fmt, abbrev, dmode, encoding, commit, &msg, sb);
-		  commit, &msg, &len,
-		  &offset, buf_p, space_p);
 	if (fmt != CMIT_FMT_ONELINE && !subject) {
-		ALLOC_GROW(*buf_p, offset + 20, *space_p);
+		strbuf_addch(sb, '\n');
-		(*buf_p)[offset++] = '\n';
 	}
 
 	/* Skip excess blank lines at the beginning of body, if any... */
 	for (;;) {
-		int linelen = get_one_line(msg, len);
+		int linelen = get_one_line(msg);
 		int ll = linelen;
 		if (!linelen)
 			break;
 		if (!is_empty_line(msg, &ll))
 			break;
 		msg += linelen;
-		len -= linelen;
 	}
 
 	/* These formats treat the title line specially. */
-	if (fmt == CMIT_FMT_ONELINE
+	if (fmt == CMIT_FMT_ONELINE || fmt == CMIT_FMT_EMAIL)
-	    || fmt == CMIT_FMT_EMAIL)
+		pp_title_line(fmt, &msg, sb, subject,
-		pp_title_line(fmt, &msg, &len, &offset,
+			      after_subject, encoding, plain_non_ascii);
-			      buf_p, space_p, indent,
-			      subject, after_subject, encoding,
-			      plain_non_ascii);
 
-	beginning_of_body = offset;
+	beginning_of_body = sb->len;
 	if (fmt != CMIT_FMT_ONELINE)
-		pp_remainder(fmt, &msg, &len, &offset,
+		pp_remainder(fmt, &msg, sb, indent);
-			     buf_p, space_p, indent);
+	strbuf_rtrim(sb);
-
-	while (offset && isspace((*buf_p)[offset-1]))
-		offset--;
-
-	ALLOC_GROW(*buf_p, offset + 20, *space_p);
-	buf = *buf_p;
 
 	/* Make sure there is an EOLN for the non-oneline case */
 	if (fmt != CMIT_FMT_ONELINE)
-		buf[offset++] = '\n';
+		strbuf_addch(sb, '\n');
 
 	/*
 	 * The caller may append additional body text in e-mail
 	 * format.  Make sure we did not strip the blank line
 	 * between the header and the body.
 	 */
-	if (fmt == CMIT_FMT_EMAIL && offset <= beginning_of_body)
+	if (fmt == CMIT_FMT_EMAIL && sb->len <= beginning_of_body)
-		buf[offset++] = '\n';
+		strbuf_addch(sb, '\n');
-	buf[offset] = '\0';
 	free(reencoded);
-	return offset;
 }
 
 struct commit *pop_commit(struct commit_list **stack)
@@ -1482,8 +1398,7 @@ static struct commit_list *merge_bases(struct commit *one, struct commit *two)
 }
 
 struct commit_list *get_merge_bases(struct commit *one,
-				    struct commit *two,
+					struct commit *two, int cleanup)
-                                    int cleanup)
 {
 	struct commit_list *list;
 	struct commit **rslt;

commit.h

@@ -3,6 +3,7 @@
 
 #include "object.h"
 #include "tree.h"
+#include "strbuf.h"
 #include "decorate.h"
 
 struct commit_list {
@@ -61,8 +62,12 @@ enum cmit_fmt {
 };
 
 extern enum cmit_fmt get_commit_format(const char *arg);
-extern long format_commit_message(const struct commit *commit, const void *template, char **buf_p, unsigned long *space_p);
+extern void format_commit_message(const struct commit *commit,
-extern unsigned long pretty_print_commit(enum cmit_fmt fmt, const struct commit *, unsigned long len, char **buf_p, unsigned long *space_p, int abbrev, const char *subject, const char *after_subject, enum date_mode dmode);
+                                  const void *format, struct strbuf *sb);
+extern void pretty_print_commit(enum cmit_fmt fmt, const struct commit*,
+                                struct strbuf *,
+                                int abbrev, const char *subject,
+                                const char *after_subject, enum date_mode);
 
 /** Removes the first commit from a list sorted by date, and adds all
  * of its parents.

log-tree.c

@@ -79,25 +79,14 @@ static int detect_any_signoff(char *letter, int size)
 	return seen_head && seen_name;
 }
 
-static unsigned long append_signoff(char **buf_p, unsigned long *buf_sz_p,
+static void append_signoff(struct strbuf *sb, const char *signoff)
-				    unsigned long at, const char *signoff)
 {
 	static const char signed_off_by[] = "Signed-off-by: ";
 	size_t signoff_len = strlen(signoff);
 	int has_signoff = 0;
 	char *cp;
-	char *buf;
-	unsigned long buf_sz;
 
-	buf = *buf_p;
+	cp = sb->buf;
-	buf_sz = *buf_sz_p;
-	if (buf_sz <= at + strlen(signed_off_by) + signoff_len + 3) {
-		buf_sz += strlen(signed_off_by) + signoff_len + 3;
-		buf = xrealloc(buf, buf_sz);
-		*buf_p = buf;
-		*buf_sz_p = buf_sz;
-	}
-	cp = buf;
 
 	/* First see if we already have the sign-off by the signer */
 	while ((cp = strstr(cp, signed_off_by))) {
@@ -105,29 +94,25 @@ static unsigned long append_signoff(char **buf_p, unsigned long *buf_sz_p,
 		has_signoff = 1;
 
 		cp += strlen(signed_off_by);
-		if (cp + signoff_len >= buf + at)
+		if (cp + signoff_len >= sb->buf + sb->len)
 			break;
 		if (strncmp(cp, signoff, signoff_len))
 			continue;
 		if (!isspace(cp[signoff_len]))
 			continue;
 		/* we already have him */
-		return at;
+		return;
 	}
 
 	if (!has_signoff)
-		has_signoff = detect_any_signoff(buf, at);
+		has_signoff = detect_any_signoff(sb->buf, sb->len);
 
 	if (!has_signoff)
-		buf[at++] = '\n';
+		strbuf_addch(sb, '\n');
 
-	strcpy(buf + at, signed_off_by);
+	strbuf_addstr(sb, signed_off_by);
-	at += strlen(signed_off_by);
+	strbuf_add(sb, signoff, signoff_len);
-	strcpy(buf + at, signoff);
+	strbuf_addch(sb, '\n');
-	at += signoff_len;
-	buf[at++] = '\n';
-	buf[at] = 0;
-	return at;
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -142,14 +127,12 @@ static unsigned int digits_in_number(unsigned int number)
 
 void show_log(struct rev_info *opt, const char *sep)
 {
-	char *msgbuf = NULL;
+	struct strbuf msgbuf;
-	unsigned long msgbuf_len = 0;
 	struct log_info *log = opt->loginfo;
 	struct commit *commit = log->commit, *parent = log->parent;
 	int abbrev = opt->diffopt.abbrev;
 	int abbrev_commit = opt->abbrev_commit ? opt->abbrev : 40;
 	const char *extra;
-	int len;
 	const char *subject = NULL, *extra_headers = opt->extra_headers;
 
 	opt->loginfo = NULL;
@@ -288,18 +271,17 @@ void show_log(struct rev_info *opt, const char *sep)
 	/*
 	 * And then the pretty-printed message itself
 	 */
-	len = pretty_print_commit(opt->commit_format, commit, ~0u,
+	strbuf_init(&msgbuf, 0);
-				  &msgbuf, &msgbuf_len, abbrev, subject,
+	pretty_print_commit(opt->commit_format, commit, &msgbuf,
-				  extra_headers, opt->date_mode);
+				  abbrev, subject, extra_headers, opt->date_mode);
 
 	if (opt->add_signoff)
-		len = append_signoff(&msgbuf, &msgbuf_len, len,
+		append_signoff(&msgbuf, opt->add_signoff);
-				     opt->add_signoff);
 	if (opt->show_log_size)
-		printf("log size %i\n", len);
+		printf("log size %i\n", (int)msgbuf.len);
 
-	printf("%s%s%s", msgbuf, extra, sep);
+	printf("%s%s%s", msgbuf.buf, extra, sep);
-	free(msgbuf);
+	strbuf_release(&msgbuf);
 }
 
 int log_tree_diff_flush(struct rev_info *opt)