Subject: [PATCH 6/9] Rewrite launch_editor, create_tag and stripspace to use strbufs.

From: Kristian Høgsberg <[email-masked]>
Date: Wed, 5 Sep 2007 20:23:35 -0400

Signed-off-by: Kristian Høgsberg <[email-masked]>
---
 builtin-stripspace.c |   45 +++++++++++++-------------
 builtin-tag.c        |   84 +++++++++++++++++++++-----------------------------
 builtin.h            |    1 -
 strbuf.c             |   43 +++++---------------------
 strbuf.h             |    2 +
 5 files changed, 68 insertions(+), 107 deletions(-)

diff --git a/builtin-stripspace.c b/builtin-stripspace.c
index 916355c..f0264a8 100644
--- a/builtin-stripspace.c
+++ b/builtin-stripspace.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "strbuf.h"
 
 /*
  * Returns the length of a line, without trailing spaces.
@@ -34,34 +35,34 @@ static size_t cleanup(char *line, size_t len)
  * If the input has only empty lines and spaces,
  * no output will be produced.
  *
- * If last line has a newline at the end, it will be removed.
+ * If the last line does not have a newline, one will be added.
  *
  * Enable skip_comments to skip every line starting with "#".
  */
-size_t stripspace(char *buffer, size_t length, int skip_comments)
+size_t stripspace(struct strbuf *sb, int skip_comments)
 {
 	int empties = -1;
 	size_t i, j, len, newlen;
 	char *eol;
 
-	for (i = j = 0; i < length; i += len, j += newlen) {
-		eol = memchr(buffer + i, '\n', length - i);
-		len = eol ? eol - (buffer + i) + 1 : length - i;
+	for (i = j = 0; i < sb->len; i += len, j += newlen) {
+		eol = memchr(sb->buf + i, '\n', sb->len - i);
+		len = eol ? eol - (sb->buf + i) + 1 : sb->len - i;
 
-		if (skip_comments && len && buffer[i] == '#') {
+		if (skip_comments && len && sb->buf[i] == '#') {
 			newlen = 0;
 			continue;
 		}
-		newlen = cleanup(buffer + i, len);
+		newlen = cleanup(sb->buf + i, len);
 
 		/* Not just an empty line? */
 		if (newlen) {
 			if (empties != -1)
-				buffer[j++] = '\n';
+				sb->buf[j++] = '\n';
 			if (empties > 0)
-				buffer[j++] = '\n';
+				sb->buf[j++] = '\n';
 			empties = 0;
-			memmove(buffer + j, buffer + i, newlen);
+			memmove(sb->buf + j, sb->buf + i, newlen);
 			continue;
 		}
 		if (empties < 0)
@@ -69,31 +70,31 @@ size_t stripspace(char *buffer, size_t length, int skip_comments)
 		empties++;
 	}
 
-	return j;
+	sb->len = j;
+	if (j > 0)
+		strbuf_add_char(sb, '\n');
+
+	return sb->len;
 }
 
 int cmd_stripspace(int argc, const char **argv, const char *prefix)
 {
-	char *buffer;
-	unsigned long size;
+	struct strbuf sb;
 	int strip_comments = 0;
 
 	if (argc > 1 && (!strcmp(argv[1], "-s") ||
 				!strcmp(argv[1], "--strip-comments")))
 		strip_comments = 1;
 
-	size = 1024;
-	buffer = xmalloc(size);
-	if (read_fd(0, &buffer, &size)) {
-		free(buffer);
+	strbuf_init(&sb);
+	if (strbuf_read_fd(&sb, 0) < 0) {
+		strbuf_release(&sb);
 		die("could not read the input");
 	}
 
-	size = stripspace(buffer, size, strip_comments);
-	write_or_die(1, buffer, size);
-	if (size)
-		putc('\n', stdout);
+	stripspace(&sb, strip_comments);
+	write_or_die(1, sb.buf, sb.len);
+	strbuf_release(&sb);
 
-	free(buffer);
 	return 0;
 }
diff --git a/builtin-tag.c b/builtin-tag.c
index d6d38ad..1aff952 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -11,13 +11,14 @@
 #include "refs.h"
 #include "tag.h"
 #include "run-command.h"
+#include "strbuf.h"
 
 static const char builtin_tag_usage[] =
   "git-tag [-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg> | -F <file>] <tagname> [<head>]";
 
 static char signingkey[1000];
 
-static void launch_editor(const char *path, char **buffer, unsigned long *len)
+static void launch_editor(const char *path, struct strbuf *sb)
 {
 	const char *editor, *terminal;
 	struct child_process child;
@@ -55,10 +56,9 @@ static void launch_editor(const char *path, char **buffer, unsigned long *len)
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
 		die("could not open '%s': %s", path, strerror(errno));
-	if (read_fd(fd, buffer, len)) {
-		free(*buffer);
+	if (strbuf_read_fd(sb, fd) < 0) {
 		die("could not read message file '%s': %s",
-						path, strerror(errno));
+		    path, strerror(errno));
 	}
 	close(fd);
 }
@@ -191,7 +191,7 @@ static int verify_tag(const char *name, const char *ref,
 	return 0;
 }
 
-static ssize_t do_sign(char *buffer, size_t size, size_t max)
+static int do_sign(struct strbuf *sb)
 {
 	struct child_process gpg;
 	const char *args[4];
@@ -219,17 +219,13 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
 	if (start_command(&gpg))
 		return error("could not run gpg.");
 
-	write_or_die(gpg.in, buffer, size);
+	write_or_die(gpg.in, sb->buf, sb->len);
 	close(gpg.in);
 	gpg.close_in = 0;
-	len = read_in_full(gpg.out, buffer + size, max - size);
-
+	len = strbuf_read_fd(sb, gpg.out);
 	finish_command(&gpg);
 
-	if (len == max - size)
-		return error("could not read the entire signature from gpg.");
-
-	return size + len;
+	return len;
 }
 
 static const char tag_template[] =
@@ -258,26 +254,24 @@ static void create_tag(const unsigned char *object, const char *tag,
 		       char *message, int sign, unsigned char *result)
 {
 	enum object_type type;
-	char header_buf[1024], *buffer = NULL;
-	int header_len, max_size;
-	unsigned long size = 0;
+	struct strbuf sb;
+	int header_len;
 
 	type = sha1_object_info(object, NULL);
 	if (type <= OBJ_NONE)
 	    die("bad object type.");
 
-	header_len = snprintf(header_buf, sizeof(header_buf),
-			  "object %s\n"
-			  "type %s\n"
-			  "tag %s\n"
-			  "tagger %s\n\n",
-			  sha1_to_hex(object),
-			  typename(type),
-			  tag,
-			  git_committer_info(1));
-
-	if (header_len > sizeof(header_buf) - 1)
-		die("tag header too big.");
+	strbuf_init(&sb);
+	strbuf_printf(&sb,
+		      "object %s\n"
+		      "type %s\n"
+		      "tag %s\n"
+		      "tagger %s\n\n",
+		      sha1_to_hex(object),
+		      typename(type),
+		      tag,
+		      git_committer_info(1));
+	header_len = sb.len;
 
 	if (!message) {
 		char *path;
@@ -292,39 +286,31 @@ static void create_tag(const unsigned char *object, const char *tag,
 		write_or_die(fd, tag_template, strlen(tag_template));
 		close(fd);
 
-		launch_editor(path, &buffer, &size);
+		launch_editor(path, &sb);
 
 		unlink(path);
 		free(path);
 	}
 	else {
-		buffer = message;
-		size = strlen(message);
+		strbuf_add(&sb, message, strlen(message));
 	}
 
-	size = stripspace(buffer, size, 1);
-
-	if (!message && !size)
+	if (stripspace(&sb, 1) <= header_len && !message)
 		die("no tag message?");
 
-	/* insert the header and add the '\n' if needed: */
-	max_size = header_len + size + (sign ? MAX_SIGNATURE_LENGTH : 0) + 1;
-	buffer = xrealloc(buffer, max_size);
-	if (size)
-		buffer[size++] = '\n';
-	memmove(buffer + header_len, buffer, size);
-	memcpy(buffer, header_buf, header_len);
-	size += header_len;
-
-	if (sign) {
-		size = do_sign(buffer, size, max_size);
-		if (size < 0)
-			die("unable to sign the tag");
-	}
+	/* Debatable, but makes t7401-tag.sh pass.  Creates an extra
+	 * blank line after the header block for tags with empty
+	 * messages. */
+	if (sb.len < header_len)
+		strbuf_add_char(&sb, '\n');
 
-	if (write_sha1_file(buffer, size, tag_type, result) < 0)
+	if (sign && do_sign(&sb) < 0)
+		die("unable to sign the tag");
+
+	if (write_sha1_file(sb.buf, sb.len, tag_type, result) < 0)
 		die("unable to write tag file");
-	free(buffer);
+
+	strbuf_release(&sb);
 }
 
 int cmd_tag(int argc, const char **argv, const char *prefix)
diff --git a/builtin.h b/builtin.h
index bb72000..91bc595 100644
--- a/builtin.h
+++ b/builtin.h
@@ -7,7 +7,6 @@ extern const char git_version_string[];
 extern const char git_usage_string[];
 
 extern void help_unknown_cmd(const char *cmd);
-extern size_t stripspace(char *buffer, size_t length, int skip_comments);
 extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
 
diff --git a/strbuf.c b/strbuf.c
index fcfc05e..ed2afea 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -6,7 +6,8 @@ void strbuf_init(struct strbuf *sb) {
 	sb->eof = sb->alloc = sb->len = 0;
 }
 
-static void strbuf_begin(struct strbuf *sb) {
+void strbuf_release(struct strbuf *sb)
+{
 	free(sb->buf);
 	strbuf_init(sb);
 }
@@ -35,7 +36,7 @@ static void strbuf_end(struct strbuf *sb) {
 
 void read_line(struct strbuf *sb, FILE *fp, int term) {
 	int ch;
-	strbuf_begin(sb);
+	strbuf_release(sb);
 	if (feof(fp)) {
 		sb->eof = 1;
 		return;
@@ -73,43 +74,15 @@ void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
 {
 	char buffer[2048];
 	va_list args;
-	int len, size = 2 * sizeof buffer;
+	int len;
 
 	va_start(args, fmt);
 	len = vsnprintf(buffer, sizeof(buffer), fmt, args);
 	va_end(args);
 
-	if (len > sizeof(buffer)) {
-		/*
-		 * Didn't fit in the buffer, but this vsnprintf at
-		 * least gives us the required length back.  Grow the
-		 * buffer acccordingly and try again.
-		 */
-		strbuf_grow(sb, len);
-		va_start(args, fmt);
-		len = vsnprintf(sb->buf + sb->len,
-				sb->alloc - sb->len, fmt, args);
-		va_end(args);
-	} else if (len >= 0) {
-		/*
-		 * The initial vsnprintf fit in the temp buffer, just
-		 * copy it to the strbuf.
-		 */
-		strbuf_add(sb, buffer, len);
-	} else {
-		/*
-		 * This vnsprintf sucks and just returns -1 when the
-		 * buffer is too small.  Keep doubling the size until
-		 * it fits.
-		 */
-		while (len < 0) {
-			strbuf_grow(sb, size);
-			va_start(args, fmt);
-			len = vsnprintf(sb->buf + sb->len,
-					sb->alloc - sb->len, fmt, args);
-			va_end(args);
-			size *= 2;
-		}
-	}
+	if (len > sizeof(buffer) || len < 0)
+		die("out of buffer space\n");
+
+	strbuf_add(sb, buffer, len);
 }
 
diff --git a/strbuf.h b/strbuf.h
index 6e630ea..a93b9e1 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -8,10 +8,12 @@ struct strbuf {
 };
 
 extern void strbuf_init(struct strbuf *);
+extern void strbuf_release(struct strbuf *);
 extern void read_line(struct strbuf *, FILE *, int);
 extern void strbuf_add(struct strbuf *sb, const char *data, size_t len);
 extern void strbuf_add_char(struct strbuf *sb, int ch);
 extern void strbuf_printf(struct strbuf *sb, const char *fmt, ...);
 extern int strbuf_read_fd(struct strbuf *sb, int fd);
+extern size_t stripspace(struct strbuf *sb, int skip_comments);
 
 #endif /* STRBUF_H */
-- 
1.5.2.GIT