{"thread":{"id":"9866","subject":"[RFC] strbuf's in builtin-apply","startedAt":"2007-09-15T13:56:50Z","lastAt":"2007-09-17T05:43:30Z","messageCount":21,"participants":["Pierre Habouzit","Junio C Hamano","Linus Torvalds","Florian Weimer"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"53117","messageId":"20070915141335.ECB4B4C152@madism.org","threadId":"9866","inReplyTo":"20070915141210.GA27494@artemis.corp","subject":"[PATCH] New strbuf APIs: splice and embed.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T13:56:50Z","receivedAt":"2007-09-15T13:56:50Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"* strbuf_splice replace a portion of the buffer with another.\n* strbuf_embed replace a strbuf buffer with the given one, that should be\n  malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this\n  function has negligible cost, else it will perform a realloc, possibly\n  with a cost.\n---\n strbuf.c |   31 +++++++++++++++++++++++++++++--\n strbuf.h |    5 +++++\n 2 files changed, 34 insertions(+), 2 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex d919047..ad82eaf 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -24,14 +24,22 @@ char *strbuf_detach(struct strbuf *sb) {\n \treturn res;\n }\n \n+void strbuf_embed(struct strbuf *sb, void *buf, size_t len, size_t alloc) {\n+\tstrbuf_release(sb);\n+\tsb->buf   = buf;\n+\tsb->len   = len;\n+\tsb->alloc = alloc;\n+\tstrbuf_grow(sb, 0);\n+\tsb->buf[sb->len] = '\\0';\n+}\n+\n void strbuf_grow(struct strbuf *sb, size_t extra) {\n \tif (sb->len + extra + 1 <= sb->len)\n \t\tdie(\"you want to use way too much memory\");\n \tALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);\n }\n \n-void strbuf_rtrim(struct strbuf *sb)\n-{\n+void strbuf_rtrim(struct strbuf *sb) {\n \twhile (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))\n \t\tsb->len--;\n \tsb->buf[sb->len] = '\\0';\n@@ -48,6 +56,25 @@ void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \n+void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n+\t\t\t\t   const void *data, size_t dlen)\n+{\n+\tif (pos + len < pos)\n+\t\tdie(\"you want to splice outside from the buffer\");\n+\tif (pos > sb->len)\n+\t\tpos = sb->len;\n+\tif (pos + len > sb->len)\n+\t\tlen = sb->len - pos;\n+\n+\tif (dlen >= len)\n+\t\tstrbuf_grow(sb, dlen - len);\n+\tmemmove(sb->buf + pos + dlen,\n+\t\t\tsb->buf + pos + len,\n+\t\t\tsb->len - pos - len);\n+\tmemcpy(sb->buf + pos, data, dlen);\n+\tstrbuf_setlen(sb, sb->len + dlen - len);\n+}\n+\n void strbuf_add(struct strbuf *sb, const void *data, size_t len) {\n \tstrbuf_grow(sb, len);\n \tmemcpy(sb->buf + sb->len, data, len);\ndiff --git a/strbuf.h b/strbuf.h\nindex 21fc111..7e009d1 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -55,6 +55,7 @@ extern void strbuf_init(struct strbuf *, size_t);\n extern void strbuf_release(struct strbuf *);\n extern void strbuf_reset(struct strbuf *);\n extern char *strbuf_detach(struct strbuf *);\n+extern void strbuf_embed(struct strbuf *, void *, size_t, size_t);\n \n /*----- strbuf size related -----*/\n static inline size_t strbuf_avail(struct strbuf *sb) {\n@@ -81,6 +82,10 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {\n /* inserts after pos, or appends if pos >= sb->len */\n extern void strbuf_insert(struct strbuf *, size_t pos, const void *, size_t);\n \n+/* splice pos..pos+len with given data */\n+extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,\n+\t\t\t\t\t\t  const void *, size_t);\n+\n extern void strbuf_add(struct strbuf *, const void *, size_t);\n static inline void strbuf_addstr(struct strbuf *sb, const char *s) {\n \tstrbuf_add(sb, s, strlen(s));\n-- \n1.5.3.1\n"},{"id":"53199","messageId":"20070916172233.8B8AD1835F@madism.org","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"[PATCH] New strbuf APIs: splice and attach.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T13:56:50Z","receivedAt":"2007-09-15T13:56:50Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"* strbuf_splice replace a portion of the buffer with another.\n* strbuf_attach replace a strbuf buffer with the given one, that should be\n  malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this\n  function has negligible cost, else it will perform a realloc, possibly\n  with a cost.\n\nAlso some style issues are fixed now.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n strbuf.c |   67 ++++++++++++++++++++++++++++++++++++++++++++++++-------------\n strbuf.h |    5 ++++\n 2 files changed, 57 insertions(+), 15 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex d919047..ff551ac 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,30 +1,45 @@\n #include \"cache.h\"\n #include \"strbuf.h\"\n \n-void strbuf_init(struct strbuf *sb, size_t hint) {\n+void strbuf_init(struct strbuf *sb, size_t hint)\n+{\n \tmemset(sb, 0, sizeof(*sb));\n \tif (hint)\n \t\tstrbuf_grow(sb, hint);\n }\n \n-void strbuf_release(struct strbuf *sb) {\n+void strbuf_release(struct strbuf *sb)\n+{\n \tfree(sb->buf);\n \tmemset(sb, 0, sizeof(*sb));\n }\n \n-void strbuf_reset(struct strbuf *sb) {\n+void strbuf_reset(struct strbuf *sb)\n+{\n \tif (sb->len)\n \t\tstrbuf_setlen(sb, 0);\n \tsb->eof = 0;\n }\n \n-char *strbuf_detach(struct strbuf *sb) {\n+char *strbuf_detach(struct strbuf *sb)\n+{\n \tchar *res = sb->buf;\n \tstrbuf_init(sb, 0);\n \treturn res;\n }\n \n-void strbuf_grow(struct strbuf *sb, size_t extra) {\n+void strbuf_attach(struct strbuf *sb, void *buf, size_t len, size_t alloc)\n+{\n+\tstrbuf_release(sb);\n+\tsb->buf   = buf;\n+\tsb->len   = len;\n+\tsb->alloc = alloc;\n+\tstrbuf_grow(sb, 0);\n+\tsb->buf[sb->len] = '\\0';\n+}\n+\n+void strbuf_grow(struct strbuf *sb, size_t extra)\n+{\n \tif (sb->len + extra + 1 <= sb->len)\n \t\tdie(\"you want to use way too much memory\");\n \tALLOC_GROW(sb->buf, sb->len + extra + 1, sb->alloc);\n@@ -37,24 +52,44 @@ void strbuf_rtrim(struct strbuf *sb)\n \tsb->buf[sb->len] = '\\0';\n }\n \n-void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len) {\n+void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)\n+{\n \tstrbuf_grow(sb, len);\n-\tif (pos >= sb->len) {\n-\t\tpos = sb->len;\n-\t} else {\n-\t\tmemmove(sb->buf + pos + len, sb->buf + pos, sb->len - pos);\n-\t}\n+\tif (pos > sb->len)\n+\t\tdie(\"`pos' is too far after the end of the buffer\");\n+\tmemmove(sb->buf + pos + len, sb->buf + pos, sb->len - pos);\n \tmemcpy(sb->buf + pos, data, len);\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \n-void strbuf_add(struct strbuf *sb, const void *data, size_t len) {\n+void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n+\t\t\t\t   const void *data, size_t dlen)\n+{\n+\tif (pos + len < pos)\n+\t\tdie(\"you want to use way too much memory\");\n+\tif (pos > sb->len)\n+\t\tdie(\"`pos' is too far after the end of the buffer\");\n+\tif (pos + len > sb->len)\n+\t\tdie(\"`pos + len' is too far after the end of the buffer\");\n+\n+\tif (dlen >= len)\n+\t\tstrbuf_grow(sb, dlen - len);\n+\tmemmove(sb->buf + pos + dlen,\n+\t\t\tsb->buf + pos + len,\n+\t\t\tsb->len - pos - len);\n+\tmemcpy(sb->buf + pos, data, dlen);\n+\tstrbuf_setlen(sb, sb->len + dlen - len);\n+}\n+\n+void strbuf_add(struct strbuf *sb, const void *data, size_t len)\n+{\n \tstrbuf_grow(sb, len);\n \tmemcpy(sb->buf + sb->len, data, len);\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \n-void strbuf_addf(struct strbuf *sb, const char *fmt, ...) {\n+void strbuf_addf(struct strbuf *sb, const char *fmt, ...)\n+{\n \tint len;\n \tva_list ap;\n \n@@ -76,7 +111,8 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...) {\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \n-size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f) {\n+size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n+{\n \tsize_t res;\n \n \tstrbuf_grow(sb, size);\n@@ -110,7 +146,8 @@ ssize_t strbuf_read(struct strbuf *sb, int fd, size_t hint)\n \treturn sb->len - oldlen;\n }\n \n-void read_line(struct strbuf *sb, FILE *fp, int term) {\n+void read_line(struct strbuf *sb, FILE *fp, int term)\n+{\n \tint ch;\n \tif (feof(fp)) {\n \t\tstrbuf_release(sb);\ndiff --git a/strbuf.h b/strbuf.h\nindex 21fc111..f163c63 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -55,6 +55,7 @@ extern void strbuf_init(struct strbuf *, size_t);\n extern void strbuf_release(struct strbuf *);\n extern void strbuf_reset(struct strbuf *);\n extern char *strbuf_detach(struct strbuf *);\n+extern void strbuf_attach(struct strbuf *, void *, size_t, size_t);\n \n /*----- strbuf size related -----*/\n static inline size_t strbuf_avail(struct strbuf *sb) {\n@@ -81,6 +82,10 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {\n /* inserts after pos, or appends if pos >= sb->len */\n extern void strbuf_insert(struct strbuf *, size_t pos, const void *, size_t);\n \n+/* splice pos..pos+len with given data */\n+extern void strbuf_splice(struct strbuf *, size_t pos, size_t len,\n+\t\t\t\t\t\t  const void *, size_t);\n+\n extern void strbuf_add(struct strbuf *, const void *, size_t);\n static inline void strbuf_addstr(struct strbuf *sb, const char *s) {\n \tstrbuf_add(sb, s, strlen(s));\n-- \n1.5.3.1\n"},{"id":"53200","messageId":"20070916172233.9798318361@madism.org","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"[PATCH] Now that cache.h needs strbuf.h, remove useless includes.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T13:56:50Z","receivedAt":"2007-09-15T13:56:50Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n archive-tar.c            |    1 -\n builtin-apply.c          |    1 -\n builtin-blame.c          |    1 -\n builtin-checkout-index.c |    1 -\n builtin-commit-tree.c    |    1 -\n builtin-fetch--tool.c    |    1 -\n builtin-rerere.c         |    1 -\n builtin-stripspace.c     |    1 -\n builtin-tag.c            |    1 -\n builtin-update-index.c   |    1 -\n cache-tree.c             |    1 -\n convert.c                |    1 -\n diff.c                   |    1 -\n fast-import.c            |    1 -\n fetch.c                  |    1 -\n imap-send.c              |    1 -\n mktag.c                  |    1 -\n mktree.c                 |    1 -\n sha1_file.c              |    1 -\n strbuf.c                 |    1 -\n 20 files changed, 0 insertions(+), 20 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex cc94cf3..a87bc4b 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -3,7 +3,6 @@\n  */\n #include \"cache.h\"\n #include \"commit.h\"\n-#include \"strbuf.h\"\n #include \"tar.h\"\n #include \"builtin.h\"\n #include \"archive.h\"\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 9735b47..ae708d7 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -12,7 +12,6 @@\n #include \"blob.h\"\n #include \"delta.h\"\n #include \"builtin.h\"\n-#include \"strbuf.h\"\n \n /*\n  *  --check turns on checking that the working tree matches the\ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex b004f06..e364b6c 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -18,7 +18,6 @@\n #include \"cache-tree.h\"\n #include \"path-list.h\"\n #include \"mailmap.h\"\n-#include \"strbuf.h\"\n \n static char blame_usage[] =\n \"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\"\ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex 153ba7d..85e8efe 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -38,7 +38,6 @@\n  */\n #include \"builtin.h\"\n #include \"cache.h\"\n-#include \"strbuf.h\"\n #include \"quote.h\"\n #include \"cache-tree.h\"\n \ndiff --git a/builtin-commit-tree.c b/builtin-commit-tree.c\nindex 325334f..88b0ab3 100644\n--- a/builtin-commit-tree.c\n+++ b/builtin-commit-tree.c\n@@ -8,7 +8,6 @@\n #include \"tree.h\"\n #include \"builtin.h\"\n #include \"utf8.h\"\n-#include \"strbuf.h\"\n \n #define BLOCKING (1ul << 14)\n \ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex 90bdc32..514a3cc 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -2,7 +2,6 @@\n #include \"cache.h\"\n #include \"refs.h\"\n #include \"commit.h\"\n-#include \"strbuf.h\"\n \n static char *get_stdin(void)\n {\ndiff --git a/builtin-rerere.c b/builtin-rerere.c\nindex 826d346..58288f6 100644\n--- a/builtin-rerere.c\n+++ b/builtin-rerere.c\n@@ -1,7 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n #include \"path-list.h\"\n-#include \"strbuf.h\"\n #include \"xdiff/xdiff.h\"\n #include \"xdiff-interface.h\"\n \ndiff --git a/builtin-stripspace.c b/builtin-stripspace.c\nindex c4cf2f0..1ce2847 100644\n--- a/builtin-stripspace.c\n+++ b/builtin-stripspace.c\n@@ -1,6 +1,5 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n-#include \"strbuf.h\"\n \n /*\n  * Returns the length of a line, without trailing spaces.\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 9f29342..82ebda1 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -8,7 +8,6 @@\n \n #include \"cache.h\"\n #include \"builtin.h\"\n-#include \"strbuf.h\"\n #include \"refs.h\"\n #include \"tag.h\"\n #include \"run-command.h\"\ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex e02431c..0b60513 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -4,7 +4,6 @@\n  * Copyright (C) Linus Torvalds, 2005\n  */\n #include \"cache.h\"\n-#include \"strbuf.h\"\n #include \"quote.h\"\n #include \"cache-tree.h\"\n #include \"tree-walk.h\"\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 8f53c99..5471844 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -1,5 +1,4 @@\n #include \"cache.h\"\n-#include \"strbuf.h\"\n #include \"tree.h\"\n #include \"cache-tree.h\"\n \ndiff --git a/convert.c b/convert.c\nindex 00a341c..508d30b 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1,7 +1,6 @@\n #include \"cache.h\"\n #include \"attr.h\"\n #include \"run-command.h\"\n-#include \"strbuf.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\ndiff --git a/diff.c b/diff.c\nindex d46dd11..a5b69ed 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -9,7 +9,6 @@\n #include \"xdiff-interface.h\"\n #include \"color.h\"\n #include \"attr.h\"\n-#include \"strbuf.h\"\n \n #ifdef NO_FAST_WORKING_DIRECTORY\n #define FAST_WORKING_DIRECTORY 0\ndiff --git a/fast-import.c b/fast-import.c\nindex 2c0bfb9..1866d34 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -149,7 +149,6 @@ Format of STDIN stream:\n #include \"pack.h\"\n #include \"refs.h\"\n #include \"csum-file.h\"\n-#include \"strbuf.h\"\n #include \"quote.h\"\n \n #define PACK_ID_BITS 16\ndiff --git a/fetch.c b/fetch.c\nindex dd6ed9e..c256e6f 100644\n--- a/fetch.c\n+++ b/fetch.c\n@@ -6,7 +6,6 @@\n #include \"tag.h\"\n #include \"blob.h\"\n #include \"refs.h\"\n-#include \"strbuf.h\"\n \n int get_tree = 0;\n int get_history = 0;\ndiff --git a/imap-send.c b/imap-send.c\nindex ecd4216..86e4a0f 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -23,7 +23,6 @@\n  */\n \n #include \"cache.h\"\n-#include \"strbuf.h\"\n \n typedef struct store_conf {\n \tchar *name;\ndiff --git a/mktag.c b/mktag.c\nindex 7567f9e..b05260c 100644\n--- a/mktag.c\n+++ b/mktag.c\n@@ -1,5 +1,4 @@\n #include \"cache.h\"\n-#include \"strbuf.h\"\n #include \"tag.h\"\n \n /*\ndiff --git a/mktree.c b/mktree.c\nindex 3891cd9..5dab4bd 100644\n--- a/mktree.c\n+++ b/mktree.c\n@@ -4,7 +4,6 @@\n  * Copyright (c) Junio C Hamano, 2006\n  */\n #include \"cache.h\"\n-#include \"strbuf.h\"\n #include \"quote.h\"\n #include \"tree.h\"\n \ndiff --git a/sha1_file.c b/sha1_file.c\nindex 64b5b46..59325d4 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -14,7 +14,6 @@\n #include \"tag.h\"\n #include \"tree.h\"\n #include \"refs.h\"\n-#include \"strbuf.h\"\n \n #ifndef O_NOATIME\n #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))\ndiff --git a/strbuf.c b/strbuf.c\nindex ff551ac..c5f9e2a 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,5 +1,4 @@\n #include \"cache.h\"\n-#include \"strbuf.h\"\n \n void strbuf_init(struct strbuf *sb, size_t hint)\n {\n-- \n1.5.3.1\n"},{"id":"53118","messageId":"20070915141340.334CA4C152@madism.org","threadId":"9866","inReplyTo":"20070915141210.GA27494@artemis.corp","subject":"[PATCH] builtin-apply: use strbuf's instead of buffer_desc's.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T14:04:01Z","receivedAt":"2007-09-15T14:04:01Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n builtin-apply.c |  203 ++++++++++++++++++++-----------------------------------\n 1 files changed, 73 insertions(+), 130 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 7057d0d..6c24f40 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1442,39 +1442,34 @@ static void show_stats(struct patch *patch)\n \tfree(qname);\n }\n \n-static int read_old_data(struct stat *st, const char *path, char **buf_p, unsigned long *alloc_p, unsigned long *size_p)\n+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n {\n \tint fd;\n-\tunsigned long got;\n \tunsigned long nsize;\n \tchar *nbuf;\n-\tunsigned long size = *size_p;\n-\tchar *buf = *buf_p;\n \n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n-\t\treturn readlink(path, buf, size) != size;\n+\t\tstrbuf_grow(buf, st->st_size);\n+\t\tif (readlink(path, buf->buf, st->st_size) != st->st_size)\n+\t\t\treturn -1;\n+\t\tstrbuf_setlen(buf, st->st_size);\n+\t\treturn 0;\n \tcase S_IFREG:\n \t\tfd = open(path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\treturn error(\"unable to open %s\", path);\n-\t\tgot = 0;\n-\t\tfor (;;) {\n-\t\t\tssize_t ret = xread(fd, buf + got, size - got);\n-\t\t\tif (ret <= 0)\n-\t\t\t\tbreak;\n-\t\t\tgot += ret;\n+\t\tif (strbuf_read(buf, fd, st->st_size) < 0) {\n+\t\t\tclose(fd);\n+\t\t\treturn -1;\n \t\t}\n \t\tclose(fd);\n-\t\tnsize = got;\n-\t\tnbuf = convert_to_git(path, buf, &nsize);\n-\t\tif (nbuf) {\n-\t\t\tfree(buf);\n-\t\t\t*buf_p = nbuf;\n-\t\t\t*alloc_p = nsize;\n-\t\t\t*size_p = nsize;\n-\t\t}\n-\t\treturn got != size;\n+\n+\t\tnsize = buf->len;\n+\t\tnbuf = convert_to_git(path, buf->buf, &nsize);\n+\t\tif (nbuf)\n+\t\t\tstrbuf_embed(buf, nbuf, nsize, nsize);\n+\t\treturn 0;\n \tdefault:\n \t\treturn -1;\n \t}\n@@ -1579,12 +1574,6 @@ static void remove_last_line(const char **rbuf, int *rsize)\n \t*rsize = offset + 1;\n }\n \n-struct buffer_desc {\n-\tchar *buffer;\n-\tunsigned long size;\n-\tunsigned long alloc;\n-};\n-\n static int apply_line(char *output, const char *patch, int plen)\n {\n \t/* plen is number of bytes to be copied from patch,\n@@ -1654,10 +1643,9 @@ static int apply_line(char *output, const char *patch, int plen)\n \treturn output + plen - buf;\n }\n \n-static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, int inaccurate_eof)\n+static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int inaccurate_eof)\n {\n \tint match_beginning, match_end;\n-\tchar *buf = desc->buffer;\n \tconst char *patch = frag->patch;\n \tint offset, size = frag->size;\n \tchar *old = xmalloc(size);\n@@ -1768,24 +1756,17 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \tlines = 0;\n \tpos = frag->newpos;\n \tfor (;;) {\n-\t\toffset = find_offset(buf, desc->size,\n+\t\toffset = find_offset(buf->buf, buf->len,\n \t\t\t\t     oldlines, oldsize, pos, &lines);\n-\t\tif (match_end && offset + oldsize != desc->size)\n+\t\tif (match_end && offset + oldsize != buf->len)\n \t\t\toffset = -1;\n \t\tif (match_beginning && offset)\n \t\t\toffset = -1;\n \t\tif (offset >= 0) {\n-\t\t\tint diff;\n-\t\t\tunsigned long size, alloc;\n-\n \t\t\tif (new_whitespace == strip_whitespace &&\n-\t\t\t    (desc->size - oldsize - offset == 0)) /* end of file? */\n+\t\t\t    (buf->len - oldsize - offset == 0)) /* end of file? */\n \t\t\t\tnewsize -= new_blank_lines_at_end;\n \n-\t\t\tdiff = newsize - oldsize;\n-\t\t\tsize = desc->size + diff;\n-\t\t\talloc = desc->alloc;\n-\n \t\t\t/* Warn if it was necessary to reduce the number\n \t\t\t * of context lines.\n \t\t\t */\n@@ -1795,19 +1776,8 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\t\t\t\t\" to apply fragment at %d\\n\",\n \t\t\t\t\tleading, trailing, pos + lines);\n \n-\t\t\tif (size > alloc) {\n-\t\t\t\talloc = size + 8192;\n-\t\t\t\tdesc->alloc = alloc;\n-\t\t\t\tbuf = xrealloc(buf, alloc);\n-\t\t\t\tdesc->buffer = buf;\n-\t\t\t}\n-\t\t\tdesc->size = size;\n-\t\t\tmemmove(buf + offset + newsize,\n-\t\t\t\tbuf + offset + oldsize,\n-\t\t\t\tsize - offset - newsize);\n-\t\t\tmemcpy(buf + offset, newlines, newsize);\n+\t\t\tstrbuf_splice(buf, offset, oldsize, newlines, newsize);\n \t\t\toffset = 0;\n-\n \t\t\tbreak;\n \t\t}\n \n@@ -1843,12 +1813,11 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \treturn offset;\n }\n \n-static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)\n+static int apply_binary_fragment(struct strbuf *buf, struct patch *patch)\n {\n-\tunsigned long dst_size;\n \tstruct fragment *fragment = patch->fragments;\n-\tvoid *data;\n-\tvoid *result;\n+\tunsigned long len;\n+\tvoid *dst;\n \n \t/* Binary patch is irreversible without the optional second hunk */\n \tif (apply_in_reverse) {\n@@ -1859,29 +1828,23 @@ static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)\n \t\t\t\t     ? patch->new_name : patch->old_name);\n \t\tfragment = fragment->next;\n \t}\n-\tdata = (void*) fragment->patch;\n \tswitch (fragment->binary_patch_method) {\n \tcase BINARY_DELTA_DEFLATED:\n-\t\tresult = patch_delta(desc->buffer, desc->size,\n-\t\t\t\t     data,\n-\t\t\t\t     fragment->size,\n-\t\t\t\t     &dst_size);\n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = result;\n-\t\tbreak;\n+\t\tdst = patch_delta(buf->buf, buf->len, fragment->patch,\n+\t\t\t\t  fragment->size, &len);\n+\t\tif (!dst)\n+\t\t\treturn -1;\n+\t\t/* XXX patch_delta NUL-terminates */\n+\t\tstrbuf_embed(buf, dst, len, len + 1);\n+\t\treturn 0;\n \tcase BINARY_LITERAL_DEFLATED:\n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = data;\n-\t\tdst_size = fragment->size;\n-\t\tbreak;\n+\t\tstrbuf_embed(buf, fragment->patch, fragment->size, fragment->size);\n+\t\treturn 0;\n \t}\n-\tif (!desc->buffer)\n-\t\treturn -1;\n-\tdesc->size = desc->alloc = dst_size;\n-\treturn 0;\n+\treturn -1;\n }\n \n-static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n+static int apply_binary(struct strbuf *buf, struct patch *patch)\n {\n \tconst char *name = patch->old_name ? patch->old_name : patch->new_name;\n \tunsigned char sha1[20];\n@@ -1900,7 +1863,7 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n \t\t/* See if the old one matches what the patch\n \t\t * applies to.\n \t\t */\n-\t\thash_sha1_file(desc->buffer, desc->size, blob_type, sha1);\n+\t\thash_sha1_file(buf->buf, buf->len, blob_type, sha1);\n \t\tif (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))\n \t\t\treturn error(\"the patch applies to '%s' (%s), \"\n \t\t\t\t     \"which does not match the \"\n@@ -1909,16 +1872,14 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n \t}\n \telse {\n \t\t/* Otherwise, the old one must be empty. */\n-\t\tif (desc->size)\n+\t\tif (buf->len)\n \t\t\treturn error(\"the patch applies to an empty \"\n \t\t\t\t     \"'%s' but it is not empty\", name);\n \t}\n \n \tget_sha1_hex(patch->new_sha1_prefix, sha1);\n \tif (is_null_sha1(sha1)) {\n-\t\tfree(desc->buffer);\n-\t\tdesc->alloc = desc->size = 0;\n-\t\tdesc->buffer = NULL;\n+\t\tstrbuf_release(buf);\n \t\treturn 0; /* deletion patch */\n \t}\n \n@@ -1926,43 +1887,44 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n \t\t/* We already have the postimage */\n \t\tenum object_type type;\n \t\tunsigned long size;\n+\t\tchar *result;\n \n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = read_sha1_file(sha1, &type, &size);\n-\t\tif (!desc->buffer)\n+\t\tresult = read_sha1_file(sha1, &type, &size);\n+\t\tif (!result)\n \t\t\treturn error(\"the necessary postimage %s for \"\n \t\t\t\t     \"'%s' cannot be read\",\n \t\t\t\t     patch->new_sha1_prefix, name);\n-\t\tdesc->alloc = desc->size = size;\n-\t}\n-\telse {\n-\t\t/* We have verified desc matches the preimage;\n+\t\t/* XXX read_sha1_file NUL-terminates */\n+\t\tstrbuf_embed(buf, result, size, size + 1);\n+\t} else {\n+\t\t/* We have verified buf matches the preimage;\n \t\t * apply the patch data to it, which is stored\n \t\t * in the patch->fragments->{patch,size}.\n \t\t */\n-\t\tif (apply_binary_fragment(desc, patch))\n+\t\tif (apply_binary_fragment(buf, patch))\n \t\t\treturn error(\"binary patch does not apply to '%s'\",\n \t\t\t\t     name);\n \n \t\t/* verify that the result matches */\n-\t\thash_sha1_file(desc->buffer, desc->size, blob_type, sha1);\n+\t\thash_sha1_file(buf->buf, buf->len, blob_type, sha1);\n \t\tif (strcmp(sha1_to_hex(sha1), patch->new_sha1_prefix))\n-\t\t\treturn error(\"binary patch to '%s' creates incorrect result (expecting %s, got %s)\", name, patch->new_sha1_prefix, sha1_to_hex(sha1));\n+\t\t\treturn error(\"binary patch to '%s' creates incorrect result (expecting %s, got %s)\",\n+\t\t\t\tname, patch->new_sha1_prefix, sha1_to_hex(sha1));\n \t}\n \n \treturn 0;\n }\n \n-static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n+static int apply_fragments(struct strbuf *buf, struct patch *patch)\n {\n \tstruct fragment *frag = patch->fragments;\n \tconst char *name = patch->old_name ? patch->old_name : patch->new_name;\n \n \tif (patch->is_binary)\n-\t\treturn apply_binary(desc, patch);\n+\t\treturn apply_binary(buf, patch);\n \n \twhile (frag) {\n-\t\tif (apply_one_fragment(desc, frag, patch->inaccurate_eof)) {\n+\t\tif (apply_one_fragment(buf, frag, patch->inaccurate_eof)) {\n \t\t\terror(\"patch failed: %s:%ld\", name, frag->oldpos);\n \t\t\tif (!apply_with_reject)\n \t\t\t\treturn -1;\n@@ -1973,76 +1935,57 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n-static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n-\t\t\t\tunsigned long *size_p)\n+static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n {\n \tif (!ce)\n \t\treturn 0;\n \n \tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n-\t\t*buf_p = xmalloc(100);\n-\t\t*size_p = snprintf(*buf_p, 100,\n-\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t\tstrbuf_grow(buf, 100);\n+\t\tstrbuf_addf(buf, \"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n \t} else {\n \t\tenum object_type type;\n-\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n-\t\tif (!*buf_p)\n+\t\tunsigned long sz;\n+\t\tchar *result;\n+\n+\t\tresult = read_sha1_file(ce->sha1, &type, &sz);\n+\t\tif (!result)\n \t\t\treturn -1;\n+\t\t/* XXX read_sha1_file NUL-terminates */\n+\t\tstrbuf_embed(buf, result, sz, sz + 1);\n \t}\n \treturn 0;\n }\n \n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n-\tchar *buf;\n-\tunsigned long size, alloc;\n-\tstruct buffer_desc desc;\n+\tstruct strbuf buf;\n \n-\tsize = 0;\n-\talloc = 0;\n-\tbuf = NULL;\n+\tstrbuf_init(&buf, 0);\n \tif (cached) {\n-\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\tif (read_file_or_gitlink(ce, &buf))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n-\t\talloc = size;\n \t} else if (patch->old_name) {\n \t\tif (S_ISGITLINK(patch->old_mode)) {\n-\t\t\tif (ce)\n-\t\t\t\tread_file_or_gitlink(ce, &buf, &size);\n-\t\t\telse {\n+\t\t\tif (ce) {\n+\t\t\t\tread_file_or_gitlink(ce, &buf);\n+\t\t\t} else {\n \t\t\t\t/*\n \t\t\t\t * There is no way to apply subproject\n \t\t\t\t * patch without looking at the index.\n \t\t\t\t */\n \t\t\t\tpatch->fragments = NULL;\n-\t\t\t\tsize = 0;\n \t\t\t}\n-\t\t}\n-\t\telse {\n-\t\t\tsize = xsize_t(st->st_size);\n-\t\t\talloc = size + 8192;\n-\t\t\tbuf = xmalloc(alloc);\n-\t\t\tif (read_old_data(st, patch->old_name,\n-\t\t\t\t\t  &buf, &alloc, &size))\n-\t\t\t\treturn error(\"read of %s failed\",\n-\t\t\t\t\t     patch->old_name);\n+\t\t} else {\n+\t\t\tif (read_old_data(st, patch->old_name, &buf))\n+\t\t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t\t}\n \t}\n \n-\tdesc.size = size;\n-\tdesc.alloc = alloc;\n-\tdesc.buffer = buf;\n-\n-\tif (apply_fragments(&desc, patch) < 0)\n+\tif (apply_fragments(&buf, patch) < 0)\n \t\treturn -1; /* note with --reject this succeeds. */\n-\n-\t/* NUL terminate the result */\n-\tif (desc.alloc <= desc.size)\n-\t\tdesc.buffer = xrealloc(desc.buffer, desc.size + 1);\n-\tdesc.buffer[desc.size] = 0;\n-\n-\tpatch->result = desc.buffer;\n-\tpatch->resultsize = desc.size;\n+\tpatch->result = buf.buf;\n+\tpatch->resultsize = buf.len;\n \n \tif (0 < patch->is_delete && patch->resultsize)\n \t\treturn error(\"removal patch leaves file contents\");\n-- \n1.5.3.1\n"},{"id":"53116","messageId":"20070915141210.GA27494@artemis.corp","threadId":"9866","inReplyTo":null,"subject":"[RFC] strbuf's in builtin-apply","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T14:12:10Z","receivedAt":"2007-09-15T14:12:10Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"  I promised it, here it comes. It comes though with some possible\nproblems, so I'd like to see them discussed first.\n\n  builtin-apply.c was messing with its custom buffers a lot,\nreallocating the buffers and so on, hence I had to create strbuf_embed\nto have a strbuf-safe API doing exactly the same. Though, because of the\n\"I want an extra NUL in the end\"-invariant, this can come with a quite\nhigh price. in the second patch, there is a hunk:\n\n\n-\t\tnsize = got;\n-\t\tnbuf = convert_to_git(path, buf, &nsize);\n-\t\tif (nbuf) {\n-\t\t\tfree(buf);\n-\t\t\t*buf_p = nbuf;\n-\t\t\t*alloc_p = nsize;\n-\t\t\t*size_p = nsize;\n-\t\t}\n-\t\treturn got != size;\n+\n+\t\tnsize = buf->len;\n+\t\tnbuf = convert_to_git(path, buf->buf, &nsize);\n+\t\tif (nbuf)\n+\t\t\tstrbuf_embed(buf, nbuf, nsize, nsize);\n+\t\treturn 0;\n\n  Here, I've not been able to check if convert_to_git was in fact always\ndealing with a NUL-terminated buffer (That would be in fact nsize+1) or\nnot, hence here this strbuf_embed will likely perform a realloc. I don't\nknow git enough to know if this can become an horrible burden though.\n\n  Another suspicious hunk is:\n\n-\tdata = (void*) fragment->patch;\n[...]\n \tcase BINARY_LITERAL_DEFLATED:\n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = data;\n-\t\tdst_size = fragment->size;\n-\t\tbreak;\n+\t\tstrbuf_embed(buf, fragment->patch, fragment->size, fragment->size);\n+\t\treturn 0;\n\n  TTBOMK the ->patch pointer is a pointer inside a buffer, not a buffer\nthat has been malloc'ed (and there are code paths before my patch that\nwould still realloc the buffer so I don't think I introduce an issue).\nIt passes the test-suite without crashing, but well, maybe this should\nbe a copy instead.\n\n  The rest is pretty straightforward.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53134","messageId":"20070915170727.GF27494@artemis.corp","threadId":"9866","inReplyTo":"20070915141210.GA27494@artemis.corp","subject":"Re: [RFC] strbuf's in builtin-apply","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T17:07:27Z","receivedAt":"2007-09-15T17:07:27Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sat, Sep 15, 2007 at 02:12:10PM +0000, Pierre Habouzit wrote:\n> +\t\tnsize = buf->len;\n> +\t\tnbuf = convert_to_git(path, buf->buf, &nsize);\n> +\t\tif (nbuf)\n> +\t\t\tstrbuf_embed(buf, nbuf, nsize, nsize);\n\n  Okay, I managed to be able to be sure that convert_to_git always have\nan extra ending NUL byte, with an intermediate patch.  So now I call\nstrbuf_embed with nsize, nsize + 1 which has negligible cost.\n\n  Though this question remains:\n\n>   Another suspicious hunk is:\n> \n> -\tdata = (void*) fragment->patch;\n> [...]\n>  \tcase BINARY_LITERAL_DEFLATED:\n> -\t\tfree(desc->buffer);\n> -\t\tdesc->buffer = data;\n> -\t\tdst_size = fragment->size;\n> -\t\tbreak;\n> +\t\tstrbuf_embed(buf, fragment->patch, fragment->size, fragment->size);\n> +\t\treturn 0;\n> \n>   TTBOMK the ->patch pointer is a pointer inside a buffer, not a buffer\n> that has been malloc'ed (and there are code paths before my patch that\n> would still realloc the buffer so I don't think I introduce an issue).\n> It passes the test-suite without crashing, but well, maybe this should\n> be a copy instead.\n> \n>   The rest is pretty straightforward.\n> \n> -- \n> ·O·  Pierre Habouzit\n> ··O                                                madcoder@debian.org\n> OOO                                                http://www.madism.org\n\n\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53198","messageId":"20070916172233.A12D04CA60@madism.org","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"[PATCH] Refactor replace_encoding_header.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-15T21:50:12Z","receivedAt":"2007-09-15T21:50:12Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"* Be more clever in how we search for \"encoding ...\\n\": parse for real\n  instead of the sloppy strstr's.\n* use strbuf_splice to do the substring replacements.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n commit.c |   59 +++++++++++++++++++++++------------------------------------\n 1 files changed, 23 insertions(+), 36 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6602e2c..13af933 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -648,47 +648,34 @@ static char *get_header(const struct commit *commit, const char *key)\n \n static char *replace_encoding_header(char *buf, const char *encoding)\n {\n-\tchar *encoding_header = strstr(buf, \"\\nencoding \");\n-\tchar *header_end = strstr(buf, \"\\n\\n\");\n-\tchar *end_of_encoding_header;\n-\tint encoding_header_pos;\n-\tint encoding_header_len;\n-\tint new_len;\n-\tint need_len;\n-\tint buflen = strlen(buf) + 1;\n-\n-\tif (!header_end)\n-\t\theader_end = buf + buflen;\n-\tif (!encoding_header || encoding_header >= header_end)\n-\t\treturn buf;\n-\tencoding_header++;\n-\tend_of_encoding_header = strchr(encoding_header, '\\n');\n-\tif (!end_of_encoding_header)\n+\tstruct strbuf tmp;\n+\tsize_t start, len;\n+\tchar *cp = buf;\n+\n+\t/* guess if there is an encoding header before a \\n\\n */\n+\twhile (strncmp(cp, \"encoding \", strlen(\"encoding \"))) {\n+\t\tcp = strchr(cp, '\\n');\n+\t\tif (!cp || *++cp == '\\n')\n+\t\t\treturn buf;\n+\t}\n+\tstart = cp - buf;\n+\tcp = strchr(cp, '\\n');\n+\tif (!cp)\n \t\treturn buf; /* should not happen but be defensive */\n-\tend_of_encoding_header++;\n-\n-\tencoding_header_len = end_of_encoding_header - encoding_header;\n-\tencoding_header_pos = encoding_header - buf;\n+\tlen = cp + 1 - (buf + start);\n \n+\tstrbuf_init(&tmp, 0);\n+\tstrbuf_attach(&tmp, buf, strlen(buf), strlen(buf) + 1);\n \tif (is_encoding_utf8(encoding)) {\n \t\t/* we have re-coded to UTF-8; drop the header */\n-\t\tmemmove(encoding_header, end_of_encoding_header,\n-\t\t\tbuflen - (encoding_header_pos + encoding_header_len));\n-\t\treturn buf;\n-\t}\n-\tnew_len = strlen(encoding);\n-\tneed_len = new_len + strlen(\"encoding \\n\");\n-\tif (encoding_header_len < need_len) {\n-\t\tbuf = xrealloc(buf, buflen + (need_len - encoding_header_len));\n-\t\tencoding_header = buf + encoding_header_pos;\n-\t\tend_of_encoding_header = encoding_header + encoding_header_len;\n+\t\tstrbuf_splice(&tmp, start, len, NULL, 0);\n+\t} else {\n+\t\t/* just replaces XXXX in 'encoding XXXX\\n' */\n+\t\tstrbuf_splice(&tmp, start + strlen(\"encoding \"),\n+\t\t\t\t\t  len - strlen(\"encoding \\n\"),\n+\t\t\t\t\t  encoding, strlen(encoding));\n \t}\n-\tmemmove(end_of_encoding_header + (need_len - encoding_header_len),\n-\t\tend_of_encoding_header,\n-\t\tbuflen - (encoding_header_pos + encoding_header_len));\n-\tmemcpy(encoding_header + 9, encoding, strlen(encoding));\n-\tencoding_header[9 + new_len] = '\\n';\n-\treturn buf;\n+\treturn tmp.buf;\n }\n \n static char *logmsg_reencode(const struct commit *commit,\n-- \n1.5.3.1\n"},{"id":"53156","messageId":"7v4phv8m26.fsf@gitster.siamese.dyndns.org","threadId":"9866","inReplyTo":"20070915141340.334CA4C152@madism.org","subject":"Re: [PATCH] builtin-apply: use strbuf's instead of buffer_desc's.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-16T00:56:49Z","receivedAt":"2007-09-16T00:56:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n>  1 files changed, 73 insertions(+), 130 deletions(-)\n\nNice reduction.\n\n> -\t\t}\n> -\t\treturn got != size;\n> +\n> +\t\tnsize = buf->len;\n> +\t\tnbuf = convert_to_git(path, buf->buf, &nsize);\n> +\t\tif (nbuf)\n> +\t\t\tstrbuf_embed(buf, nbuf, nsize, nsize);\n> +\t\treturn 0;\n\n\nI suspect that changing the convert_to_git() interface to work\non strbuf instead of (char*, size_t *) pair might make things\nsimpler and easier.\n"},{"id":"53157","messageId":"7vwsur77g7.fsf@gitster.siamese.dyndns.org","threadId":"9866","inReplyTo":"20070915141335.ECB4B4C152@madism.org","subject":"Re: [PATCH] New strbuf APIs: splice and embed.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-16T00:57:44Z","receivedAt":"2007-09-16T00:57:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> * strbuf_splice replace a portion of the buffer with another.\n> * strbuf_embed replace a strbuf buffer with the given one, that should be\n>   malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this\n>   function has negligible cost, else it will perform a realloc, possibly\n>   with a cost.\n\n\"embed\" does not sound quite right, does it?  It is a reverse\noperation of strbuf_detach() as far as I can tell.\n\n> -void strbuf_rtrim(struct strbuf *sb)\n> -{\n> +void strbuf_rtrim(struct strbuf *sb) {\n>  \twhile (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))\n>  \t\tsb->len--;\n>  \tsb->buf[sb->len] = '\\0';\n\nThis is changing the style in the wrong direction, isn't it?  We\nstart our functions like this:\n\n\ttype name(proto)\n        {\n        \t...\n\n> +void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n> +\t\t\t\t   const void *data, size_t dlen)\n> +{\n> +\tif (pos + len < pos)\n> +\t\tdie(\"you want to splice outside from the buffer\");\n\nThat is a funny error message for an integer wrap-around check.\n\n> +\tif (pos > sb->len)\n> +\t\tpos = sb->len;\n\nShouldn't this be flagged as a programming error?\n\n> +\tif (pos + len > sb->len)\n> +\t\tlen = sb->len - pos;\n\nLikewise.\n\nBy the way, this is the kind of situation I wish everybody wrote\ntheir comparison in textual order.  I had to draw a picture like\nthis to see what was going on.\n\n        sb->buf\n        xxxxxxxxxxxzzzzzxxxxxxxxxxx\\0\n        ^                         ^\n        0                   sb->len\n                   ^    ^               ^    ^        \n                   pos  pos+len         pos  pos+len  \n                   v                    v\n                   yyyyyyyyyy           yyyyyyyyyy    \n                             ^                    ^   \n                             dlen                 dlen\n                   -------------        --------------\n                   pos < sb->len        sb->len < pos\n"},{"id":"53174","messageId":"20070916081012.GA21810@artemis.corp","threadId":"9866","inReplyTo":"7vwsur77g7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] New strbuf APIs: splice and embed.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T08:10:12Z","receivedAt":"2007-09-16T08:10:12Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Sep 16, 2007 at 12:57:44AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> > * strbuf_splice replace a portion of the buffer with another.\n> > * strbuf_embed replace a strbuf buffer with the given one, that should be\n> >   malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this\n> >   function has negligible cost, else it will perform a realloc, possibly\n> >   with a cost.\n> \n> \"embed\" does not sound quite right, does it?  It is a reverse\n> operation of strbuf_detach() as far as I can tell.\n\n  Well I don't like either, and indeed strbuf_attach() seems better.\n\n> > -void strbuf_rtrim(struct strbuf *sb)\n> > -{\n> > +void strbuf_rtrim(struct strbuf *sb) {\n> >  \twhile (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))\n> >  \t\tsb->len--;\n> >  \tsb->buf[sb->len] = '\\0';\n> \n> This is changing the style in the wrong direction, isn't it?  We\n> start our functions like this:\n> \n> \ttype name(proto)\n>         {\n\n  Well that's what I usually do for my code, but I thought git was\nputting the opening brace on the same line, my bad.\n\n> > +void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n> > +\t\t\t\t   const void *data, size_t dlen)\n> > +{\n> > +\tif (pos + len < pos)\n> > +\t\tdie(\"you want to splice outside from the buffer\");\n> \n> That is a funny error message for an integer wrap-around check.\n\n  right ;)\n\n> \n> > +\tif (pos > sb->len)\n> > +\t\tpos = sb->len;\n>\n> Shouldn't this be flagged as a programming error?\n> \n> > +\tif (pos + len > sb->len)\n> > +\t\tlen = sb->len - pos;\n> \n> Likewise.\n\n  I just attached the same semantics that was chose for strbuf_insert\nwhen the insertion position is outside from the buffer. Though I can\nenforce those to stay inside the buffer and just die(). I don't care\nmuch I shall say. Maybe it hides some programmers being sloppy, hence is\na bad semantics. I'll propose a patch where the check die()s instead of\n\"fixing\" the values.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53176","messageId":"20070916081519.GB21810@artemis.corp","threadId":"9866","inReplyTo":"7v4phv8m26.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-apply: use strbuf's instead of buffer_desc's.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T08:15:19Z","receivedAt":"2007-09-16T08:15:19Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Sep 16, 2007 at 12:56:49AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> >  1 files changed, 73 insertions(+), 130 deletions(-)\n> \n> Nice reduction.\n> \n> > -\t\t}\n> > -\t\treturn got != size;\n> > +\n> > +\t\tnsize = buf->len;\n> > +\t\tnbuf = convert_to_git(path, buf->buf, &nsize);\n> > +\t\tif (nbuf)\n> > +\t\t\tstrbuf_embed(buf, nbuf, nsize, nsize);\n> > +\t\treturn 0;\n> \n> \n> I suspect that changing the convert_to_git() interface to work\n> on strbuf instead of (char*, size_t *) pair might make things\n> simpler and easier.\n\n  Well, yes, maybe it could use:\n  (const char *, char *, size_t, struct strbuf *out);\n\n  But this function is used elsewhere where there isn't strbuf's (yet ?)\nso I wasn't willing to do such a big change. But as you seem to think it\nwould help, I'll evaluate where it's going.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53203","messageId":"20070916172233.A60E42D43A4@madism.org","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"[PATCH] Remove preemptive allocations.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T08:19:01Z","receivedAt":"2007-09-16T08:19:01Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Careful profiling shows that we spend more time guessing what pattern\nallocation will have, whereas we can delay it only at the point where\nadd_rfc2047 will be used and don't allocate huge memory area for the many\ncases where it's not.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n commit.c |   35 +++++------------------------------\n 1 files changed, 5 insertions(+), 30 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 13af933..85889f9 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -501,6 +501,7 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,\n \treturn;\n \n needquote:\n+\tstrbuf_grow(sb, len * 3 + strlen(encoding) + 100);\n \tstrbuf_addf(sb, \"=?%s?q?\", encoding);\n \tfor (i = last = 0; i < len; i++) {\n \t\tunsigned ch = line[i] & 0xFF;\n@@ -520,14 +521,6 @@ needquote:\n \tstrbuf_addstr(sb, \"?=\");\n }\n \n-static unsigned long bound_rfc2047(unsigned long len, const char *encoding)\n-{\n-\t/* upper bound of q encoded string of length 'len' */\n-\tunsigned long elen = strlen(encoding);\n-\n-\treturn len * 3 + elen + 100;\n-}\n-\n static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb,\n \t\t\t const char *line, enum date_mode dmode,\n \t\t\t const char *encoding)\n@@ -560,8 +553,7 @@ static void add_user_info(const char *what, enum cmit_fmt fmt, struct strbuf *sb\n \t\tadd_rfc2047(sb, line, display_name_length, encoding);\n \t\tstrbuf_add(sb, name_tail, namelen - display_name_length);\n \t\tstrbuf_addch(sb, '\\n');\n-\t}\n-\telse {\n+\t} else {\n \t\tstrbuf_addf(sb, \"%s: %.*s%.*s\\n\", what,\n \t\t\t      (fmt == CMIT_FMT_FULLER) ? 4 : 0,\n \t\t\t      filler, namelen, line);\n@@ -955,19 +947,12 @@ static void pp_header(enum cmit_fmt fmt,\n \t\t * FULLER shows both authors and dates.\n \t\t */\n \t\tif (!memcmp(line, \"author \", 7)) {\n-\t\t\tunsigned long len = linelen;\n-\t\t\tif (fmt == CMIT_FMT_EMAIL)\n-\t\t\t\tlen = bound_rfc2047(linelen, encoding);\n-\t\t\tstrbuf_grow(sb, len + 80);\n+\t\t\tstrbuf_grow(sb, linelen + 80);\n \t\t\tadd_user_info(\"Author\", fmt, sb, line + 7, dmode, encoding);\n \t\t}\n-\n \t\tif (!memcmp(line, \"committer \", 10) &&\n \t\t    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {\n-\t\t\tunsigned long len = linelen;\n-\t\t\tif (fmt == CMIT_FMT_EMAIL)\n-\t\t\t\tlen = bound_rfc2047(linelen, encoding);\n-\t\t\tstrbuf_grow(sb, len + 80);\n+\t\t\tstrbuf_grow(sb, linelen + 80);\n \t\t\tadd_user_info(\"Commit\", fmt, sb, line + 10, dmode, encoding);\n \t\t}\n \t}\n@@ -982,7 +967,6 @@ static void pp_title_line(enum cmit_fmt fmt,\n \t\t\t  int plain_non_ascii)\n {\n \tstruct strbuf title;\n-\tunsigned long len;\n \n \tstrbuf_init(&title, 80);\n \n@@ -1004,16 +988,7 @@ static void pp_title_line(enum cmit_fmt fmt,\n \t\tstrbuf_add(&title, line, linelen);\n \t}\n \n-\t/* Enough slop for the MIME header and rfc2047 */\n-\tlen = bound_rfc2047(title.len, encoding) + 1000;\n-\tif (subject)\n-\t\tlen += strlen(subject);\n-\tif (after_subject)\n-\t\tlen += strlen(after_subject);\n-\tif (encoding)\n-\t\tlen += strlen(encoding);\n-\n-\tstrbuf_grow(sb, title.len + len);\n+\tstrbuf_grow(sb, title.len + 1024);\n \tif (subject) {\n \t\tstrbuf_addstr(sb, subject);\n \t\tadd_rfc2047(sb, title.buf, title.len, encoding);\n-- \n1.5.3.1\n"},{"id":"53201","messageId":"20070916172233.90C9E1835B@madism.org","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"[PATCH] Rewrite convert_to_{git,working_tree} to use strbuf's.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T13:51:04Z","receivedAt":"2007-09-16T13:51:04Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"* Now, those functions take an \"out\" strbuf argument, where they store their\n  result if any. In that case, it also returns 1, else it returns 0.\n* those functions support \"in place\" editing, in the sense that it's OK to\n  call them this way:\n    convert_to_git(path, sb->buf, sb->len, sb);\n  When doable, conversions are done in place for real, else the strbuf\n  content is just replaced with the new one, transparentely for the caller.\n\nIf you want to create a new filter working this way, being the accumulation\nof filter1, filter2, ... filtern, then your meta_filter would be:\n\n    int meta_filter(..., const char *src, size_t len, struct strbuf *sb)\n    {\n        int ret = 0;\n        ret |= filter1(...., src, len, sb);\n        if (ret) {\n            src = sb->buf;\n            len = sb->len;\n        }\n        ret |= filter2(...., src, len, sb);\n        if (ret) {\n            src = sb->buf;\n            len = sb->len;\n        }\n        ....\n        return ret | filtern(..., src, len, sb);\n    }\n\nThat's why subfilters the convert_to_* functions called were also rewritten\nto work this way.\n\nSigned-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n builtin-apply.c   |   27 ++--\n builtin-archive.c |   78 +++++------\n cache.h           |    6 +-\n convert.c         |  414 ++++++++++++++++++++++-------------------------------\n diff.c            |   12 +-\n entry.c           |   10 +-\n sha1_file.c       |   10 +-\n 7 files changed, 240 insertions(+), 317 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 7057d0d..9735b47 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1446,8 +1446,7 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign\n {\n \tint fd;\n \tunsigned long got;\n-\tunsigned long nsize;\n-\tchar *nbuf;\n+\tstruct strbuf nbuf;\n \tunsigned long size = *size_p;\n \tchar *buf = *buf_p;\n \n@@ -1466,13 +1465,12 @@ static int read_old_data(struct stat *st, const char *path, char **buf_p, unsign\n \t\t\tgot += ret;\n \t\t}\n \t\tclose(fd);\n-\t\tnsize = got;\n-\t\tnbuf = convert_to_git(path, buf, &nsize);\n-\t\tif (nbuf) {\n+\t\tstrbuf_init(&nbuf, 0);\n+\t\tif (convert_to_git(path, buf, size, &nbuf)) {\n \t\t\tfree(buf);\n-\t\t\t*buf_p = nbuf;\n-\t\t\t*alloc_p = nsize;\n-\t\t\t*size_p = nsize;\n+\t\t\t*buf_p = nbuf.buf;\n+\t\t\t*alloc_p = nbuf.alloc;\n+\t\t\t*size_p = nbuf.len;\n \t\t}\n \t\treturn got != size;\n \tdefault:\n@@ -2438,7 +2436,7 @@ static void add_index_file(const char *path, unsigned mode, void *buf, unsigned\n static int try_create_file(const char *path, unsigned int mode, const char *buf, unsigned long size)\n {\n \tint fd;\n-\tchar *nbuf;\n+\tstruct strbuf nbuf;\n \n \tif (S_ISGITLINK(mode)) {\n \t\tstruct stat st;\n@@ -2457,9 +2455,11 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \tif (fd < 0)\n \t\treturn -1;\n \n-\tnbuf = convert_to_working_tree(path, buf, &size);\n-\tif (nbuf)\n-\t\tbuf = nbuf;\n+\tstrbuf_init(&nbuf, 0);\n+\tif (convert_to_working_tree(path, buf, size, &nbuf)) {\n+\t\tsize = nbuf.len;\n+\t\tbuf  = nbuf.buf;\n+\t}\n \n \twhile (size) {\n \t\tint written = xwrite(fd, buf, size);\n@@ -2472,8 +2472,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \t}\n \tif (close(fd) < 0)\n \t\tdie(\"closing file %s: %s\", path, strerror(errno));\n-\tif (nbuf)\n-\t\tfree(nbuf);\n+\tstrbuf_release(&nbuf);\n \treturn 0;\n }\n \ndiff --git a/builtin-archive.c b/builtin-archive.c\nindex 6fa424d..843a9e3 100644\n--- a/builtin-archive.c\n+++ b/builtin-archive.c\n@@ -81,22 +81,21 @@ static int run_remote_archiver(const char *remote, int argc,\n \treturn !!rv;\n }\n \n-static void *format_subst(const struct commit *commit, const char *format,\n-                          unsigned long *sizep)\n+static void format_subst(const struct commit *commit,\n+                         const char *src, size_t len,\n+                         struct strbuf *buf)\n {\n-\tunsigned long len = *sizep;\n-\tconst char *a = format;\n-\tstruct strbuf result;\n+\tchar *to_free = NULL;\n \tstruct strbuf fmt;\n \n-\tstrbuf_init(&result, 0);\n+\tif (src == buf->buf)\n+\t\tto_free = strbuf_detach(buf);\n \tstrbuf_init(&fmt, 0);\n-\n \tfor (;;) {\n \t\tconst char *b, *c;\n \n-\t\tb = memmem(a, len, \"$Format:\", 8);\n-\t\tif (!b || a + len < b + 9)\n+\t\tb = memmem(src, len, \"$Format:\", 8);\n+\t\tif (!b || src + len < b + 9)\n \t\t\tbreak;\n \t\tc = memchr(b + 8, '$', len - 8);\n \t\tif (!c)\n@@ -105,62 +104,57 @@ static void *format_subst(const struct commit *commit, const char *format,\n \t\tstrbuf_reset(&fmt);\n \t\tstrbuf_add(&fmt, b + 8, c - b - 8);\n \n-\t\tstrbuf_add(&result, a, b - a);\n-\t\tformat_commit_message(commit, fmt.buf, &result);\n-\t\tlen -= c + 1 - a;\n-\t\ta = c + 1;\n+\t\tstrbuf_add(buf, src, b - src);\n+\t\tformat_commit_message(commit, fmt.buf, buf);\n+\t\tlen -= c + 1 - src;\n+\t\tsrc  = c + 1;\n \t}\n-\n-\tstrbuf_add(&result, a, len);\n-\n-\t*sizep = result.len;\n-\n+\tstrbuf_add(buf, src, len);\n \tstrbuf_release(&fmt);\n-\treturn strbuf_detach(&result);\n+\tfree(to_free);\n }\n \n-static void *convert_to_archive(const char *path,\n-                                const void *src, unsigned long *sizep,\n-                                const struct commit *commit)\n+static int convert_to_archive(const char *path,\n+                              const void *src, size_t len,\n+                              struct strbuf *buf,\n+                              const struct commit *commit)\n {\n \tstatic struct git_attr *attr_export_subst;\n \tstruct git_attr_check check[1];\n \n \tif (!commit)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-        if (!attr_export_subst)\n-                attr_export_subst = git_attr(\"export-subst\", 12);\n+\tif (!attr_export_subst)\n+\t\tattr_export_subst = git_attr(\"export-subst\", 12);\n \n \tcheck[0].attr = attr_export_subst;\n \tif (git_checkattr(path, ARRAY_SIZE(check), check))\n-\t\treturn NULL;\n+\t\treturn 0;\n \tif (!ATTR_TRUE(check[0].value))\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-\treturn format_subst(commit, src, sizep);\n+\tformat_subst(commit, src, len, buf);\n+\treturn 1;\n }\n \n void *sha1_file_to_archive(const char *path, const unsigned char *sha1,\n                            unsigned int mode, enum object_type *type,\n-                           unsigned long *size,\n+                           unsigned long *sizep,\n                            const struct commit *commit)\n {\n-\tvoid *buffer, *converted;\n+\tvoid *buffer;\n \n-\tbuffer = read_sha1_file(sha1, type, size);\n+\tbuffer = read_sha1_file(sha1, type, sizep);\n \tif (buffer && S_ISREG(mode)) {\n-\t\tconverted = convert_to_working_tree(path, buffer, size);\n-\t\tif (converted) {\n-\t\t\tfree(buffer);\n-\t\t\tbuffer = converted;\n-\t\t}\n-\n-\t\tconverted = convert_to_archive(path, buffer, size, commit);\n-\t\tif (converted) {\n-\t\t\tfree(buffer);\n-\t\t\tbuffer = converted;\n-\t\t}\n+\t\tstruct strbuf buf;\n+\n+\t\tstrbuf_init(&buf, 0);\n+\t\tstrbuf_attach(&buf, buffer, *sizep, *sizep + 1);\n+\t\tconvert_to_working_tree(path, buf.buf, buf.len, &buf);\n+\t\tconvert_to_archive(path, buf.buf, buf.len, &buf, commit);\n+\t\t*sizep = buf.len;\n+\t\tbuffer = buf.buf;\n \t}\n \n \treturn buffer;\ndiff --git a/cache.h b/cache.h\nindex b9a461a..8650d62 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -2,6 +2,7 @@\n #define CACHE_H\n \n #include \"git-compat-util.h\"\n+#include \"strbuf.h\"\n \n #include SHA1_HEADER\n #include <zlib.h>\n@@ -590,8 +591,9 @@ extern void trace_printf(const char *format, ...);\n extern void trace_argv_printf(const char **argv, int count, const char *format, ...);\n \n /* convert.c */\n-extern char *convert_to_git(const char *path, const char *src, unsigned long *sizep);\n-extern char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep);\n+/* returns 1 if *dst was used */\n+extern int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst);\n+extern int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst);\n \n /* diff.c */\n extern int diff_auto_refresh_index;\ndiff --git a/convert.c b/convert.c\nindex d77c8eb..00a341c 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -1,6 +1,7 @@\n #include \"cache.h\"\n #include \"attr.h\"\n #include \"run-command.h\"\n+#include \"strbuf.h\"\n \n /*\n  * convert.c - convert a file when checking it out and checking it in.\n@@ -80,24 +81,19 @@ static int is_binary(unsigned long size, struct text_stat *stats)\n \treturn 0;\n }\n \n-static char *crlf_to_git(const char *path, const char *src, unsigned long *sizep, int action)\n+static int crlf_to_git(const char *path, const char *src, size_t len,\n+                       struct strbuf *buf, int action)\n {\n-\tchar *buffer, *dst;\n-\tunsigned long size, nsize;\n \tstruct text_stat stats;\n+\tchar *dst;\n \n-\tif ((action == CRLF_BINARY) || !auto_crlf)\n-\t\treturn NULL;\n-\n-\tsize = *sizep;\n-\tif (!size)\n-\t\treturn NULL;\n-\n-\tgather_stats(src, size, &stats);\n+\tif ((action == CRLF_BINARY) || !auto_crlf || !len)\n+\t\treturn 0;\n \n+\tgather_stats(src, len, &stats);\n \t/* No CR? Nothing to convert, regardless. */\n \tif (!stats.cr)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tif (action == CRLF_GUESS) {\n \t\t/*\n@@ -106,24 +102,17 @@ static char *crlf_to_git(const char *path, const char *src, unsigned long *sizep\n \t\t * stuff?\n \t\t */\n \t\tif (stats.cr != stats.crlf)\n-\t\t\treturn NULL;\n+\t\t\treturn 0;\n \n \t\t/*\n \t\t * And add some heuristics for binary vs text, of course...\n \t\t */\n-\t\tif (is_binary(size, &stats))\n-\t\t\treturn NULL;\n+\t\tif (is_binary(len, &stats))\n+\t\t\treturn 0;\n \t}\n \n-\t/*\n-\t * Ok, allocate a new buffer, fill it in, and return it\n-\t * to let the caller know that we switched buffers.\n-\t */\n-\tnsize = size - stats.crlf;\n-\tbuffer = xmalloc(nsize);\n-\t*sizep = nsize;\n-\n-\tdst = buffer;\n+\tstrbuf_grow(buf, len);\n+\tdst = buf->buf;\n \tif (action == CRLF_GUESS) {\n \t\t/*\n \t\t * If we guessed, we already know we rejected a file with\n@@ -134,71 +123,72 @@ static char *crlf_to_git(const char *path, const char *src, unsigned long *sizep\n \t\t\tunsigned char c = *src++;\n \t\t\tif (c != '\\r')\n \t\t\t\t*dst++ = c;\n-\t\t} while (--size);\n+\t\t} while (--len);\n \t} else {\n \t\tdo {\n \t\t\tunsigned char c = *src++;\n-\t\t\tif (! (c == '\\r' && (1 < size && *src == '\\n')))\n+\t\t\tif (! (c == '\\r' && (1 < len && *src == '\\n')))\n \t\t\t\t*dst++ = c;\n-\t\t} while (--size);\n+\t\t} while (--len);\n \t}\n-\n-\treturn buffer;\n+\tstrbuf_setlen(buf, dst - buf->buf);\n+\treturn 1;\n }\n \n-static char *crlf_to_worktree(const char *path, const char *src, unsigned long *sizep, int action)\n+static int crlf_to_worktree(const char *path, const char *src, size_t len,\n+                            struct strbuf *buf, int action)\n {\n-\tchar *buffer, *dst;\n-\tunsigned long size, nsize;\n+\tchar *to_free = NULL;\n \tstruct text_stat stats;\n-\tunsigned char last;\n \n \tif ((action == CRLF_BINARY) || (action == CRLF_INPUT) ||\n \t    auto_crlf <= 0)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-\tsize = *sizep;\n-\tif (!size)\n-\t\treturn NULL;\n+\tif (!len)\n+\t\treturn 0;\n \n-\tgather_stats(src, size, &stats);\n+\tgather_stats(src, len, &stats);\n \n \t/* No LF? Nothing to convert, regardless. */\n \tif (!stats.lf)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \t/* Was it already in CRLF format? */\n \tif (stats.lf == stats.crlf)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tif (action == CRLF_GUESS) {\n \t\t/* If we have any bare CR characters, we're not going to touch it */\n \t\tif (stats.cr != stats.crlf)\n-\t\t\treturn NULL;\n+\t\t\treturn 0;\n \n-\t\tif (is_binary(size, &stats))\n-\t\t\treturn NULL;\n+\t\tif (is_binary(len, &stats))\n+\t\t\treturn 0;\n \t}\n \n-\t/*\n-\t * Ok, allocate a new buffer, fill it in, and return it\n-\t * to let the caller know that we switched buffers.\n-\t */\n-\tnsize = size + stats.lf - stats.crlf;\n-\tbuffer = xmalloc(nsize);\n-\t*sizep = nsize;\n-\tlast = 0;\n-\n-\tdst = buffer;\n-\tdo {\n-\t\tunsigned char c = *src++;\n-\t\tif (c == '\\n' && last != '\\r')\n-\t\t\t*dst++ = '\\r';\n-\t\t*dst++ = c;\n-\t\tlast = c;\n-\t} while (--size);\n-\n-\treturn buffer;\n+\t/* are we \"faking\" in place editing ? */\n+\tif (src == buf->buf)\n+\t\tto_free = strbuf_detach(buf);\n+\n+\tstrbuf_grow(buf, len + stats.lf - stats.crlf);\n+\tfor (;;) {\n+\t\tconst char *nl = memchr(src, '\\n', len);\n+\t\tif (!nl)\n+\t\t\tbreak;\n+\t\tif (nl > src && nl[-1] == '\\r') {\n+\t\t\tstrbuf_add(buf, src, nl + 1 - src);\n+\t\t} else {\n+\t\t\tstrbuf_add(buf, src, nl - src);\n+\t\t\tstrbuf_addstr(buf, \"\\r\\n\");\n+\t\t}\n+\t\tlen -= nl + 1 - src;\n+\t\tsrc  = nl + 1;\n+\t}\n+\tstrbuf_add(buf, src, len);\n+\n+\tfree(to_free);\n+\treturn 1;\n }\n \n static int filter_buffer(const char *path, const char *src,\n@@ -246,8 +236,8 @@ static int filter_buffer(const char *path, const char *src,\n \treturn (write_err || status);\n }\n \n-static char *apply_filter(const char *path, const char *src,\n-\t\t\t  unsigned long *sizep, const char *cmd)\n+static int apply_filter(const char *path, const char *src, size_t len,\n+                        struct strbuf *dst, const char *cmd)\n {\n \t/*\n \t * Create a pipeline to have the command filter the buffer's\n@@ -255,21 +245,19 @@ static char *apply_filter(const char *path, const char *src,\n \t *\n \t * (child --> cmd) --> us\n \t */\n-\tconst int SLOP = 4096;\n \tint pipe_feed[2];\n-\tint status;\n-\tchar *dst;\n-\tunsigned long dstsize, dstalloc;\n+\tint status, ret = 1;\n \tstruct child_process child_process;\n+\tstruct strbuf nbuf;\n \n \tif (!cmd)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tmemset(&child_process, 0, sizeof(child_process));\n \n \tif (pipe(pipe_feed) < 0) {\n \t\terror(\"cannot create pipe to run external filter %s\", cmd);\n-\t\treturn NULL;\n+\t\treturn 0;\n \t}\n \n \tfflush(NULL);\n@@ -278,54 +266,37 @@ static char *apply_filter(const char *path, const char *src,\n \t\terror(\"cannot fork to run external filter %s\", cmd);\n \t\tclose(pipe_feed[0]);\n \t\tclose(pipe_feed[1]);\n-\t\treturn NULL;\n+\t\treturn 0;\n \t}\n \tif (!child_process.pid) {\n \t\tdup2(pipe_feed[1], 1);\n \t\tclose(pipe_feed[0]);\n \t\tclose(pipe_feed[1]);\n-\t\texit(filter_buffer(path, src, *sizep, cmd));\n+\t\texit(filter_buffer(path, src, len, cmd));\n \t}\n \tclose(pipe_feed[1]);\n \n-\tdstalloc = *sizep;\n-\tdst = xmalloc(dstalloc);\n-\tdstsize = 0;\n-\n-\twhile (1) {\n-\t\tssize_t numread = xread(pipe_feed[0], dst + dstsize,\n-\t\t\t\t\tdstalloc - dstsize);\n-\n-\t\tif (numread <= 0) {\n-\t\t\tif (!numread)\n-\t\t\t\tbreak;\n-\t\t\terror(\"read from external filter %s failed\", cmd);\n-\t\t\tfree(dst);\n-\t\t\tdst = NULL;\n-\t\t\tbreak;\n-\t\t}\n-\t\tdstsize += numread;\n-\t\tif (dstalloc <= dstsize + SLOP) {\n-\t\t\tdstalloc = dstsize + SLOP;\n-\t\t\tdst = xrealloc(dst, dstalloc);\n-\t\t}\n+\tstrbuf_init(&nbuf, 0);\n+\tif (strbuf_read(&nbuf, pipe_feed[0], len) < 0) {\n+\t\terror(\"read from external filter %s failed\", cmd);\n+\t\tret = 0;\n \t}\n \tif (close(pipe_feed[0])) {\n-\t\terror(\"read from external filter %s failed\", cmd);\n-\t\tfree(dst);\n-\t\tdst = NULL;\n+\t\tret = error(\"read from external filter %s failed\", cmd);\n+\t\tret = 0;\n \t}\n-\n \tstatus = finish_command(&child_process);\n \tif (status) {\n-\t\terror(\"external filter %s failed %d\", cmd, -status);\n-\t\tfree(dst);\n-\t\tdst = NULL;\n+\t\tret = error(\"external filter %s failed %d\", cmd, -status);\n+\t\tret = 0;\n \t}\n \n-\tif (dst)\n-\t\t*sizep = dstsize;\n-\treturn dst;\n+\tif (ret) {\n+\t\t*dst = nbuf;\n+\t} else {\n+\t\tstrbuf_release(&nbuf);\n+\t}\n+\treturn ret;\n }\n \n static struct convert_driver {\n@@ -449,137 +420,104 @@ static int count_ident(const char *cp, unsigned long size)\n \treturn cnt;\n }\n \n-static char *ident_to_git(const char *path, const char *src, unsigned long *sizep, int ident)\n+static int ident_to_git(const char *path, const char *src, size_t len,\n+                        struct strbuf *buf, int ident)\n {\n-\tint cnt;\n-\tunsigned long size;\n-\tchar *dst, *buf;\n+\tchar *dst, *dollar;\n \n-\tif (!ident)\n-\t\treturn NULL;\n-\tsize = *sizep;\n-\tcnt = count_ident(src, size);\n-\tif (!cnt)\n-\t\treturn NULL;\n-\tbuf = xmalloc(size);\n-\n-\tfor (dst = buf; size; size--) {\n-\t\tchar ch = *src++;\n-\t\t*dst++ = ch;\n-\t\tif ((ch == '$') && (3 <= size) &&\n-\t\t    !memcmp(\"Id:\", src, 3)) {\n-\t\t\tunsigned long rem = size - 3;\n-\t\t\tconst char *cp = src + 3;\n-\t\t\tdo {\n-\t\t\t\tch = *cp++;\n-\t\t\t\tif (ch == '$')\n-\t\t\t\t\tbreak;\n-\t\t\t\trem--;\n-\t\t\t} while (rem);\n-\t\t\tif (!rem)\n-\t\t\t\tcontinue;\n+\tif (!ident || !count_ident(src, len))\n+\t\treturn 0;\n+\n+\tstrbuf_grow(buf, len);\n+\tdst = buf->buf;\n+\tfor (;;) {\n+\t\tdollar = memchr(src, '$', len);\n+\t\tif (!dollar)\n+\t\t\tbreak;\n+\t\tmemcpy(dst, src, dollar + 1 - src);\n+\t\tdst += dollar + 1 - src;\n+\t\tlen -= dollar + 1 - src;\n+\t\tsrc  = dollar + 1;\n+\n+\t\tif (len > 3 && !memcmp(src, \"Id:\", 3)) {\n+\t\t\tdollar = memchr(src + 3, '$', len - 3);\n+\t\t\tif (!dollar)\n+\t\t\t\tbreak;\n \t\t\tmemcpy(dst, \"Id$\", 3);\n \t\t\tdst += 3;\n-\t\t\tsize -= (cp - src);\n-\t\t\tsrc = cp;\n+\t\t\tlen -= dollar + 1 - src;\n+\t\t\tsrc  = dollar + 1;\n \t\t}\n \t}\n-\n-\t*sizep = dst - buf;\n-\treturn buf;\n+\tmemcpy(dst, src, len);\n+\tstrbuf_setlen(buf, dst + len - buf->buf);\n+\treturn 1;\n }\n \n-static char *ident_to_worktree(const char *path, const char *src, unsigned long *sizep, int ident)\n+static int ident_to_worktree(const char *path, const char *src, size_t len,\n+                             struct strbuf *buf, int ident)\n {\n-\tint cnt;\n-\tunsigned long size;\n-\tchar *dst, *buf;\n \tunsigned char sha1[20];\n+\tchar *to_free = NULL, *dollar;\n+\tint cnt;\n \n \tif (!ident)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-\tsize = *sizep;\n-\tcnt = count_ident(src, size);\n+\tcnt = count_ident(src, len);\n \tif (!cnt)\n-\t\treturn NULL;\n+\t\treturn 0;\n \n-\thash_sha1_file(src, size, \"blob\", sha1);\n-\tbuf = xmalloc(size + cnt * 43);\n-\n-\tfor (dst = buf; size; size--) {\n-\t\tconst char *cp;\n-\t\t/* Fetch next source character, move the pointer on */\n-\t\tchar ch = *src++;\n-\t\t/* Copy the current character to the destination */\n-\t\t*dst++ = ch;\n-\t\t/* If the current character is \"$\" or there are less than three\n-\t\t * remaining bytes or the two bytes following this one are not\n-\t\t * \"Id\", then simply read the next character */\n-\t\tif ((ch != '$') || (size < 3) || memcmp(\"Id\", src, 2))\n-\t\t\tcontinue;\n-\t\t/*\n-\t\t * Here when\n-\t\t *  - There are more than 2 bytes remaining\n-\t\t *  - The current three bytes are \"$Id\"\n-\t\t * with\n-\t\t *  - ch == \"$\"\n-\t\t *  - src[0] == \"I\"\n-\t\t */\n+\t/* are we \"faking\" in place editing ? */\n+\tif (src == buf->buf)\n+\t\tto_free = strbuf_detach(buf);\n+\thash_sha1_file(src, len, \"blob\", sha1);\n \n-\t\t/*\n-\t\t * It's possible that an expanded Id has crept its way into the\n-\t\t * repository, we cope with that by stripping the expansion out\n-\t\t */\n-\t\tif (src[2] == ':') {\n-\t\t\t/* Expanded keywords have \"$Id:\" at the front */\n+\tstrbuf_grow(buf, len + cnt * 43);\n+\tfor (;;) {\n+\t\t/* step 1: run to the next '$' */\n+\t\tdollar = memchr(src, '$', len);\n+\t\tif (!dollar)\n+\t\t\tbreak;\n+\t\tstrbuf_add(buf, src, dollar + 1 - src);\n+\t\tlen -= dollar + 1 - src;\n+\t\tsrc  = dollar + 1;\n \n-\t\t\t/* discard up to but not including the closing $ */\n-\t\t\tunsigned long rem = size - 3;\n-\t\t\t/* Point at first byte after the \":\" */\n-\t\t\tcp = src + 3;\n-\t\t\t/*\n-\t\t\t * Throw away characters until either\n-\t\t\t *  - we reach a \"$\"\n-\t\t\t *  - we run out of bytes (rem == 0)\n-\t\t\t */\n-\t\t\tdo {\n-\t\t\t\tch = *cp;\n-\t\t\t\tif (ch == '$')\n-\t\t\t\t\tbreak;\n-\t\t\t\tcp++;\n-\t\t\t\trem--;\n-\t\t\t} while (rem);\n-\t\t\t/* If the above finished because it ran out of characters, then\n-\t\t\t * this is an incomplete keyword, so don't run the expansion */\n-\t\t\tif (!rem)\n-\t\t\t\tcontinue;\n-\t\t} else if (src[2] == '$')\n-\t\t\tcp = src + 2;\n-\t\telse\n-\t\t\t/* Anything other than \"$Id:XXX$\" or $Id$ and we skip the\n-\t\t\t * expansion */\n+\t\t/* step 2: does it looks like a bit like Id:xxx$ or Id$ ? */\n+\t\tif (len < 3 || memcmp(\"Id\", src, 2))\n \t\t\tcontinue;\n \n-\t\t/* cp is now pointing at the last $ of the keyword */\n-\n-\t\tmemcpy(dst, \"Id: \", 4);\n-\t\tdst += 4;\n-\t\tmemcpy(dst, sha1_to_hex(sha1), 40);\n-\t\tdst += 40;\n-\t\t*dst++ = ' ';\n+\t\t/* step 3: skip over Id$ or Id:xxxxx$ */\n+\t\tif (src[2] == '$') {\n+\t\t\tsrc += 3;\n+\t\t\tlen -= 3;\n+\t\t} else if (src[2] == ':') {\n+\t\t\t/*\n+\t\t\t * It's possible that an expanded Id has crept its way into the\n+\t\t\t * repository, we cope with that by stripping the expansion out\n+\t\t\t */\n+\t\t\tdollar = memchr(src + 3, '$', len - 3);\n+\t\t\tif (!dollar) {\n+\t\t\t\t/* incomplete keyword, no more '$', so just quit the loop */\n+\t\t\t\tbreak;\n+\t\t\t}\n \n-\t\t/* Adjust for the characters we've discarded */\n-\t\tsize -= (cp - src);\n-\t\tsrc = cp;\n+\t\t\tlen -= dollar + 1 - src;\n+\t\t\tsrc  = dollar + 1;\n+\t\t} else {\n+\t\t\t/* it wasn't a \"Id$\" or \"Id:xxxx$\" */\n+\t\t\tcontinue;\n+\t\t}\n \n-\t\t/* Copy the final \"$\" */\n-\t\t*dst++ = *src++;\n-\t\tsize--;\n+\t\t/* step 4: substitute */\n+\t\tstrbuf_addstr(buf, \"Id: \");\n+\t\tstrbuf_add(buf, sha1_to_hex(sha1), 40);\n+\t\tstrbuf_addstr(buf, \" $\");\n \t}\n+\tstrbuf_add(buf, src, len);\n \n-\t*sizep = dst - buf;\n-\treturn buf;\n+\tfree(to_free);\n+\treturn 1;\n }\n \n static int git_path_check_crlf(const char *path, struct git_attr_check *check)\n@@ -618,13 +556,12 @@ static int git_path_check_ident(const char *path, struct git_attr_check *check)\n \treturn !!ATTR_TRUE(value);\n }\n \n-char *convert_to_git(const char *path, const char *src, unsigned long *sizep)\n+int convert_to_git(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n \tstruct git_attr_check check[3];\n \tint crlf = CRLF_GUESS;\n-\tint ident = 0;\n+\tint ident = 0, ret = 0;\n \tchar *filter = NULL;\n-\tchar *buf, *buf2;\n \n \tsetup_convert_check(check);\n \tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n@@ -636,30 +573,25 @@ char *convert_to_git(const char *path, const char *src, unsigned long *sizep)\n \t\t\tfilter = drv->clean;\n \t}\n \n-\tbuf = apply_filter(path, src, sizep, filter);\n-\n-\tbuf2 = crlf_to_git(path, buf ? buf : src, sizep, crlf);\n-\tif (buf2) {\n-\t\tfree(buf);\n-\t\tbuf = buf2;\n+\tret |= apply_filter(path, src, len, dst, filter);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n \t}\n-\n-\tbuf2 = ident_to_git(path, buf ? buf : src, sizep, ident);\n-\tif (buf2) {\n-\t\tfree(buf);\n-\t\tbuf = buf2;\n+\tret |= crlf_to_git(path, src, len, dst, crlf);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n \t}\n-\n-\treturn buf;\n+\treturn ret | ident_to_git(path, src, len, dst, ident);\n }\n \n-char *convert_to_working_tree(const char *path, const char *src, unsigned long *sizep)\n+int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)\n {\n \tstruct git_attr_check check[3];\n \tint crlf = CRLF_GUESS;\n-\tint ident = 0;\n+\tint ident = 0, ret = 0;\n \tchar *filter = NULL;\n-\tchar *buf, *buf2;\n \n \tsetup_convert_check(check);\n \tif (!git_checkattr(path, ARRAY_SIZE(check), check)) {\n@@ -671,19 +603,15 @@ char *convert_to_working_tree(const char *path, const char *src, unsigned long *\n \t\t\tfilter = drv->smudge;\n \t}\n \n-\tbuf = ident_to_worktree(path, src, sizep, ident);\n-\n-\tbuf2 = crlf_to_worktree(path, buf ? buf : src, sizep, crlf);\n-\tif (buf2) {\n-\t\tfree(buf);\n-\t\tbuf = buf2;\n+\tret |= ident_to_worktree(path, src, len, dst, ident);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n \t}\n-\n-\tbuf2 = apply_filter(path, buf ? buf : src, sizep, filter);\n-\tif (buf2) {\n-\t\tfree(buf);\n-\t\tbuf = buf2;\n+\tret |= crlf_to_worktree(path, src, len, dst, crlf);\n+\tif (ret) {\n+\t\tsrc = dst->buf;\n+\t\tlen = dst->len;\n \t}\n-\n-\treturn buf;\n+\treturn ret | apply_filter(path, src, len, dst, filter);\n }\ndiff --git a/diff.c b/diff.c\nindex 693140b..d46dd11 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1600,10 +1600,9 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \n \tif (!s->sha1_valid ||\n \t    reuse_worktree_file(s->path, s->sha1, 0)) {\n+\t\tstruct strbuf buf;\n \t\tstruct stat st;\n \t\tint fd;\n-\t\tchar *buf;\n-\t\tunsigned long size;\n \n \t\tif (!strcmp(s->path, \"-\"))\n \t\t\treturn populate_from_stdin(s);\n@@ -1644,13 +1643,12 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\t/*\n \t\t * Convert from working tree format to canonical git format\n \t\t */\n-\t\tsize = s->size;\n-\t\tbuf = convert_to_git(s->path, s->data, &size);\n-\t\tif (buf) {\n+\t\tstrbuf_init(&buf, 0);\n+\t\tif (convert_to_git(s->path, s->data, s->size, &buf)) {\n \t\t\tmunmap(s->data, s->size);\n \t\t\ts->should_munmap = 0;\n-\t\t\ts->data = buf;\n-\t\t\ts->size = size;\n+\t\t\ts->data = buf.buf;\n+\t\t\ts->size = buf.len;\n \t\t\ts->should_free = 1;\n \t\t}\n \t}\ndiff --git a/entry.c b/entry.c\nindex fc3a506..4a8c73b 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -104,7 +104,8 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout\n \tlong wrote;\n \n \tswitch (ntohl(ce->ce_mode) & S_IFMT) {\n-\t\tchar *buf, *new;\n+\t\tchar *new;\n+\t\tstruct strbuf buf;\n \t\tunsigned long size;\n \n \tcase S_IFREG:\n@@ -116,10 +117,11 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout\n \t\t/*\n \t\t * Convert from git internal format to working tree format\n \t\t */\n-\t\tbuf = convert_to_working_tree(ce->name, new, &size);\n-\t\tif (buf) {\n+\t\tstrbuf_init(&buf, 0);\n+\t\tif (convert_to_working_tree(ce->name, new, size, &buf)) {\n \t\t\tfree(new);\n-\t\t\tnew = buf;\n+\t\t\tnew = buf.buf;\n+\t\t\tsize = buf.len;\n \t\t}\n \n \t\tif (to_tempfile) {\ndiff --git a/sha1_file.c b/sha1_file.c\nindex aea1096..64b5b46 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2343,12 +2343,12 @@ int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object,\n \t * Convert blobs to git internal format\n \t */\n \tif ((type == OBJ_BLOB) && S_ISREG(st->st_mode)) {\n-\t\tunsigned long nsize = size;\n-\t\tchar *nbuf = convert_to_git(path, buf, &nsize);\n-\t\tif (nbuf) {\n+\t\tstruct strbuf nbuf;\n+\t\tstrbuf_init(&nbuf, 0);\n+\t\tif (convert_to_git(path, buf, size, &nbuf)) {\n \t\t\tmunmap(buf, size);\n-\t\t\tsize = nsize;\n-\t\t\tbuf = nbuf;\n+\t\t\tsize = nbuf.len;\n+\t\t\tbuf = nbuf.buf;\n \t\t\tre_allocated = 1;\n \t\t}\n \t}\n-- \n1.5.3.1\n"},{"id":"53202","messageId":"20070916172233.9C1F818364@madism.org","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"[PATCH] builtin-apply: use strbuf's instead of buffer_desc's.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T16:54:42Z","receivedAt":"2007-09-16T16:54:42Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit <madcoder@debian.org>\n---\n builtin-apply.c |  212 +++++++++++++++++++------------------------------------\n 1 files changed, 72 insertions(+), 140 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex ae708d7..05011bb 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -1441,37 +1441,28 @@ static void show_stats(struct patch *patch)\n \tfree(qname);\n }\n \n-static int read_old_data(struct stat *st, const char *path, char **buf_p, unsigned long *alloc_p, unsigned long *size_p)\n+static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n {\n \tint fd;\n-\tunsigned long got;\n-\tstruct strbuf nbuf;\n-\tunsigned long size = *size_p;\n-\tchar *buf = *buf_p;\n \n \tswitch (st->st_mode & S_IFMT) {\n \tcase S_IFLNK:\n-\t\treturn readlink(path, buf, size) != size;\n+\t\tstrbuf_grow(buf, st->st_size);\n+\t\tif (readlink(path, buf->buf, st->st_size) != st->st_size)\n+\t\t\treturn -1;\n+\t\tstrbuf_setlen(buf, st->st_size);\n+\t\treturn 0;\n \tcase S_IFREG:\n \t\tfd = open(path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\treturn error(\"unable to open %s\", path);\n-\t\tgot = 0;\n-\t\tfor (;;) {\n-\t\t\tssize_t ret = xread(fd, buf + got, size - got);\n-\t\t\tif (ret <= 0)\n-\t\t\t\tbreak;\n-\t\t\tgot += ret;\n+\t\tif (strbuf_read(buf, fd, st->st_size) < 0) {\n+\t\t\tclose(fd);\n+\t\t\treturn -1;\n \t\t}\n \t\tclose(fd);\n-\t\tstrbuf_init(&nbuf, 0);\n-\t\tif (convert_to_git(path, buf, size, &nbuf)) {\n-\t\t\tfree(buf);\n-\t\t\t*buf_p = nbuf.buf;\n-\t\t\t*alloc_p = nbuf.alloc;\n-\t\t\t*size_p = nbuf.len;\n-\t\t}\n-\t\treturn got != size;\n+\t\tconvert_to_git(path, buf->buf, buf->len, buf);\n+\t\treturn 0;\n \tdefault:\n \t\treturn -1;\n \t}\n@@ -1576,12 +1567,6 @@ static void remove_last_line(const char **rbuf, int *rsize)\n \t*rsize = offset + 1;\n }\n \n-struct buffer_desc {\n-\tchar *buffer;\n-\tunsigned long size;\n-\tunsigned long alloc;\n-};\n-\n static int apply_line(char *output, const char *patch, int plen)\n {\n \t/* plen is number of bytes to be copied from patch,\n@@ -1651,10 +1636,9 @@ static int apply_line(char *output, const char *patch, int plen)\n \treturn output + plen - buf;\n }\n \n-static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, int inaccurate_eof)\n+static int apply_one_fragment(struct strbuf *buf, struct fragment *frag, int inaccurate_eof)\n {\n \tint match_beginning, match_end;\n-\tchar *buf = desc->buffer;\n \tconst char *patch = frag->patch;\n \tint offset, size = frag->size;\n \tchar *old = xmalloc(size);\n@@ -1765,24 +1749,17 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \tlines = 0;\n \tpos = frag->newpos;\n \tfor (;;) {\n-\t\toffset = find_offset(buf, desc->size,\n+\t\toffset = find_offset(buf->buf, buf->len,\n \t\t\t\t     oldlines, oldsize, pos, &lines);\n-\t\tif (match_end && offset + oldsize != desc->size)\n+\t\tif (match_end && offset + oldsize != buf->len)\n \t\t\toffset = -1;\n \t\tif (match_beginning && offset)\n \t\t\toffset = -1;\n \t\tif (offset >= 0) {\n-\t\t\tint diff;\n-\t\t\tunsigned long size, alloc;\n-\n \t\t\tif (new_whitespace == strip_whitespace &&\n-\t\t\t    (desc->size - oldsize - offset == 0)) /* end of file? */\n+\t\t\t    (buf->len - oldsize - offset == 0)) /* end of file? */\n \t\t\t\tnewsize -= new_blank_lines_at_end;\n \n-\t\t\tdiff = newsize - oldsize;\n-\t\t\tsize = desc->size + diff;\n-\t\t\talloc = desc->alloc;\n-\n \t\t\t/* Warn if it was necessary to reduce the number\n \t\t\t * of context lines.\n \t\t\t */\n@@ -1792,19 +1769,8 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \t\t\t\t\t\" to apply fragment at %d\\n\",\n \t\t\t\t\tleading, trailing, pos + lines);\n \n-\t\t\tif (size > alloc) {\n-\t\t\t\talloc = size + 8192;\n-\t\t\t\tdesc->alloc = alloc;\n-\t\t\t\tbuf = xrealloc(buf, alloc);\n-\t\t\t\tdesc->buffer = buf;\n-\t\t\t}\n-\t\t\tdesc->size = size;\n-\t\t\tmemmove(buf + offset + newsize,\n-\t\t\t\tbuf + offset + oldsize,\n-\t\t\t\tsize - offset - newsize);\n-\t\t\tmemcpy(buf + offset, newlines, newsize);\n+\t\t\tstrbuf_splice(buf, offset, oldsize, newlines, newsize);\n \t\t\toffset = 0;\n-\n \t\t\tbreak;\n \t\t}\n \n@@ -1840,12 +1806,11 @@ static int apply_one_fragment(struct buffer_desc *desc, struct fragment *frag, i\n \treturn offset;\n }\n \n-static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)\n+static int apply_binary_fragment(struct strbuf *buf, struct patch *patch)\n {\n-\tunsigned long dst_size;\n \tstruct fragment *fragment = patch->fragments;\n-\tvoid *data;\n-\tvoid *result;\n+\tunsigned long len;\n+\tvoid *dst;\n \n \t/* Binary patch is irreversible without the optional second hunk */\n \tif (apply_in_reverse) {\n@@ -1856,29 +1821,24 @@ static int apply_binary_fragment(struct buffer_desc *desc, struct patch *patch)\n \t\t\t\t     ? patch->new_name : patch->old_name);\n \t\tfragment = fragment->next;\n \t}\n-\tdata = (void*) fragment->patch;\n \tswitch (fragment->binary_patch_method) {\n \tcase BINARY_DELTA_DEFLATED:\n-\t\tresult = patch_delta(desc->buffer, desc->size,\n-\t\t\t\t     data,\n-\t\t\t\t     fragment->size,\n-\t\t\t\t     &dst_size);\n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = result;\n-\t\tbreak;\n+\t\tdst = patch_delta(buf->buf, buf->len, fragment->patch,\n+\t\t\t\t  fragment->size, &len);\n+\t\tif (!dst)\n+\t\t\treturn -1;\n+\t\t/* XXX patch_delta NUL-terminates */\n+\t\tstrbuf_attach(buf, dst, len, len + 1);\n+\t\treturn 0;\n \tcase BINARY_LITERAL_DEFLATED:\n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = data;\n-\t\tdst_size = fragment->size;\n-\t\tbreak;\n+\t\tstrbuf_reset(buf);\n+\t\tstrbuf_add(buf, fragment->patch, fragment->size);\n+\t\treturn 0;\n \t}\n-\tif (!desc->buffer)\n-\t\treturn -1;\n-\tdesc->size = desc->alloc = dst_size;\n-\treturn 0;\n+\treturn -1;\n }\n \n-static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n+static int apply_binary(struct strbuf *buf, struct patch *patch)\n {\n \tconst char *name = patch->old_name ? patch->old_name : patch->new_name;\n \tunsigned char sha1[20];\n@@ -1897,7 +1857,7 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n \t\t/* See if the old one matches what the patch\n \t\t * applies to.\n \t\t */\n-\t\thash_sha1_file(desc->buffer, desc->size, blob_type, sha1);\n+\t\thash_sha1_file(buf->buf, buf->len, blob_type, sha1);\n \t\tif (strcmp(sha1_to_hex(sha1), patch->old_sha1_prefix))\n \t\t\treturn error(\"the patch applies to '%s' (%s), \"\n \t\t\t\t     \"which does not match the \"\n@@ -1906,16 +1866,14 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n \t}\n \telse {\n \t\t/* Otherwise, the old one must be empty. */\n-\t\tif (desc->size)\n+\t\tif (buf->len)\n \t\t\treturn error(\"the patch applies to an empty \"\n \t\t\t\t     \"'%s' but it is not empty\", name);\n \t}\n \n \tget_sha1_hex(patch->new_sha1_prefix, sha1);\n \tif (is_null_sha1(sha1)) {\n-\t\tfree(desc->buffer);\n-\t\tdesc->alloc = desc->size = 0;\n-\t\tdesc->buffer = NULL;\n+\t\tstrbuf_release(buf);\n \t\treturn 0; /* deletion patch */\n \t}\n \n@@ -1923,43 +1881,44 @@ static int apply_binary(struct buffer_desc *desc, struct patch *patch)\n \t\t/* We already have the postimage */\n \t\tenum object_type type;\n \t\tunsigned long size;\n+\t\tchar *result;\n \n-\t\tfree(desc->buffer);\n-\t\tdesc->buffer = read_sha1_file(sha1, &type, &size);\n-\t\tif (!desc->buffer)\n+\t\tresult = read_sha1_file(sha1, &type, &size);\n+\t\tif (!result)\n \t\t\treturn error(\"the necessary postimage %s for \"\n \t\t\t\t     \"'%s' cannot be read\",\n \t\t\t\t     patch->new_sha1_prefix, name);\n-\t\tdesc->alloc = desc->size = size;\n-\t}\n-\telse {\n-\t\t/* We have verified desc matches the preimage;\n+\t\t/* XXX read_sha1_file NUL-terminates */\n+\t\tstrbuf_attach(buf, result, size, size + 1);\n+\t} else {\n+\t\t/* We have verified buf matches the preimage;\n \t\t * apply the patch data to it, which is stored\n \t\t * in the patch->fragments->{patch,size}.\n \t\t */\n-\t\tif (apply_binary_fragment(desc, patch))\n+\t\tif (apply_binary_fragment(buf, patch))\n \t\t\treturn error(\"binary patch does not apply to '%s'\",\n \t\t\t\t     name);\n \n \t\t/* verify that the result matches */\n-\t\thash_sha1_file(desc->buffer, desc->size, blob_type, sha1);\n+\t\thash_sha1_file(buf->buf, buf->len, blob_type, sha1);\n \t\tif (strcmp(sha1_to_hex(sha1), patch->new_sha1_prefix))\n-\t\t\treturn error(\"binary patch to '%s' creates incorrect result (expecting %s, got %s)\", name, patch->new_sha1_prefix, sha1_to_hex(sha1));\n+\t\t\treturn error(\"binary patch to '%s' creates incorrect result (expecting %s, got %s)\",\n+\t\t\t\tname, patch->new_sha1_prefix, sha1_to_hex(sha1));\n \t}\n \n \treturn 0;\n }\n \n-static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n+static int apply_fragments(struct strbuf *buf, struct patch *patch)\n {\n \tstruct fragment *frag = patch->fragments;\n \tconst char *name = patch->old_name ? patch->old_name : patch->new_name;\n \n \tif (patch->is_binary)\n-\t\treturn apply_binary(desc, patch);\n+\t\treturn apply_binary(buf, patch);\n \n \twhile (frag) {\n-\t\tif (apply_one_fragment(desc, frag, patch->inaccurate_eof)) {\n+\t\tif (apply_one_fragment(buf, frag, patch->inaccurate_eof)) {\n \t\t\terror(\"patch failed: %s:%ld\", name, frag->oldpos);\n \t\t\tif (!apply_with_reject)\n \t\t\t\treturn -1;\n@@ -1970,76 +1929,57 @@ static int apply_fragments(struct buffer_desc *desc, struct patch *patch)\n \treturn 0;\n }\n \n-static int read_file_or_gitlink(struct cache_entry *ce, char **buf_p,\n-\t\t\t\tunsigned long *size_p)\n+static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n {\n \tif (!ce)\n \t\treturn 0;\n \n \tif (S_ISGITLINK(ntohl(ce->ce_mode))) {\n-\t\t*buf_p = xmalloc(100);\n-\t\t*size_p = snprintf(*buf_p, 100,\n-\t\t\t\"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t\tstrbuf_grow(buf, 100);\n+\t\tstrbuf_addf(buf, \"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n \t} else {\n \t\tenum object_type type;\n-\t\t*buf_p = read_sha1_file(ce->sha1, &type, size_p);\n-\t\tif (!*buf_p)\n+\t\tunsigned long sz;\n+\t\tchar *result;\n+\n+\t\tresult = read_sha1_file(ce->sha1, &type, &sz);\n+\t\tif (!result)\n \t\t\treturn -1;\n+\t\t/* XXX read_sha1_file NUL-terminates */\n+\t\tstrbuf_attach(buf, result, sz, sz + 1);\n \t}\n \treturn 0;\n }\n \n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n-\tchar *buf;\n-\tunsigned long size, alloc;\n-\tstruct buffer_desc desc;\n+\tstruct strbuf buf;\n \n-\tsize = 0;\n-\talloc = 0;\n-\tbuf = NULL;\n+\tstrbuf_init(&buf, 0);\n \tif (cached) {\n-\t\tif (read_file_or_gitlink(ce, &buf, &size))\n+\t\tif (read_file_or_gitlink(ce, &buf))\n \t\t\treturn error(\"read of %s failed\", patch->old_name);\n-\t\talloc = size;\n \t} else if (patch->old_name) {\n \t\tif (S_ISGITLINK(patch->old_mode)) {\n-\t\t\tif (ce)\n-\t\t\t\tread_file_or_gitlink(ce, &buf, &size);\n-\t\t\telse {\n+\t\t\tif (ce) {\n+\t\t\t\tread_file_or_gitlink(ce, &buf);\n+\t\t\t} else {\n \t\t\t\t/*\n \t\t\t\t * There is no way to apply subproject\n \t\t\t\t * patch without looking at the index.\n \t\t\t\t */\n \t\t\t\tpatch->fragments = NULL;\n-\t\t\t\tsize = 0;\n \t\t\t}\n-\t\t}\n-\t\telse {\n-\t\t\tsize = xsize_t(st->st_size);\n-\t\t\talloc = size + 8192;\n-\t\t\tbuf = xmalloc(alloc);\n-\t\t\tif (read_old_data(st, patch->old_name,\n-\t\t\t\t\t  &buf, &alloc, &size))\n-\t\t\t\treturn error(\"read of %s failed\",\n-\t\t\t\t\t     patch->old_name);\n+\t\t} else {\n+\t\t\tif (read_old_data(st, patch->old_name, &buf))\n+\t\t\t\treturn error(\"read of %s failed\", patch->old_name);\n \t\t}\n \t}\n \n-\tdesc.size = size;\n-\tdesc.alloc = alloc;\n-\tdesc.buffer = buf;\n-\n-\tif (apply_fragments(&desc, patch) < 0)\n+\tif (apply_fragments(&buf, patch) < 0)\n \t\treturn -1; /* note with --reject this succeeds. */\n-\n-\t/* NUL terminate the result */\n-\tif (desc.alloc <= desc.size)\n-\t\tdesc.buffer = xrealloc(desc.buffer, desc.size + 1);\n-\tdesc.buffer[desc.size] = 0;\n-\n-\tpatch->result = desc.buffer;\n-\tpatch->resultsize = desc.size;\n+\tpatch->result = buf.buf;\n+\tpatch->resultsize = buf.len;\n \n \tif (0 < patch->is_delete && patch->resultsize)\n \t\treturn error(\"removal patch leaves file contents\");\n@@ -2459,19 +2399,11 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf,\n \t\tsize = nbuf.len;\n \t\tbuf  = nbuf.buf;\n \t}\n+\twrite_or_die(fd, buf, size);\n+\tstrbuf_release(&nbuf);\n \n-\twhile (size) {\n-\t\tint written = xwrite(fd, buf, size);\n-\t\tif (written < 0)\n-\t\t\tdie(\"writing file %s: %s\", path, strerror(errno));\n-\t\tif (!written)\n-\t\t\tdie(\"out of space writing file %s\", path);\n-\t\tbuf += written;\n-\t\tsize -= written;\n-\t}\n \tif (close(fd) < 0)\n \t\tdie(\"closing file %s: %s\", path, strerror(errno));\n-\tstrbuf_release(&nbuf);\n \treturn 0;\n }\n \n-- \n1.5.3.1\n"},{"id":"53197","messageId":"20070916172134.GA26457@artemis.corp","threadId":"9866","inReplyTo":"20070915141210.GA27494@artemis.corp","subject":"Re: [RFC] strbuf's in builtin-apply","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T17:21:34Z","receivedAt":"2007-09-16T17:21:34Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Following this mail will happen a new janitoring series. This is a\nrewrite of the former, using Junio's advice to use strbufs in\nconvert_to_* functions. The patch hence becomes more intrusive than\nbefore (in convert.c mostly). Note that this imply that now strbuf.h is\nincluded from cache.h so all git sources see strbuf's.\n\nThe convert_to_git patches gain some marginal efficiency as the new API\nmakes the reuse of the buffers possible when in-place editing works\n(e.g. the \\r\\n -> \\n can be done in place, we save a malloc here). Else\nnothing should have changed significantly.\n\nThe last 2 patches are new. The first one is a simplification of the\ncode splicing the \"encoding\" header in commit.c, reusing the logic\nalready in strbuf.c for that matter, and also making the parsing code\neasier to read (IMHO).\n\nThe latter further simplify some code that was trying to guess if\nrfc2047 encoding of some header was needed. Thanks to strbuf_grow, and\nthe fact that now at each point we can grow buffers (which was harder\nbefore), I tried to wait until we are sure if rfc2047 encoding will be\nneeded or not to extend the buffer. I've benchmarked many tools (on real\nrepositories, with commiters having non ascii chars in their name) using\nthe pretty printer without noticeable changes in the numbers (and rather\nagain, a trend to be faster, but with less than a percent gain, so I\nwon't call it a real gain).\n\nThe series is based on next, as many patches are definitely not suitable\nfor master :)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53204","messageId":"20070916172833.GB26457@artemis.corp","threadId":"9866","inReplyTo":"20070916172134.GA26457@artemis.corp","subject":"Re: [RFC] strbuf's in builtin-apply","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T17:28:33Z","receivedAt":"2007-09-16T17:28:33Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"  Damn it I forgot to add -n to my format-patch invocation *again*. The\nproper ordering is:\n\nThe builtin-apply part:\n  [1/6] New strbuf APIs: splice and attach.\n  [2/6] Rewrite convert_to_{git,working_tree} to use strbuf's.\n  [3/6] Now that cache.h needs strbuf.h, remove useless includes.\n  [4/6] builtin-apply: use strbuf's instead of buffer_desc's.\n\nAnd the two somehow more independant patches (need 1/6 still):\n  [5/6] Refactor replace_encoding_header.\n  [6/6] Remove preemptive allocations.\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53207","messageId":"alpine.LFD.0.999.0709161126020.16478@woody.linux-foundation.org","threadId":"9866","inReplyTo":"20070916172233.90C9E1835B@madism.org","subject":"Re: [PATCH] Rewrite convert_to_{git,working_tree} to use strbuf's.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-09-16T18:27:36Z","receivedAt":"2007-09-16T18:27:36Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 16 Sep 2007, Pierre Habouzit wrote:\n..\n>  7 files changed, 240 insertions(+), 317 deletions(-)\n\nI like how all these patches seem to be removing more lines than \nthey add.\n\nThe end result looks good from a visual standpoint too.\n\nSo:\n\n\tAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n\nfor the whole series.\n\n\t\tLinus\n"},{"id":"53212","messageId":"874phugy6h.fsf@mid.deneb.enyo.de","threadId":"9866","inReplyTo":"20070916172233.8B8AD1835F@madism.org","subject":"Re: [PATCH] New strbuf APIs: splice and attach.","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2007-09-16T20:20:06Z","receivedAt":"2007-09-16T20:20:06Z","isPatch":true,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Pierre Habouzit:\n\n> +void strbuf_grow(struct strbuf *sb, size_t extra)\n> +{\n>  \tif (sb->len + extra + 1 <= sb->len)\n>  \t\tdie(\"you want to use way too much memory\");\n\nBy the way, this comparison is always false because sb->len is signed.\n"},{"id":"53216","messageId":"20070916205136.GE26457@artemis.corp","threadId":"9866","inReplyTo":"874phugy6h.fsf@mid.deneb.enyo.de","subject":"Re: [PATCH] New strbuf APIs: splice and attach.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-16T20:51:36Z","receivedAt":"2007-09-16T20:51:36Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, Sep 16, 2007 at 08:20:06PM +0000, Florian Weimer wrote:\n> * Pierre Habouzit:\n> \n> > +void strbuf_grow(struct strbuf *sb, size_t extra)\n> > +{\n> >  \tif (sb->len + extra + 1 <= sb->len)\n> >  \t\tdie(\"you want to use way too much memory\");\n> \n> By the way, this comparison is always false because sb->len is signed.\n\n  News to me. Actually it's not, it's a size_t :)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"53236","messageId":"7v3axe5ih1.fsf@gitster.siamese.dyndns.org","threadId":"9866","inReplyTo":"20070916172833.GB26457@artemis.corp","subject":"Re: [RFC] strbuf's in builtin-apply","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-16T22:54:50Z","receivedAt":"2007-09-16T22:54:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> The builtin-apply part:\n>   [1/6] New strbuf APIs: splice and attach.\n>   [2/6] Rewrite convert_to_{git,working_tree} to use strbuf's.\n>   [3/6] Now that cache.h needs strbuf.h, remove useless includes.\n>   [4/6] builtin-apply: use strbuf's instead of buffer_desc's.\n>\n> And the two somehow more independant patches (need 1/6 still):\n>   [5/6] Refactor replace_encoding_header.\n>   [6/6] Remove preemptive allocations.\n\nQuite nice.  Thanks, will queue.\n"},{"id":"53261","messageId":"87myvlzw1p.fsf@mid.deneb.enyo.de","threadId":"9866","inReplyTo":"20070916205136.GE26457@artemis.corp","subject":"Re: [PATCH] New strbuf APIs: splice and attach.","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2007-09-17T05:43:30Z","receivedAt":"2007-09-17T05:43:30Z","isPatch":true,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Pierre Habouzit:\n\n> On Sun, Sep 16, 2007 at 08:20:06PM +0000, Florian Weimer wrote:\n>> * Pierre Habouzit:\n>> \n>> > +void strbuf_grow(struct strbuf *sb, size_t extra)\n>> > +{\n>> >  \tif (sb->len + extra + 1 <= sb->len)\n>> >  \t\tdie(\"you want to use way too much memory\");\n>> \n>> By the way, this comparison is always false because sb->len is signed.\n>\n>   News to me. Actually it's not, it's a size_t :)\n\nAh, then this has changed somewhere.  It used to be int.  Good.\n"}]}