Re: [PATCH] Simplify strbuf uses in fast-import.c using the proper functions.

11 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Simplify strbuf uses in fast-import.c using the proper functions.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:33

Pierre Habouzit [off-list ref] writes:
  oh boy, yes I fixed that in my local patch collection. I'm waiting for
a few hours (days ?) to see if there will be some more comments, I've
integrated every single one done here already (and some I had on IRC
too), and I'll repost a new clean series that I intend to be a real
proposal for inclusion.
Ah, I actually did the single trivial fix-up (ALLOC_GROW) and
have been looking at it, but I'll discard it.  Thanks.
  And yes, this patch is a perfect example of the gain we have to share
a common buffer API. The code looks (at least to me) way nicer, and if
you look in the details, we perform as many memory allocations, copies,
and so on as in the previous version.
Wait.  What is your point in saying this?  Is that a good thing
to do "as many"?  "API is cleaned-up and it is much easier to
read but we do not do more than before" is certainly a *BIG*
plus, so perhaps that is what you meant, but when I first read
it I thought you were saying "we are not optimizing it at all"
in a negative sense.

Re: [PATCH] Simplify strbuf uses in fast-import.c using the proper functions.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

On Wed, Sep 05, 2007 at 08:05:24AM +0000, Junio C Hamano wrote:
Pierre Habouzit [off-list ref] writes:
quoted
  oh boy, yes I fixed that in my local patch collection. I'm waiting for
a few hours (days ?) to see if there will be some more comments, I've
integrated every single one done here already (and some I had on IRC
too), and I'll repost a new clean series that I intend to be a real
proposal for inclusion.
Ah, I actually did the single trivial fix-up (ALLOC_GROW) and
have been looking at it, but I'll discard it.  Thanks.
  Yeah I integrated that as well already. I think I'll post the patches
at the end of the Day (French Timezone so in 5 or 6 hours at least) to
be sure nobody has any good remarks to do first. If you want I can share
my branch somewhere if you want to look at it, just ask.
quoted
  And yes, this patch is a perfect example of the gain we have to share
a common buffer API. The code looks (at least to me) way nicer, and if
you look in the details, we perform as many memory allocations, copies,
and so on as in the previous version.
Wait.  What is your point in saying this?  Is that a good thing
to do "as many"?  "API is cleaned-up and it is much easier to
read but we do not do more than before" is certainly a *BIG*
plus, so perhaps that is what you meant, but when I first read
it I thought you were saying "we are not optimizing it at all"
in a negative sense.
  Sorry if I chose wrong words to say that, it is clearly a big plus ! I
wrote it having in mind that very often abstractions and high level APIs
often degrade performance. It does not, and that's a huge win. The point
is that "despite the somehow higher level API we don't loose an inch of
performance".

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[PATCH] Simplify write_tree using strbuf's.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

Signed-off-by: Pierre Habouzit <redacted>
---
 mktree.c |   22 +++++++---------------
 1 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/mktree.c b/mktree.c
index 86de5eb..3d11ac3 100644
--- a/mktree.c
+++ b/mktree.c
@@ -44,30 +44,22 @@ static int ent_compare(const void *a_, const void *b_)
 
 static void write_tree(unsigned char *sha1)
 {
-	char *buffer;
-	unsigned long size, offset;
+	struct strbuf buf = STRBUF_INIT;
+	size_t size;
 	int i;
 
 	qsort(entries, used, sizeof(*entries), ent_compare);
 	for (size = i = 0; i < used; i++)
 		size += 32 + entries[i]->len;
-	buffer = xmalloc(size);
-	offset = 0;
+	strbuf_grow(&buf, size);
 
 	for (i = 0; i < used; i++) {
 		struct treeent *ent = entries[i];
-
-		if (offset + ent->len + 100 < size) {
-			size = alloc_nr(offset + ent->len + 100);
-			buffer = xrealloc(buffer, size);
-		}
-		offset += sprintf(buffer + offset, "%o ", ent->mode);
-		offset += sprintf(buffer + offset, "%s", ent->name);
-		buffer[offset++] = 0;
-		hashcpy((unsigned char*)buffer + offset, ent->sha1);
-		offset += 20;
+		strbuf_addf(&buf, "%o %s%c", ent->mode, ent->name, '\0');
+		strbuf_add(&buf, ent->sha1, 20);
 	}
-	write_sha1_file(buffer, offset, tree_type, sha1);
+
+	write_sha1_file(buf.buf, buf.len, tree_type, sha1);
 }
 
 static const char mktree_usage[] = "git-mktree [-z]";
-- 
1.5.3

[PATCH] Simplify strbuf uses in archive-tar.c using the proper functions.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

  This is just cleaner way to deal with strbufs, using its API rather than
reinventing it in the module (e.g. strbuf_append_string is just the plain
strbuf_addstr function, and it was used to perform what strbuf_addch does
anyways).
---
 archive-tar.c |   65 ++++++++++++++-------------------------------------------
 1 files changed, 16 insertions(+), 49 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index a0763c5..c84d7c0 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -78,19 +78,6 @@ static void write_trailer(void)
 	}
 }
 
-static void strbuf_append_string(struct strbuf *sb, const char *s)
-{
-	int slen = strlen(s);
-	int total = sb->len + slen;
-	if (total + 1 > sb->alloc) {
-		sb->buf = xrealloc(sb->buf, total + 1);
-		sb->alloc = total + 1;
-	}
-	memcpy(sb->buf + sb->len, s, slen);
-	sb->len = total;
-	sb->buf[total] = '\0';
-}
-
 /*
  * pax extended header records have the format "%u %s=%s\n".  %u contains
  * the size of the whole string (including the %u), the first %s is the
@@ -100,26 +87,17 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
 static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
                                      const char *value, unsigned int valuelen)
 {
-	char *p;
-	int len, total, tmp;
+	int len, tmp;
 
 	/* "%u %s=%s\n" */
 	len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1;
 	for (tmp = len; tmp > 9; tmp /= 10)
 		len++;
 
-	total = sb->len + len;
-	if (total > sb->alloc) {
-		sb->buf = xrealloc(sb->buf, total);
-		sb->alloc = total;
-	}
-
-	p = sb->buf;
-	p += sprintf(p, "%u %s=", len, keyword);
-	memcpy(p, value, valuelen);
-	p += valuelen;
-	*p = '\n';
-	sb->len = total;
+	strbuf_grow(sb, len);
+	strbuf_addf(sb, "%u %s=", len, keyword);
+	strbuf_add(sb, value, valuelen);
+	strbuf_addch(sb, '\n');
 }
 
 static unsigned int ustar_header_chksum(const struct ustar_header *header)
@@ -153,8 +131,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 	struct strbuf ext_header;
 
 	memset(&header, 0, sizeof(header));
-	ext_header.buf = NULL;
-	ext_header.len = ext_header.alloc = 0;
+	strbuf_init(&ext_header);
 
 	if (!sha1) {
 		*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
@@ -225,8 +202,8 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 
 	if (ext_header.len > 0) {
 		write_entry(sha1, NULL, 0, ext_header.buf, ext_header.len);
-		free(ext_header.buf);
 	}
+	strbuf_release(&ext_header);
 	write_blocked(&header, sizeof(header));
 	if (S_ISREG(mode) && buffer && size > 0)
 		write_blocked(buffer, size);
@@ -235,11 +212,11 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 static void write_global_extended_header(const unsigned char *sha1)
 {
 	struct strbuf ext_header;
-	ext_header.buf = NULL;
-	ext_header.len = ext_header.alloc = 0;
+
+	strbuf_init(&ext_header);
 	strbuf_append_ext_header(&ext_header, "comment", sha1_to_hex(sha1), 40);
 	write_entry(NULL, NULL, 0, ext_header.buf, ext_header.len);
-	free(ext_header.buf);
+	strbuf_release(&ext_header);
 }
 
 static int git_tar_config(const char *var, const char *value)
@@ -260,28 +237,18 @@ static int write_tar_entry(const unsigned char *sha1,
                            const char *base, int baselen,
                            const char *filename, unsigned mode, int stage)
 {
-	static struct strbuf path;
+	static struct strbuf path = STRBUF_INIT;
 	int filenamelen = strlen(filename);
 	void *buffer;
 	enum object_type type;
 	unsigned long size;
 
-	if (!path.alloc) {
-		path.buf = xmalloc(PATH_MAX);
-		path.alloc = PATH_MAX;
-		path.len = path.eof = 0;
-	}
-	if (path.alloc < baselen + filenamelen + 1) {
-		free(path.buf);
-		path.buf = xmalloc(baselen + filenamelen + 1);
-		path.alloc = baselen + filenamelen + 1;
-	}
-	memcpy(path.buf, base, baselen);
-	memcpy(path.buf + baselen, filename, filenamelen);
-	path.len = baselen + filenamelen;
-	path.buf[path.len] = '\0';
+	strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
+	strbuf_reset(&path);
+	strbuf_add(&path, base, baselen);
+	strbuf_add(&path, filename, filenamelen);
 	if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
-		strbuf_append_string(&path, "/");
+		strbuf_addch(&path, '/');
 		buffer = NULL;
 		size = 0;
 	} else {
-- 
1.5.3

[PATCH] Use proper strbuf API, and also simplify cmd_data code.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

  This patch features the use of strbuf_detach, and prevent the programmer
to mess with allocation directly. The code is as efficent as before, just
more concise and more straightforward.
---
 fast-import.c |   29 ++++++++++++-----------------
 1 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 2f7baf4..1a02481 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -340,7 +340,7 @@ static struct tag *last_tag;
 
 /* Input stream parsing */
 static whenspec_type whenspec = WHENSPEC_RAW;
-static struct strbuf command_buf;
+static struct strbuf command_buf = STRBUF_INIT;
 static int unread_command_buf;
 static struct recent_command cmd_hist = {&cmd_hist, &cmd_hist, NULL};
 static struct recent_command *cmd_tail = &cmd_hist;
@@ -1638,17 +1638,15 @@ static void cmd_mark(void)
 
 static void *cmd_data (size_t *size)
 {
-	size_t length;
-	char *buffer;
+	struct strbuf buffer = STRBUF_INIT;
 
 	if (prefixcmp(command_buf.buf, "data "))
 		die("Expected 'data n' command, found: %s", command_buf.buf);
 
 	if (!prefixcmp(command_buf.buf + 5, "<<")) {
 		char *term = xstrdup(command_buf.buf + 5 + 2);
-		size_t sz = 8192, term_len = command_buf.len - 5 - 2;
-		length = 0;
-		buffer = xmalloc(sz);
+		size_t term_len = command_buf.len - 5 - 2;
+
 		for (;;) {
 			read_line(&command_buf, stdin, '\n');
 			if (command_buf.eof)
@@ -1656,21 +1654,18 @@ static void *cmd_data (size_t *size)
 			if (term_len == command_buf.len
 				&& !strcmp(term, command_buf.buf))
 				break;
-			ALLOC_GROW(buffer, length + command_buf.len + 1, sz);
-			memcpy(buffer + length,
-				command_buf.buf,
-				command_buf.len);
-			length += command_buf.len;
-			buffer[length++] = '\n';
+			strbuf_addbuf(&buffer, &command_buf);
+			strbuf_addch(&buffer, '\n');
 		}
 		free(term);
 	}
 	else {
-		size_t n = 0;
+		size_t n = 0, length;
+
 		length = strtoul(command_buf.buf + 5, NULL, 10);
-		buffer = xmalloc(length);
+
 		while (n < length) {
-			size_t s = fread(buffer + n, 1, length - n, stdin);
+			size_t s = strbuf_fread(&buffer, length - n, stdin);
 			if (!s && feof(stdin))
 				die("EOF in data (%lu bytes remaining)",
 					(unsigned long)(length - n));
@@ -1679,8 +1674,8 @@ static void *cmd_data (size_t *size)
 	}
 
 	skip_optional_lf();
-	*size = length;
-	return buffer;
+	*size = buffer.len;
+	return strbuf_detach(&buffer);
 }
 
 static int validate_raw_date(const char *src, char *result, int maxlen)
-- 
1.5.3

[PATCH] Rework strbuf API and semantics.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

  A strbuf can be used to store byte arrays, or as an extended string
library. The `buf' member can be passed to any C legacy string function,
because strbuf operations always ensure there is a terminating \0 at the end
of the buffer, not accounted in the `len' field of the structure.

  A strbuf can be used to generate a string/buffer whose final size is not
really known, and then "strbuf_detach" can be used to get the built buffer,
and keep the wrapping "strbuf" structure usable for further work again.

  Other interesting feature: strbuf_grow(sb, size) ensure that there is
enough allocated space in `sb' to put `size' new octets of data in the
buffer. It helps avoiding reallocating data for nothing when the problem the
strbuf helps to solve has a known typical size.

Signed-off-by: Pierre Habouzit <redacted>
---
 archive-tar.c |    2 +-
 fast-import.c |   15 ++++----
 mktree.c      |    4 +--
 strbuf.c      |  103 +++++++++++++++++++++++++++++++++++++++++++++++++--------
 strbuf.h      |   42 ++++++++++++++++++++++-
 5 files changed, 138 insertions(+), 28 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 66fe3e3..a0763c5 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -166,7 +166,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 		sprintf(header.name, "%s.paxheader", sha1_to_hex(sha1));
 	} else {
 		if (verbose)
-			fprintf(stderr, "%.*s\n", path->len, path->buf);
+			fprintf(stderr, "%.*s\n", (int)path->len, path->buf);
 		if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
 			*header.typeflag = TYPEFLAG_DIR;
 			mode = (mode | 0777) & ~tar_umask;
diff --git a/fast-import.c b/fast-import.c
index 078079d..2f7baf4 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1595,7 +1595,7 @@ static void read_next_command(void)
 		} else {
 			struct recent_command *rc;
 
-			command_buf.buf = NULL;
+			strbuf_detach(&command_buf);
 			read_line(&command_buf, stdin, '\n');
 			if (command_buf.eof)
 				return;
@@ -1649,7 +1649,6 @@ static void *cmd_data (size_t *size)
 		size_t sz = 8192, term_len = command_buf.len - 5 - 2;
 		length = 0;
 		buffer = xmalloc(sz);
-		command_buf.buf = NULL;
 		for (;;) {
 			read_line(&command_buf, stdin, '\n');
 			if (command_buf.eof)
@@ -1657,11 +1656,11 @@ static void *cmd_data (size_t *size)
 			if (term_len == command_buf.len
 				&& !strcmp(term, command_buf.buf))
 				break;
-			ALLOC_GROW(buffer, length + command_buf.len, sz);
+			ALLOC_GROW(buffer, length + command_buf.len + 1, sz);
 			memcpy(buffer + length,
 				command_buf.buf,
-				command_buf.len - 1);
-			length += command_buf.len - 1;
+				command_buf.len);
+			length += command_buf.len;
 			buffer[length++] = '\n';
 		}
 		free(term);
@@ -2101,7 +2100,7 @@ static void cmd_new_commit(void)
 	}
 
 	/* file_change* */
-	while (!command_buf.eof && command_buf.len > 1) {
+	while (!command_buf.eof && command_buf.len > 0) {
 		if (!prefixcmp(command_buf.buf, "M "))
 			file_change_m(b);
 		else if (!prefixcmp(command_buf.buf, "D "))
@@ -2256,7 +2255,7 @@ static void cmd_reset_branch(void)
 	else
 		b = new_branch(sp);
 	read_next_command();
-	if (!cmd_from(b) && command_buf.len > 1)
+	if (!cmd_from(b) && command_buf.len > 0)
 		unread_command_buf = 1;
 }
 
@@ -2273,7 +2272,7 @@ static void cmd_checkpoint(void)
 
 static void cmd_progress(void)
 {
-	fwrite(command_buf.buf, 1, command_buf.len - 1, stdout);
+	fwrite(command_buf.buf, 1, command_buf.len, stdout);
 	fputc('\n', stdout);
 	fflush(stdout);
 	skip_optional_lf();
diff --git a/mktree.c b/mktree.c
index d86dde8..86de5eb 100644
--- a/mktree.c
+++ b/mktree.c
@@ -92,7 +92,6 @@ int main(int ac, char **av)
 
 	strbuf_init(&sb);
 	while (1) {
-		int len;
 		char *ptr, *ntr;
 		unsigned mode;
 		enum object_type type;
@@ -101,7 +100,6 @@ int main(int ac, char **av)
 		read_line(&sb, stdin, line_termination);
 		if (sb.eof)
 			break;
-		len = sb.len;
 		ptr = sb.buf;
 		/* Input is non-recursive ls-tree output format
 		 * mode SP type SP sha1 TAB name
@@ -111,7 +109,7 @@ int main(int ac, char **av)
 			die("input format error: %s", sb.buf);
 		ptr = ntr + 1; /* type */
 		ntr = strchr(ptr, ' ');
-		if (!ntr || sb.buf + len <= ntr + 41 ||
+		if (!ntr || sb.buf + sb.len <= ntr + 40 ||
 		    ntr[41] != '\t' ||
 		    get_sha1_hex(ntr + 1, sha1))
 			die("input format error: %s", sb.buf);
diff --git a/strbuf.c b/strbuf.c
index e33d06b..7866fbe 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -2,40 +2,115 @@
 #include "strbuf.h"
 
 void strbuf_init(struct strbuf *sb) {
-	sb->buf = NULL;
-	sb->eof = sb->alloc = sb->len = 0;
+	memset(sb, 0, sizeof(*sb));
 }
 
-static void strbuf_begin(struct strbuf *sb) {
+void strbuf_release(struct strbuf *sb) {
 	free(sb->buf);
+	memset(sb, 0, sizeof(*sb));
+}
+
+void strbuf_reset(struct strbuf *sb) {
+	if (sb->len)
+		sb->buf[sb->len = 0] = '\0';
+}
+
+char *strbuf_detach(struct strbuf *sb) {
+	char *res = sb->buf;
 	strbuf_init(sb);
+	return res;
+}
+
+void strbuf_grow(struct strbuf *sb, size_t extra) {
+	if (sb->len + extra + 1 < sb->len)
+		die("you want to use way too much memory");
+	ALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);
+}
+
+void strbuf_add(struct strbuf *sb, const void *data, size_t len) {
+	strbuf_grow(sb, len);
+	memcpy(sb->buf + sb->len, data, len);
+	sb->len += len;
+	sb->buf[sb->len] = '\0';
+}
+
+void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)
+{
+	size_t len;
+	va_list ap2;
+
+	va_copy(ap2, ap);
+
+	len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
+	if (len < 0) {
+		len = 0;
+	}
+	if (len >= sb->alloc - sb->len) {
+		strbuf_grow(sb, len);
+		len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap2);
+		if (len >= sb->alloc - sb->len) {
+			len = sb->alloc - sb->len - 1;
+		}
+	}
+	sb->len = sb->len + len;
+	sb->buf[sb->len] = '\0';
 }
 
-static void inline strbuf_add(struct strbuf *sb, int ch) {
-	if (sb->alloc <= sb->len) {
-		sb->alloc = sb->alloc * 3 / 2 + 16;
-		sb->buf = xrealloc(sb->buf, sb->alloc);
+size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f) {
+	size_t res;
+
+	strbuf_grow(sb, size);
+	res = fread(sb->buf + sb->len, 1, size, f);
+	if (res > 0) {
+		sb->len += res;
+		sb->buf[sb->len] = '\0';
 	}
-	sb->buf[sb->len++] = ch;
+	return res;
 }
 
-static void strbuf_end(struct strbuf *sb) {
-	strbuf_add(sb, 0);
+ssize_t strbuf_read(struct strbuf *sb, int fd)
+{
+	size_t oldlen = sb->len;
+
+	for (;;) {
+		ssize_t cnt;
+
+		strbuf_grow(sb, 8192);
+		cnt = xread(fd, sb->buf + sb->len, sb->alloc - sb->len - 1);
+		if (cnt < 0) {
+			sb->buf[sb->len = oldlen] = '\0';
+			return -1;
+		}
+		if (!cnt)
+			break;
+		sb->len += cnt;
+	}
+
+	sb->buf[sb->len] = '\0';
+	return sb->len - oldlen;
 }
 
 void read_line(struct strbuf *sb, FILE *fp, int term) {
 	int ch;
-	strbuf_begin(sb);
 	if (feof(fp)) {
+		strbuf_release(sb);
 		sb->eof = 1;
 		return;
 	}
+
+	strbuf_reset(sb);
 	while ((ch = fgetc(fp)) != EOF) {
 		if (ch == term)
 			break;
-		strbuf_add(sb, ch);
+		strbuf_grow(sb, 1);
+		sb->buf[sb->len++] = ch;
 	}
-	if (ch == EOF && sb->len == 0)
+	if (ch == EOF && sb->len == 0) {
+		strbuf_release(sb);
 		sb->eof = 1;
-	strbuf_end(sb);
+	}
+
+	strbuf_grow(sb, 1);
+	sb->buf[sb->len] = '\0';
 }
+
diff --git a/strbuf.h b/strbuf.h
index 74cc012..db1e438 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -1,13 +1,51 @@
 #ifndef STRBUF_H
 #define STRBUF_H
 struct strbuf {
-	int alloc;
-	int len;
+	size_t alloc;
+	size_t len;
 	int eof;
 	char *buf;
 };
 
+#define STRBUF_INIT  { 0, 0, 0, NULL }
+
+/* strbuf life cycle */
 extern void strbuf_init(struct strbuf *);
+extern void strbuf_release(struct strbuf *);
+extern void strbuf_reset(struct strbuf *);
+extern char *strbuf_detach(struct strbuf *);
+
+
+extern void strbuf_grow(struct strbuf *, size_t);
+extern void strbuf_add(struct strbuf *, const void *, size_t);
+
+static inline void strbuf_addstr(struct strbuf *sb, const char *s) {
+	strbuf_add(sb, s, strlen(s));
+}
+static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {
+	strbuf_add(sb, sb2->buf, sb2->len);
+}
+static inline void strbuf_addch(struct strbuf *sb, size_t c) {
+	strbuf_grow(sb, 1);
+	sb->buf[sb->len++] = c;
+	sb->buf[sb->len] = '\0';
+}
+
+__attribute__((format(printf,2,0)))
+extern void strbuf_addvf(struct strbuf *, const char *, va_list);
+
+static inline __attribute__((format(printf,2,3)))
+void strbuf_addf(struct strbuf *sb, const char *fmt, ...) {
+	va_list ap;
+	va_start(ap, fmt);
+	strbuf_addvf(sb, fmt, ap);
+	va_end(ap);
+}
+
+
+extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);
+extern ssize_t strbuf_read(struct strbuf *, int fd);
+
 extern void read_line(struct strbuf *, FILE *, int);
 
 #endif /* STRBUF_H */
-- 
1.5.3

[PATCH] Simplify strbuf uses in fast-import.c using the proper functions.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

  This is just cleaner way to deal with strbufs, using its API rather than
reinventing it in the module (e.g. strbuf_append_string is just the plain
strbuf_addstr function, and it was used to perform what strbuf_addch does
anyways).
---
 archive-tar.c |   65 ++++++++++++++-------------------------------------------
 1 files changed, 16 insertions(+), 49 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index a0763c5..c84d7c0 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -78,19 +78,6 @@ static void write_trailer(void)
 	}
 }
 
-static void strbuf_append_string(struct strbuf *sb, const char *s)
-{
-	int slen = strlen(s);
-	int total = sb->len + slen;
-	if (total + 1 > sb->alloc) {
-		sb->buf = xrealloc(sb->buf, total + 1);
-		sb->alloc = total + 1;
-	}
-	memcpy(sb->buf + sb->len, s, slen);
-	sb->len = total;
-	sb->buf[total] = '\0';
-}
-
 /*
  * pax extended header records have the format "%u %s=%s\n".  %u contains
  * the size of the whole string (including the %u), the first %s is the
@@ -100,26 +87,17 @@ static void strbuf_append_string(struct strbuf *sb, const char *s)
 static void strbuf_append_ext_header(struct strbuf *sb, const char *keyword,
                                      const char *value, unsigned int valuelen)
 {
-	char *p;
-	int len, total, tmp;
+	int len, tmp;
 
 	/* "%u %s=%s\n" */
 	len = 1 + 1 + strlen(keyword) + 1 + valuelen + 1;
 	for (tmp = len; tmp > 9; tmp /= 10)
 		len++;
 
-	total = sb->len + len;
-	if (total > sb->alloc) {
-		sb->buf = xrealloc(sb->buf, total);
-		sb->alloc = total;
-	}
-
-	p = sb->buf;
-	p += sprintf(p, "%u %s=", len, keyword);
-	memcpy(p, value, valuelen);
-	p += valuelen;
-	*p = '\n';
-	sb->len = total;
+	strbuf_grow(sb, len);
+	strbuf_addf(sb, "%u %s=", len, keyword);
+	strbuf_add(sb, value, valuelen);
+	strbuf_addch(sb, '\n');
 }
 
 static unsigned int ustar_header_chksum(const struct ustar_header *header)
@@ -153,8 +131,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 	struct strbuf ext_header;
 
 	memset(&header, 0, sizeof(header));
-	ext_header.buf = NULL;
-	ext_header.len = ext_header.alloc = 0;
+	strbuf_init(&ext_header);
 
 	if (!sha1) {
 		*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
@@ -225,8 +202,8 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 
 	if (ext_header.len > 0) {
 		write_entry(sha1, NULL, 0, ext_header.buf, ext_header.len);
-		free(ext_header.buf);
 	}
+	strbuf_release(&ext_header);
 	write_blocked(&header, sizeof(header));
 	if (S_ISREG(mode) && buffer && size > 0)
 		write_blocked(buffer, size);
@@ -235,11 +212,11 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
 static void write_global_extended_header(const unsigned char *sha1)
 {
 	struct strbuf ext_header;
-	ext_header.buf = NULL;
-	ext_header.len = ext_header.alloc = 0;
+
+	strbuf_init(&ext_header);
 	strbuf_append_ext_header(&ext_header, "comment", sha1_to_hex(sha1), 40);
 	write_entry(NULL, NULL, 0, ext_header.buf, ext_header.len);
-	free(ext_header.buf);
+	strbuf_release(&ext_header);
 }
 
 static int git_tar_config(const char *var, const char *value)
@@ -260,28 +237,18 @@ static int write_tar_entry(const unsigned char *sha1,
                            const char *base, int baselen,
                            const char *filename, unsigned mode, int stage)
 {
-	static struct strbuf path;
+	static struct strbuf path = STRBUF_INIT;
 	int filenamelen = strlen(filename);
 	void *buffer;
 	enum object_type type;
 	unsigned long size;
 
-	if (!path.alloc) {
-		path.buf = xmalloc(PATH_MAX);
-		path.alloc = PATH_MAX;
-		path.len = path.eof = 0;
-	}
-	if (path.alloc < baselen + filenamelen + 1) {
-		free(path.buf);
-		path.buf = xmalloc(baselen + filenamelen + 1);
-		path.alloc = baselen + filenamelen + 1;
-	}
-	memcpy(path.buf, base, baselen);
-	memcpy(path.buf + baselen, filename, filenamelen);
-	path.len = baselen + filenamelen;
-	path.buf[path.len] = '\0';
+	strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
+	strbuf_reset(&path);
+	strbuf_add(&path, base, baselen);
+	strbuf_add(&path, filename, filenamelen);
 	if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
-		strbuf_append_string(&path, "/");
+		strbuf_addch(&path, '/');
 		buffer = NULL;
 		size = 0;
 	} else {
-- 
1.5.3

[PATCH] Further strbuf re-engineering.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

Signed-off-by: Pierre Habouzit <redacted>
---
 builtin-apply.c       |   29 ++++++-----------------
 builtin-blame.c       |   34 +++++++++-------------------
 builtin-commit-tree.c |   59 ++++++++++--------------------------------------
 diff.c                |   25 ++++++--------------
 4 files changed, 40 insertions(+), 107 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index 25b1447..4d8b270 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -12,6 +12,7 @@
 #include "blob.h"
 #include "delta.h"
 #include "builtin.h"
+#include "strbuf.h"
 
 /*
  *  --check turns on checking that the working tree matches the
@@ -181,34 +182,20 @@ static void say_patch_name(FILE *output, const char *pre, struct patch *patch, c
 
 static void *read_patch_file(int fd, unsigned long *sizep)
 {
-	unsigned long size = 0, alloc = CHUNKSIZE;
-	void *buffer = xmalloc(alloc);
+	struct strbuf buf = STRBUF_INIT;
 
-	for (;;) {
-		ssize_t nr = alloc - size;
-		if (nr < 1024) {
-			alloc += CHUNKSIZE;
-			buffer = xrealloc(buffer, alloc);
-			nr = alloc - size;
-		}
-		nr = xread(fd, (char *) buffer + size, nr);
-		if (!nr)
-			break;
-		if (nr < 0)
-			die("git-apply: read returned %s", strerror(errno));
-		size += nr;
-	}
-	*sizep = size;
+	if (strbuf_read(&buf, fd) < 0)
+		die("git-apply: read returned %s", strerror(errno));
+	*sizep = buf.len;
 
 	/*
 	 * Make sure that we have some slop in the buffer
 	 * so that we can do speculative "memcmp" etc, and
 	 * see to it that it is NUL-filled.
 	 */
-	if (alloc < size + SLOP)
-		buffer = xrealloc(buffer, size + SLOP);
-	memset((char *) buffer + size, 0, SLOP);
-	return buffer;
+	strbuf_grow(&buf, SLOP);
+	memset(buf.buf + buf.len, 0, SLOP);
+	return strbuf_detach(&buf);
 }
 
 static unsigned long linelen(const char *buffer, unsigned long size)
diff --git a/builtin-blame.c b/builtin-blame.c
index dc88a95..c2d36a1 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -18,6 +18,7 @@
 #include "cache-tree.h"
 #include "path-list.h"
 #include "mailmap.h"
+#include "strbuf.h"
 
 static char blame_usage[] =
 "git-blame [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [-L n,m] [-S <revs-file>] [-M] [-C] [-C] [--contents <filename>] [--incremental] [commit] [--] file\n"
@@ -2001,11 +2002,10 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 	struct commit *commit;
 	struct origin *origin;
 	unsigned char head_sha1[20];
-	char *buf;
+	struct strbuf buf = STRBUF_INIT;
 	const char *ident;
 	int fd;
 	time_t now;
-	unsigned long fin_size;
 	int size, len;
 	struct cache_entry *ce;
 	unsigned mode;
@@ -2026,6 +2026,7 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 	if (!contents_from || strcmp("-", contents_from)) {
 		struct stat st;
 		const char *read_from;
+		unsigned long fin_size;
 
 		if (contents_from) {
 			if (stat(contents_from, &st) < 0)
@@ -2038,19 +2039,19 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 			read_from = path;
 		}
 		fin_size = xsize_t(st.st_size);
-		buf = xmalloc(fin_size+1);
 		mode = canon_mode(st.st_mode);
 		switch (st.st_mode & S_IFMT) {
 		case S_IFREG:
 			fd = open(read_from, O_RDONLY);
 			if (fd < 0)
 				die("cannot open %s", read_from);
-			if (read_in_full(fd, buf, fin_size) != fin_size)
+			if (strbuf_read(&buf, fd) != xsize_t(st.st_size))
 				die("cannot read %s", read_from);
 			break;
 		case S_IFLNK:
-			if (readlink(read_from, buf, fin_size+1) != fin_size)
+			if (readlink(read_from, buf.buf, buf.alloc) != fin_size)
 				die("cannot readlink %s", read_from);
+			buf.len = fin_size;
 			break;
 		default:
 			die("unsupported file type %s", read_from);
@@ -2059,26 +2060,13 @@ static struct commit *fake_working_tree_commit(const char *path, const char *con
 	else {
 		/* Reading from stdin */
 		contents_from = "standard input";
-		buf = NULL;
-		fin_size = 0;
 		mode = 0;
-		while (1) {
-			ssize_t cnt = 8192;
-			buf = xrealloc(buf, fin_size + cnt);
-			cnt = xread(0, buf + fin_size, cnt);
-			if (cnt < 0)
-				die("read error %s from stdin",
-				    strerror(errno));
-			if (!cnt)
-				break;
-			fin_size += cnt;
-		}
-		buf = xrealloc(buf, fin_size + 1);
+		if (strbuf_read(&buf, 0) < 0)
+			die("read error %s from stdin", strerror(errno));
 	}
-	buf[fin_size] = 0;
-	origin->file.ptr = buf;
-	origin->file.size = fin_size;
-	pretend_sha1_file(buf, fin_size, OBJ_BLOB, origin->blob_sha1);
+	origin->file.ptr = buf.buf;
+	origin->file.size = buf.len;
+	pretend_sha1_file(buf.buf, buf.len, OBJ_BLOB, origin->blob_sha1);
 	commit->util = origin;
 
 	/*
diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index ccbcbe3..ee74814 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -8,42 +8,13 @@
 #include "tree.h"
 #include "builtin.h"
 #include "utf8.h"
+#include "strbuf.h"
 
 #define BLOCKING (1ul << 14)
 
 /*
  * FIXME! Share the code with "write-tree.c"
  */
-static void init_buffer(char **bufp, unsigned int *sizep)
-{
-	*bufp = xmalloc(BLOCKING);
-	*sizep = 0;
-}
-
-static void add_buffer(char **bufp, unsigned int *sizep, const char *fmt, ...)
-{
-	char one_line[2048];
-	va_list args;
-	int len;
-	unsigned long alloc, size, newsize;
-	char *buf;
-
-	va_start(args, fmt);
-	len = vsnprintf(one_line, sizeof(one_line), fmt, args);
-	va_end(args);
-	size = *sizep;
-	newsize = size + len + 1;
-	alloc = (size + 32767) & ~32767;
-	buf = *bufp;
-	if (newsize > alloc) {
-		alloc = (newsize + 32767) & ~32767;
-		buf = xrealloc(buf, alloc);
-		*bufp = buf;
-	}
-	*sizep = newsize - 1;
-	memcpy(buf + size, one_line, len);
-}
-
 static void check_valid(unsigned char *sha1, enum object_type expect)
 {
 	enum object_type type = sha1_object_info(sha1, NULL);
@@ -87,9 +58,7 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	int parents = 0;
 	unsigned char tree_sha1[20];
 	unsigned char commit_sha1[20];
-	char comment[1000];
-	char *buffer;
-	unsigned int size;
+	struct strbuf buffer = STRBUF_INIT;
 	int encoding_is_utf8;
 
 	git_config(git_default_config);
@@ -118,8 +87,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	/* Not having i18n.commitencoding is the same as having utf-8 */
 	encoding_is_utf8 = is_encoding_utf8(git_commit_encoding);
 
-	init_buffer(&buffer, &size);
-	add_buffer(&buffer, &size, "tree %s\n", sha1_to_hex(tree_sha1));
+	strbuf_grow(&buffer, 8192); /* should avoid reallocs for the headers */
+	strbuf_addf(&buffer, "tree %s\n", sha1_to_hex(tree_sha1));
 
 	/*
 	 * NOTE! This ordering means that the same exact tree merged with a
@@ -127,26 +96,24 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	 * if everything else stays the same.
 	 */
 	for (i = 0; i < parents; i++)
-		add_buffer(&buffer, &size, "parent %s\n", sha1_to_hex(parent_sha1[i]));
+		strbuf_addf(&buffer, "parent %s\n", sha1_to_hex(parent_sha1[i]));
 
 	/* Person/date information */
-	add_buffer(&buffer, &size, "author %s\n", git_author_info(1));
-	add_buffer(&buffer, &size, "committer %s\n", git_committer_info(1));
+	strbuf_addf(&buffer, "author %s\n", git_author_info(1));
+	strbuf_addf(&buffer, "committer %s\n", git_committer_info(1));
 	if (!encoding_is_utf8)
-		add_buffer(&buffer, &size,
-				"encoding %s\n", git_commit_encoding);
-	add_buffer(&buffer, &size, "\n");
+		strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
+	strbuf_addch(&buffer, '\n');
 
 	/* And add the comment */
-	while (fgets(comment, sizeof(comment), stdin) != NULL)
-		add_buffer(&buffer, &size, "%s", comment);
+	if (strbuf_read(&buffer, 0) < 0)
+		die("git-commit-tree: read returned %s", strerror(errno));
 
 	/* And check the encoding */
-	buffer[size] = '\0';
-	if (encoding_is_utf8 && !is_utf8(buffer))
+	if (encoding_is_utf8 && !is_utf8(buffer.buf))
 		fprintf(stderr, commit_utf8_warn);
 
-	if (!write_sha1_file(buffer, size, commit_type, commit_sha1)) {
+	if (!write_sha1_file(buffer.buf, buffer.len, commit_type, commit_sha1)) {
 		printf("%s\n", sha1_to_hex(commit_sha1));
 		return 0;
 	}
diff --git a/diff.c b/diff.c
index 0d30d05..05721f8 100644
--- a/diff.c
+++ b/diff.c
@@ -9,6 +9,7 @@
 #include "xdiff-interface.h"
 #include "color.h"
 #include "attr.h"
+#include "strbuf.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0
@@ -1546,25 +1547,15 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
 static int populate_from_stdin(struct diff_filespec *s)
 {
 #define INCREMENT 1024
-	char *buf;
-	unsigned long size;
-	ssize_t got;
-
-	size = 0;
-	buf = NULL;
-	while (1) {
-		buf = xrealloc(buf, size + INCREMENT);
-		got = xread(0, buf + size, INCREMENT);
-		if (!got)
-			break; /* EOF */
-		if (got < 0)
-			return error("error while reading from stdin %s",
+	struct strbuf buf = STRBUF_INIT;
+
+	if (strbuf_read(&buf, 0) < 0)
+		return error("error while reading from stdin %s",
 				     strerror(errno));
-		size += got;
-	}
+
 	s->should_munmap = 0;
-	s->data = buf;
-	s->size = size;
+	s->size = buf.len;
+	s->data = strbuf_detach(&buf);
 	s->should_free = 1;
 	return 0;
 }
-- 
1.5.3

[PATCH] Eradicate yet-another-buffer implementation in buitin-rerere.c

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

Signed-off-by: Pierre Habouzit <redacted>
---
 builtin-rerere.c |   53 +++++++++++++++--------------------------------------
 1 files changed, 15 insertions(+), 38 deletions(-)
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 29d057c..2025004 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "path-list.h"
+#include "strbuf.h"
 #include "xdiff/xdiff.h"
 #include "xdiff-interface.h"
 
@@ -66,38 +67,14 @@ static int write_rr(struct path_list *rr, int out_fd)
 	return commit_lock_file(&write_lock);
 }
 
-struct buffer {
-	char *ptr;
-	int nr, alloc;
-};
-
-static void append_line(struct buffer *buffer, const char *line)
-{
-	int len = strlen(line);
-
-	if (buffer->nr + len > buffer->alloc) {
-		buffer->alloc = alloc_nr(buffer->nr + len);
-		buffer->ptr = xrealloc(buffer->ptr, buffer->alloc);
-	}
-	memcpy(buffer->ptr + buffer->nr, line, len);
-	buffer->nr += len;
-}
-
-static void clear_buffer(struct buffer *buffer)
-{
-	free(buffer->ptr);
-	buffer->ptr = NULL;
-	buffer->nr = buffer->alloc = 0;
-}
-
 static int handle_file(const char *path,
 	 unsigned char *sha1, const char *output)
 {
 	SHA_CTX ctx;
 	char buf[1024];
 	int hunk = 0, hunk_no = 0;
-	struct buffer minus = { NULL, 0, 0 }, plus = { NULL, 0, 0 };
-	struct buffer *one = &minus, *two = &plus;
+	struct strbuf minus = STRBUF_INIT, plus = STRBUF_INIT;
+	struct strbuf *one = &minus, *two = &plus;
 	FILE *f = fopen(path, "r");
 	FILE *out;
 
@@ -122,36 +99,36 @@ static int handle_file(const char *path,
 		else if (!prefixcmp(buf, "======="))
 			hunk = 2;
 		else if (!prefixcmp(buf, ">>>>>>> ")) {
-			int one_is_longer = (one->nr > two->nr);
-			int common_len = one_is_longer ? two->nr : one->nr;
-			int cmp = memcmp(one->ptr, two->ptr, common_len);
+			int one_is_longer = (one->len > two->len);
+			int common_len = one_is_longer ? two->len : one->len;
+			int cmp = memcmp(one->buf, two->buf, common_len);
 
 			hunk_no++;
 			hunk = 0;
 			if ((cmp > 0) || ((cmp == 0) && one_is_longer)) {
-				struct buffer *swap = one;
+				struct strbuf *swap = one;
 				one = two;
 				two = swap;
 			}
 			if (out) {
 				fputs("<<<<<<<\n", out);
-				fwrite(one->ptr, one->nr, 1, out);
+				fwrite(one->buf, one->len, 1, out);
 				fputs("=======\n", out);
-				fwrite(two->ptr, two->nr, 1, out);
+				fwrite(two->buf, two->len, 1, out);
 				fputs(">>>>>>>\n", out);
 			}
 			if (sha1) {
-				SHA1_Update(&ctx, one->ptr, one->nr);
+				SHA1_Update(&ctx, one->buf, one->len);
 				SHA1_Update(&ctx, "\0", 1);
-				SHA1_Update(&ctx, two->ptr, two->nr);
+				SHA1_Update(&ctx, two->buf, two->len);
 				SHA1_Update(&ctx, "\0", 1);
 			}
-			clear_buffer(one);
-			clear_buffer(two);
+			strbuf_release(one);
+			strbuf_release(two);
 		} else if (hunk == 1)
-			append_line(one, buf);
+			strbuf_addstr(one, buf);
 		else if (hunk == 2)
-			append_line(two, buf);
+			strbuf_addstr(two, buf);
 		else if (out)
 			fputs(buf, out);
 	}
-- 
1.5.3

[PATCH] More strbuf uses in cache-tree.c.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

  Should even be marginally faster.

Signed-off-by: Pierre Habouzit <redacted>
---
 cache-tree.c |   57 ++++++++++++++++++++-------------------------------------
 1 files changed, 20 insertions(+), 37 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c
index 077f034..27ca062 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "strbuf.h"
 #include "tree.h"
 #include "cache-tree.h"
 
@@ -235,8 +236,7 @@ static int update_one(struct cache_tree *it,
 		      int missing_ok,
 		      int dryrun)
 {
-	unsigned long size, offset;
-	char *buffer;
+	struct strbuf buffer = STRBUF_INIT;
 	int i;
 
 	if (0 <= it->entry_count && has_sha1_file(it->sha1))
@@ -293,9 +293,7 @@ static int update_one(struct cache_tree *it,
 	/*
 	 * Then write out the tree object for this level.
 	 */
-	size = 8192;
-	buffer = xmalloc(size);
-	offset = 0;
+	strbuf_grow(&buffer, 8192);
 
 	for (i = 0; i < entries; i++) {
 		struct cache_entry *ce = cache[i];
@@ -332,15 +330,9 @@ static int update_one(struct cache_tree *it,
 		if (!ce->ce_mode)
 			continue; /* entry being removed */
 
-		if (size < offset + entlen + 100) {
-			size = alloc_nr(offset + entlen + 100);
-			buffer = xrealloc(buffer, size);
-		}
-		offset += sprintf(buffer + offset,
-				  "%o %.*s", mode, entlen, path + baselen);
-		buffer[offset++] = 0;
-		hashcpy((unsigned char*)buffer + offset, sha1);
-		offset += 20;
+		strbuf_grow(&buffer, entlen + 100);
+		strbuf_addf(&buffer, "%o %.*s%c", mode, entlen, path + baselen, '\0');
+		strbuf_add(&buffer, sha1, 20);
 
 #if DEBUG
 		fprintf(stderr, "cache-tree update-one %o %.*s\n",
@@ -349,10 +341,10 @@ static int update_one(struct cache_tree *it,
 	}
 
 	if (dryrun)
-		hash_sha1_file(buffer, offset, tree_type, it->sha1);
+		hash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
 	else
-		write_sha1_file(buffer, offset, tree_type, it->sha1);
-	free(buffer);
+		write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);
+	strbuf_release(&buffer);
 	it->entry_count = i;
 #if DEBUG
 	fprintf(stderr, "cache-tree update-one (%d ent, %d subtree) %s\n",
@@ -378,12 +370,10 @@ int cache_tree_update(struct cache_tree *it,
 	return 0;
 }
 
-static void *write_one(struct cache_tree *it,
+static void write_one(struct cache_tree *it,
 		       char *path,
 		       int pathlen,
-		       char *buffer,
-		       unsigned long *size,
-		       unsigned long *offset)
+			   struct strbuf *buffer)
 {
 	int i;
 
@@ -393,13 +383,9 @@ static void *write_one(struct cache_tree *it,
 	 * tree-sha1 (missing if invalid)
 	 * subtree_nr "cache-tree" entries for subtrees.
 	 */
-	if (*size < *offset + pathlen + 100) {
-		*size = alloc_nr(*offset + pathlen + 100);
-		buffer = xrealloc(buffer, *size);
-	}
-	*offset += sprintf(buffer + *offset, "%.*s%c%d %d\n",
-			   pathlen, path, 0,
-			   it->entry_count, it->subtree_nr);
+	strbuf_grow(buffer, pathlen + 100);
+	strbuf_add(buffer, path, pathlen);
+	strbuf_addf(buffer, "%c%d %d\n", 0, it->entry_count, it->subtree_nr);
 
 #if DEBUG
 	if (0 <= it->entry_count)
@@ -412,8 +398,7 @@ static void *write_one(struct cache_tree *it,
 #endif
 
 	if (0 <= it->entry_count) {
-		hashcpy((unsigned char*)buffer + *offset, it->sha1);
-		*offset += 20;
+		strbuf_add(buffer, it->sha1, 20);
 	}
 	for (i = 0; i < it->subtree_nr; i++) {
 		struct cache_tree_sub *down = it->down[i];
@@ -423,21 +408,19 @@ static void *write_one(struct cache_tree *it,
 					     prev->name, prev->namelen) <= 0)
 				die("fatal - unsorted cache subtree");
 		}
-		buffer = write_one(down->cache_tree, down->name, down->namelen,
-				   buffer, size, offset);
+		write_one(down->cache_tree, down->name, down->namelen, buffer);
 	}
-	return buffer;
 }
 
 void *cache_tree_write(struct cache_tree *root, unsigned long *size_p)
 {
 	char path[PATH_MAX];
-	unsigned long size = 8192;
-	char *buffer = xmalloc(size);
+	struct strbuf buffer = STRBUF_INIT;
 
-	*size_p = 0;
 	path[0] = 0;
-	return write_one(root, path, 0, buffer, &size, size_p);
+	write_one(root, path, 0, &buffer);
+	*size_p = buffer.len;
+	return strbuf_detach(&buffer);
 }
 
 static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)
-- 
1.5.3

Re: [PATCH] Simplify strbuf uses in fast-import.c using the proper functions.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:33

  Sorry, this one is a false positive :|
  I should have been more careful.

On Wed, Sep 05, 2007 at 07:18:38PM +0000, Pierre Habouzit wrote:
  This is just cleaner way to deal with strbufs, using its API rather than
reinventing it in the module (e.g. strbuf_append_string is just the plain
strbuf_addstr function, and it was used to perform what strbuf_addch does
anyways).
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help