{"thread":{"id":"9932","subject":"let's refactor quoting ...","startedAt":"2007-09-18T17:18:02Z","lastAt":"2007-09-20T16:10:07Z","messageCount":29,"participants":["Pierre Habouzit","Junio C Hamano","Andreas Ericsson","David Kastrup","Edgar Toernig","Johannes Sixt","Kalle Olavi Niemitalo","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"53518","messageId":"20070918224119.17650344AB3@madism.org","threadId":"9932","inReplyTo":"20070918223947.GB4535@artemis.corp","subject":"[PATCH 1/5] strbuf API additions and enhancements.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-18T17:18:02Z","receivedAt":"2007-09-18T17:18:02Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Add strbuf_remove, change strbuf_insert:\n As both are special cases of strbuf_splice, implement them as such.\n gcc is able to do the math and generate almost optimal code this way.\n\nAdd strbuf_addvf (vsprintf-like)\n\nAdd strbuf_swap:\n Exchange the values of its arguments.\n Use it in fast-import.c\n\nAlso fix spacing issues in strbuf.h\n\nSigned-off-by: Pierre Habouzit \n---\n commit.c | 2 +-\n fast-import.c | 4 +---\n strbuf.c | 38 ++++++++++++++++++++++++++++----------\n strbuf.h | 19 +++++++++++++------\n 4 files changed, 43 insertions(+), 20 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex f86fa77..55b08ec 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -656,7 +656,7 @@ static char *replace_encoding_header(char *buf, const char *encoding)\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\tstrbuf_splice(&tmp, start, len, NULL, 0);\n+\t\tstrbuf_remove(&tmp, start, len);\n \t} else {\n \t\t/* just replaces XXXX in 'encoding XXXX\\n' */\n \t\tstrbuf_splice(&tmp, start + strlen(\"encoding \"),\ndiff --git a/fast-import.c b/fast-import.c\nindex f990658..eddae22 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1111,9 +1111,7 @@ static int store_object(\n \t\tif (last->no_swap) {\n \t\t\tlast->data = *dat;\n \t\t} else {\n-\t\t\tstruct strbuf tmp = *dat;\n-\t\t\t*dat = last->data;\n-\t\t\tlast->data = tmp;\n+\t\t\tstrbuf_swap(&last->data, dat);\n \t\t}\n \t\tlast->offset = e->offset;\n \t}\ndiff --git a/strbuf.c b/strbuf.c\nindex 59383ac..51aa2de 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -50,16 +50,6 @@ 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-{\n-\tstrbuf_grow(sb, len);\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_splice(struct strbuf *sb, size_t pos, size_t len,\n \t\t\t\t const void *data, size_t dlen)\n {\n@@ -79,6 +69,16 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n \tstrbuf_setlen(sb, sb->len + dlen - len);\n }\n \n+void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)\n+{\n+\tstrbuf_splice(sb, pos, 0, data, len);\n+}\n+\n+void strbuf_remove(struct strbuf *sb, size_t pos, size_t len)\n+{\n+\tstrbuf_splice(sb, pos, len, NULL, 0);\n+}\n+\n void strbuf_add(struct strbuf *sb, const void *data, size_t len)\n {\n \tstrbuf_grow(sb, len);\n@@ -109,6 +109,24 @@ void strbuf_addf(struct strbuf *sb, const char *fmt, ...)\n \tstrbuf_setlen(sb, sb->len + len);\n }\n \n+void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)\n+{\n+\tint len;\n+\n+\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n+\tif (len < 0) {\n+\t\tlen = 0;\n+\t}\n+\tif (len > strbuf_avail(sb)) {\n+\t\tstrbuf_grow(sb, len);\n+\t\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n+\t\tif (len > strbuf_avail(sb)) {\n+\t\t\tdie(\"this should not happen, your snprintf is broken\");\n+\t\t}\n+\t}\n+\tstrbuf_setlen(sb, sb->len + len);\n+}\n+\n size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)\n {\n \tsize_t res;\ndiff --git a/strbuf.h b/strbuf.h\nindex b2cbd97..ac3fb7b 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -55,15 +55,20 @@ 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+static inline void strbuf_swap(struct strbuf *a, struct strbuf *b) {\n+\tstruct strbuf tmp = *a;\n+\t*a = *b;\n+\t*b = tmp;\n+}\n \n /*----- strbuf size related -----*/\n static inline size_t strbuf_avail(struct strbuf *sb) {\n- return sb->alloc ? sb->alloc - sb->len - 1 : 0;\n+\treturn sb->alloc ? sb->alloc - sb->len - 1 : 0;\n }\n static inline void strbuf_setlen(struct strbuf *sb, size_t len) {\n- assert (len < sb->alloc);\n- sb->len = len;\n- sb->buf[len] = '\\0';\n+\tassert (len < sb->alloc);\n+\tsb->len = len;\n+\tsb->buf[len] = '\\0';\n }\n \n extern void strbuf_grow(struct strbuf *, size_t);\n@@ -78,12 +83,12 @@ static inline void strbuf_addch(struct strbuf *sb, int c) {\n \tsb->buf[sb->len] = '\\0';\n }\n \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+extern void strbuf_remove(struct strbuf *, size_t pos, size_t len);\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+ 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@@ -95,6 +100,8 @@ static inline void strbuf_addbuf(struct strbuf *sb, struct strbuf *sb2) {\n \n __attribute__((format(printf,2,3)))\n extern void strbuf_addf(struct strbuf *sb, const char *fmt, ...);\n+__attribute__((format(printf,2,0)))\n+extern void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list);\n \n extern size_t strbuf_fread(struct strbuf *, size_t, FILE *);\n /* XXX: if read fails, any partial read is undone */\n-- \n1.5.3.1\n"},{"id":"53517","messageId":"20070918224120.1DC44344AB3@madism.org","threadId":"9932","inReplyTo":"20070918223947.GB4535@artemis.corp","subject":"[PATCH 2/5] sq_quote_argv and add_to_string rework with strbuf's.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-18T20:15:16Z","receivedAt":"2007-09-18T20:15:16Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"* sq_quote_buf is made public, and works on a strbuf.\n* sq_quote_argv also works on a strbuf.\n* make sq_quote_argv take a \"maxlen\" argument to check the buffer won't grow\n too big.\n\nSigned-off-by: Pierre Habouzit \n---\n connect.c | 21 ++++++--------\n git.c | 16 +++-------\n quote.c | 91 ++++++++++++++++---------------------------------------------\n quote.h | 9 ++----\n rsh.c | 33 ++++++----------------\n trace.c | 35 +++++++-----------------\n 6 files changed, 60 insertions(+), 145 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 1653a0e..06d279e 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -577,16 +577,13 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)\n \tif (pid < 0)\n \t\tdie(\"unable to fork\");\n \tif (!pid) {\n-\t\tchar command[MAX_CMD_LEN];\n-\t\tchar *posn = command;\n-\t\tint size = MAX_CMD_LEN;\n-\t\tint of = 0;\n+\t\tstruct strbuf cmd;\n \n-\t\tof |= add_to_string(&posn, &size, prog, 0);\n-\t\tof |= add_to_string(&posn, &size, \" \", 0);\n-\t\tof |= add_to_string(&posn, &size, path, 1);\n-\n-\t\tif (of)\n+\t\tstrbuf_init(&cmd, MAX_CMD_LEN);\n+\t\tstrbuf_addstr(&cmd, prog);\n+\t\tstrbuf_addch(&cmd, ' ');\n+\t\tsq_quote_buf(&cmd, path);\n+\t\tif (cmd.len >= MAX_CMD_LEN)\n \t\t\tdie(\"command line too long\");\n \n \t\tdup2(pipefd[1][0], 0);\n@@ -606,10 +603,10 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)\n \t\t\t\tssh_basename++;\n \n \t\t\tif (!port)\n-\t\t\t\texeclp(ssh, ssh_basename, host, command, NULL);\n+\t\t\t\texeclp(ssh, ssh_basename, host, cmd.buf, NULL);\n \t\t\telse\n \t\t\t\texeclp(ssh, ssh_basename, \"-p\", port, host,\n-\t\t\t\t command, NULL);\n+\t\t\t\t cmd.buf, NULL);\n \t\t}\n \t\telse {\n \t\t\tunsetenv(ALTERNATE_DB_ENVIRONMENT);\n@@ -618,7 +615,7 @@ pid_t git_connect(int fd[2], char *url, const char *prog, int flags)\n \t\t\tunsetenv(GIT_WORK_TREE_ENVIRONMENT);\n \t\t\tunsetenv(GRAFT_ENVIRONMENT);\n \t\t\tunsetenv(INDEX_ENVIRONMENT);\n-\t\t\texeclp(\"sh\", \"sh\", \"-c\", command, NULL);\n+\t\t\texeclp(\"sh\", \"sh\", \"-c\", cmd.buf, NULL);\n \t\t}\n \t\tdie(\"exec failed\");\n \t}\ndiff --git a/git.c b/git.c\nindex 56ae8cc..9eaca1d 100644\n--- a/git.c\n+++ b/git.c\n@@ -187,19 +187,13 @@ static int handle_alias(int *argcp, const char ***argv)\n \tif (alias_string) {\n \t\tif (alias_string[0] == '!') {\n \t\t\tif (*argcp > 1) {\n-\t\t\t\tint i, sz = PATH_MAX;\n-\t\t\t\tchar *s = xmalloc(sz), *new_alias = s;\n+\t\t\t\tstruct strbuf buf;\n \n-\t\t\t\tadd_to_string(&s, &sz, alias_string, 0);\n+\t\t\t\tstrbuf_init(&buf, PATH_MAX);\n+\t\t\t\tstrbuf_addstr(&buf, alias_string);\n+\t\t\t\tsq_quote_argv(&buf, (*argv) + 1, *argcp - 1, PATH_MAX);\n \t\t\t\tfree(alias_string);\n-\t\t\t\talias_string = new_alias;\n-\t\t\t\tfor (i = 1; i < *argcp &&\n-\t\t\t\t\t!add_to_string(&s, &sz, \" \", 0) &&\n-\t\t\t\t\t!add_to_string(&s, &sz, (*argv)[i], 1)\n-\t\t\t\t\t; i++)\n-\t\t\t\t\t; /* do nothing */\n-\t\t\t\tif (!sz)\n-\t\t\t\t\tdie(\"Too many or long arguments\");\n+\t\t\t\talias_string = buf.buf;\n \t\t\t}\n \t\t\ttrace_printf(\"trace: alias to shell cmd: %s => %s\\n\",\n \t\t\t\t alias_command, alias_string + 1);\ndiff --git a/quote.c b/quote.c\nindex d88bf75..4df3262 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -20,29 +20,26 @@ static inline int need_bs_quote(char c)\n \treturn (c == '\\'' || c == '!');\n }\n \n-static size_t sq_quote_buf(char *dst, size_t n, const char *src)\n+void sq_quote_buf(struct strbuf *dst, const char *src)\n {\n-\tchar c;\n-\tchar *bp = dst;\n-\tsize_t len = 0;\n-\n-\tEMIT('\\'');\n-\twhile ((c = *src++)) {\n-\t\tif (need_bs_quote(c)) {\n-\t\t\tEMIT('\\'');\n-\t\t\tEMIT('\\\\');\n-\t\t\tEMIT(c);\n-\t\t\tEMIT('\\'');\n-\t\t} else {\n-\t\t\tEMIT(c);\n+\tchar *to_free = NULL;\n+\n+\tif (dst->buf == src)\n+\t\tto_free = strbuf_detach(dst);\n+\n+\tstrbuf_addch(dst, '\\'');\n+\twhile (*src) {\n+\t\tsize_t len = strcspn(src, \"'\\\\\");\n+\t\tstrbuf_add(dst, src, len);\n+\t\tsrc += len;\n+\t\twhile (need_bs_quote(*src)) {\n+\t\t\tstrbuf_addstr(dst, \"'\\\\\");\n+\t\t\tstrbuf_addch(dst, *src++);\n+\t\t\tstrbuf_addch(dst, '\\'');\n \t\t}\n \t}\n-\tEMIT('\\'');\n-\n-\tif ( n )\n-\t\t*bp = 0;\n-\n-\treturn len;\n+\tstrbuf_addch(dst, '\\'');\n+\tfree(to_free);\n }\n \n void sq_quote_print(FILE *stream, const char *src)\n@@ -62,11 +59,10 @@ void sq_quote_print(FILE *stream, const char *src)\n \tfputc('\\'', stream);\n }\n \n-char *sq_quote_argv(const char** argv, int count)\n+void sq_quote_argv(struct strbuf *dst, const char** argv, int count,\n+ size_t maxlen)\n {\n-\tchar *buf, *to;\n \tint i;\n-\tsize_t len = 0;\n \n \t/* Count argv if needed. */\n \tif (count < 0) {\n@@ -74,53 +70,14 @@ char *sq_quote_argv(const char** argv, int count)\n \t\t\t; /* just counting */\n \t}\n \n-\t/* Special case: no argv. */\n-\tif (!count)\n-\t\treturn xcalloc(1,1);\n-\n-\t/* Get destination buffer length. */\n-\tfor (i = 0; i < count; i++)\n-\t\tlen += sq_quote_buf(NULL, 0, argv[i]) + 1;\n-\n-\t/* Alloc destination buffer. */\n-\tto = buf = xmalloc(len + 1);\n-\n \t/* Copy into destination buffer. */\n+\tstrbuf_grow(dst, 32 * count);\n \tfor (i = 0; i < count; ++i) {\n-\t\t*to++ = ' ';\n-\t\tto += sq_quote_buf(to, len, argv[i]);\n+\t\tstrbuf_addch(dst, ' ');\n+\t\tsq_quote_buf(dst, argv[i]);\n+\t\tif (maxlen && dst->len > maxlen)\n+\t\t\tdie(\"Too many or long arguments\");\n \t}\n-\n-\treturn buf;\n-}\n-\n-/*\n- * Append a string to a string buffer, with or without shell quoting.\n- * Return true if the buffer overflowed.\n- */\n-int add_to_string(char **ptrp, int *sizep, const char *str, int quote)\n-{\n-\tchar *p = *ptrp;\n-\tint size = *sizep;\n-\tint oc;\n-\tint err = 0;\n-\n-\tif (quote)\n-\t\toc = sq_quote_buf(p, size, str);\n-\telse {\n-\t\toc = strlen(str);\n-\t\tmemcpy(p, str, (size <= oc) ? size - 1 : oc);\n-\t}\n-\n-\tif (size <= oc) {\n-\t\terr = 1;\n-\t\toc = size - 1;\n-\t}\n-\n-\t*ptrp += oc;\n-\t**ptrp = '\\0';\n-\t*sizep -= oc;\n-\treturn err;\n }\n \n char *sq_dequote(char *arg)\ndiff --git a/quote.h b/quote.h\nindex 8a59cc5..78e8d3e 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -29,13 +29,10 @@\n */\n \n extern void sq_quote_print(FILE *stream, const char *src);\n-extern char *sq_quote_argv(const char** argv, int count);\n \n-/*\n- * Append a string to a string buffer, with or without shell quoting.\n- * Return true if the buffer overflowed.\n- */\n-extern int add_to_string(char **ptrp, int *sizep, const char *str, int quote);\n+extern void sq_quote_buf(struct strbuf *, const char *src);\n+extern void sq_quote_argv(struct strbuf *, const char **argv, int count,\n+ size_t maxlen);\n \n /* This unwraps what sq_quote() produces in place, but returns\n * NULL if the input does not look like what sq_quote would have\ndiff --git a/rsh.c b/rsh.c\nindex 5754a23..e4ce255 100644\n--- a/rsh.c\n+++ b/rsh.c\n@@ -7,14 +7,10 @@\n int setup_connection(int *fd_in, int *fd_out, const char *remote_prog,\n \t\t char *url, int rmt_argc, char **rmt_argv)\n {\n+\tstruct strbuf cmd;\n \tchar *host;\n \tchar *path;\n \tint sv[2];\n-\tchar command[COMMAND_SIZE];\n-\tchar *posn;\n-\tint sizen;\n-\tint of;\n-\tint i;\n \tpid_t pid;\n \n \tif (!strcmp(url, \"-\")) {\n@@ -37,24 +33,13 @@ int setup_connection(int *fd_in, int *fd_out, const char *remote_prog,\n \t\treturn error(\"Bad URL: %s\", url);\n \t}\n \t/* $GIT_RSH \"env GIT_DIR= \" */\n-\tsizen = COMMAND_SIZE;\n-\tposn = command;\n-\tof = 0;\n-\tof |= add_to_string(&posn, &sizen, \"env \", 0);\n-\tof |= add_to_string(&posn, &sizen, GIT_DIR_ENVIRONMENT \"=\", 0);\n-\tof |= add_to_string(&posn, &sizen, path, 1);\n-\tof |= add_to_string(&posn, &sizen, \" \", 0);\n-\tof |= add_to_string(&posn, &sizen, remote_prog, 1);\n-\n-\tfor ( i = 0 ; i < rmt_argc ; i++ ) {\n-\t\tof |= add_to_string(&posn, &sizen, \" \", 0);\n-\t\tof |= add_to_string(&posn, &sizen, rmt_argv[i], 1);\n-\t}\n-\n-\tof |= add_to_string(&posn, &sizen, \" -\", 0);\n-\n-\tif ( of )\n-\t\treturn error(\"Command line too long\");\n+\tstrbuf_init(&cmd, COMMAND_SIZE);\n+\tstrbuf_addstr(&cmd, \"env \" GIT_DIR_ENVIRONMENT \"=\");\n+\tsq_quote_buf(&cmd, path);\n+\tstrbuf_addch(&cmd, ' ');\n+\tsq_quote_buf(&cmd, remote_prog);\n+\tsq_quote_argv(&cmd, (const char **)rmt_argv, rmt_argc, COMMAND_SIZE - 2);\n+\tstrbuf_addstr(&cmd, \" -\");\n \n \tif (socketpair(AF_UNIX, SOCK_STREAM, 0, sv))\n \t\treturn error(\"Couldn't create socket\");\n@@ -74,7 +59,7 @@ int setup_connection(int *fd_in, int *fd_out, const char *remote_prog,\n \t\tclose(sv[1]);\n \t\tdup2(sv[0], 0);\n \t\tdup2(sv[0], 1);\n-\t\texeclp(ssh, ssh_basename, host, command, NULL);\n+\t\texeclp(ssh, ssh_basename, host, cmd.buf, NULL);\n \t}\n \tclose(sv[0]);\n \t*fd_in = sv[1];\ndiff --git a/trace.c b/trace.c\nindex 7961a27..ed1cdf0 100644\n--- a/trace.c\n+++ b/trace.c\n@@ -113,39 +113,24 @@ void trace_printf(const char *format, ...)\n \n void trace_argv_printf(const char **argv, int count, const char *format, ...)\n {\n-\tchar *argv_str, *format_str, *trace_str;\n-\tsize_t argv_len, format_len, trace_len;\n-\tva_list rest;\n+\tstruct strbuf trace;\n+\tva_list ap;\n \tint need_close = 0;\n \tint fd = get_trace_fd(&need_close);\n \n \tif (!fd)\n \t\treturn;\n \n-\t/* Get the argv string. */\n-\targv_str = sq_quote_argv(argv, count);\n-\targv_len = strlen(argv_str);\n-\n-\t/* Get the formated string. */\n-\tva_start(rest, format);\n-\tnfvasprintf(&format_str, format, rest);\n-\tva_end(rest);\n+\tstrbuf_init(&trace, 0);\n \n-\t/* Allocate buffer for trace string. */\n-\tformat_len = strlen(format_str);\n-\ttrace_len = argv_len + format_len + 1; /* + 1 for \\n */\n-\ttrace_str = xmalloc(trace_len + 1);\n+\tva_start(ap, format);\n+\tstrbuf_addvf(&trace, format, ap);\n+\tva_end(ap);\n+\tsq_quote_argv(&trace, argv, count, 0);\n+\tstrbuf_addch(&trace, '\\n');\n \n-\t/* Copy everything into the trace string. */\n-\tstrncpy(trace_str, format_str, format_len);\n-\tstrncpy(trace_str + format_len, argv_str, argv_len);\n-\tstrcpy(trace_str + trace_len - 1, \"\\n\");\n-\n-\twrite_or_whine_pipe(fd, trace_str, trace_len, err_msg);\n-\n-\tfree(argv_str);\n-\tfree(format_str);\n-\tfree(trace_str);\n+\twrite_or_whine_pipe(fd, trace.buf, trace.len, err_msg);\n+\tstrbuf_release(&trace);\n \n \tif (need_close)\n \t\tclose(fd);\n-- \n1.5.3.1\n"},{"id":"53521","messageId":"20070918224121.24C3B344AB3@madism.org","threadId":"9932","inReplyTo":"20070918223947.GB4535@artemis.corp","subject":"[PATCH 3/5] Rework unquote_c_style to work on a strbuf.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-18T21:22:47Z","receivedAt":"2007-09-18T21:22:47Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"If the gain is not obvious in the diffstat, the resulting code is more\nreadable, _and_ in checkout-index/update-index we now reuse the same buffer\nto unquote strings instead of always freeing/mallocing.\n\nThis also is more coherent with the next patch that reworks quoting\nfunctions.\n\nThe quoting function is also made more efficient scanning for backslashes\nand treating portions of strings without a backslash at once.\n\nSigned-off-by: Pierre Habouzit \n---\n builtin-apply.c | 125 +++++++++++++++++++++++-----------------------\n builtin-checkout-index.c | 27 +++++-----\n builtin-update-index.c | 51 ++++++++++---------\n fast-import.c | 47 ++++++++---------\n mktree.c | 25 +++++----\n quote.c | 92 ++++++++++++++++------------------\n quote.h | 2 +-\n 7 files changed, 184 insertions(+), 185 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 6a5e389..cffbe52 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -231,35 +231,33 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n {\n \tint len;\n \tconst char *start = line;\n-\tchar *name;\n \n \tif (*line == '\"') {\n+\t\tstruct strbuf name;\n+\n \t\t/* Proposed \"new-style\" GNU patch/diff format; see\n \t\t * http://marc.theaimsgroup.com/?l=git&m=112927316408690&w=2\n \t\t */\n-\t\tname = unquote_c_style(line, NULL);\n-\t\tif (name) {\n-\t\t\tchar *cp = name;\n-\t\t\twhile (p_value) {\n-\t\t\t\tcp = strchr(name, '/');\n+\t\tstrbuf_init(&name, 0);\n+\t\tif (!unquote_c_style(&name, line, NULL)) {\n+\t\t\tchar *cp;\n+\n+\t\t\tfor (cp = name.buf; p_value; p_value--) {\n+\t\t\t\tcp = strchr(name.buf, '/');\n \t\t\t\tif (!cp)\n \t\t\t\t\tbreak;\n \t\t\t\tcp++;\n-\t\t\t\tp_value--;\n \t\t\t}\n \t\t\tif (cp) {\n \t\t\t\t/* name can later be freed, so we need\n \t\t\t\t * to memmove, not just return cp\n \t\t\t\t */\n-\t\t\t\tmemmove(name, cp, strlen(cp) + 1);\n+\t\t\t\tstrbuf_remove(&name, 0, cp - name.buf);\n \t\t\t\tfree(def);\n-\t\t\t\treturn name;\n-\t\t\t}\n-\t\t\telse {\n-\t\t\t\tfree(name);\n-\t\t\t\tname = NULL;\n+\t\t\t\treturn name.buf;\n \t\t\t}\n \t\t}\n+\t\tstrbuf_release(&name);\n \t}\n \n \tfor (;;) {\n@@ -566,29 +564,30 @@ static const char *stop_at_slash(const char *line, int llen)\n */\n static char *git_header_name(char *line, int llen)\n {\n-\tint len;\n \tconst char *name;\n \tconst char *second = NULL;\n+\tsize_t len;\n \n \tline += strlen(\"diff --git \");\n \tllen -= strlen(\"diff --git \");\n \n \tif (*line == '\"') {\n \t\tconst char *cp;\n-\t\tchar *first = unquote_c_style(line, &second);\n-\t\tif (!first)\n-\t\t\treturn NULL;\n+\t\tstruct strbuf first;\n+\t\tstruct strbuf sp;\n+\n+\t\tstrbuf_init(&first, 0);\n+\t\tstrbuf_init(&sp, 0);\n+\n+\t\tif (unquote_c_style(&first, line, &second))\n+\t\t\tgoto free_and_fail1;\n \n \t\t/* advance to the first slash */\n-\t\tcp = stop_at_slash(first, strlen(first));\n-\t\tif (!cp || cp == first) {\n-\t\t\t/* we do not accept absolute paths */\n-\t\tfree_first_and_fail:\n-\t\t\tfree(first);\n-\t\t\treturn NULL;\n-\t\t}\n-\t\tlen = strlen(cp+1);\n-\t\tmemmove(first, cp+1, len+1); /* including NUL */\n+\t\tcp = stop_at_slash(first.buf, first.len);\n+\t\t/* we do not accept absolute paths */\n+\t\tif (!cp || cp == first.buf)\n+\t\t\tgoto free_and_fail1;\n+\t\tstrbuf_remove(&first, 0, cp + 1 - first.buf);\n \n \t\t/* second points at one past closing dq of name.\n \t\t * find the second name.\n@@ -597,40 +596,40 @@ static char *git_header_name(char *line, int llen)\n \t\t\tsecond++;\n \n \t\tif (line + llen <= second)\n-\t\t\tgoto free_first_and_fail;\n+\t\t\tgoto free_and_fail1;\n \t\tif (*second == '\"') {\n-\t\t\tchar *sp = unquote_c_style(second, NULL);\n-\t\t\tif (!sp)\n-\t\t\t\tgoto free_first_and_fail;\n-\t\t\tcp = stop_at_slash(sp, strlen(sp));\n-\t\t\tif (!cp || cp == sp) {\n-\t\t\tfree_both_and_fail:\n-\t\t\t\tfree(sp);\n-\t\t\t\tgoto free_first_and_fail;\n-\t\t\t}\n+\t\t\tif (unquote_c_style(&sp, second, NULL))\n+\t\t\t\tgoto free_and_fail1;\n+\t\t\tcp = stop_at_slash(sp.buf, sp.len);\n+\t\t\tif (!cp || cp == sp.buf)\n+\t\t\t\tgoto free_and_fail1;\n \t\t\t/* They must match, otherwise ignore */\n-\t\t\tif (strcmp(cp+1, first))\n-\t\t\t\tgoto free_both_and_fail;\n-\t\t\tfree(sp);\n-\t\t\treturn first;\n+\t\t\tif (strcmp(cp + 1, first.buf))\n+\t\t\t\tgoto free_and_fail1;\n+\t\t\tstrbuf_release(&sp);\n+\t\t\treturn first.buf;\n \t\t}\n \n \t\t/* unquoted second */\n \t\tcp = stop_at_slash(second, line + llen - second);\n \t\tif (!cp || cp == second)\n-\t\t\tgoto free_first_and_fail;\n+\t\t\tgoto free_and_fail1;\n \t\tcp++;\n-\t\tif (line + llen - cp != len + 1 ||\n-\t\t memcmp(first, cp, len))\n-\t\t\tgoto free_first_and_fail;\n-\t\treturn first;\n+\t\tif (line + llen - cp != first.len + 1 ||\n+\t\t memcmp(first.buf, cp, first.len))\n+\t\t\tgoto free_and_fail1;\n+\t\treturn first.buf;\n+\n+\tfree_and_fail1:\n+\t\tstrbuf_release(&first);\n+\t\tstrbuf_release(&sp);\n+\t\treturn NULL;\n \t}\n \n \t/* unquoted first name */\n \tname = stop_at_slash(line, llen);\n \tif (!name || name == line)\n \t\treturn NULL;\n-\n \tname++;\n \n \t/* since the first name is unquoted, a dq if exists must be\n@@ -638,28 +637,30 @@ static char *git_header_name(char *line, int llen)\n \t */\n \tfor (second = name; second < line + llen; second++) {\n \t\tif (*second == '\"') {\n-\t\t\tconst char *cp = second;\n+\t\t\tstruct strbuf sp;\n \t\t\tconst char *np;\n-\t\t\tchar *sp = unquote_c_style(second, NULL);\n-\n-\t\t\tif (!sp)\n-\t\t\t\treturn NULL;\n-\t\t\tnp = stop_at_slash(sp, strlen(sp));\n-\t\t\tif (!np || np == sp) {\n-\t\t\tfree_second_and_fail:\n-\t\t\t\tfree(sp);\n-\t\t\t\treturn NULL;\n-\t\t\t}\n+\n+\t\t\tstrbuf_init(&sp, 0);\n+\t\t\tif (unquote_c_style(&sp, second, NULL))\n+\t\t\t\tgoto free_and_fail2;\n+\n+\t\t\tnp = stop_at_slash(sp.buf, sp.len);\n+\t\t\tif (!np || np == sp.buf)\n+\t\t\t\tgoto free_and_fail2;\n \t\t\tnp++;\n-\t\t\tlen = strlen(np);\n-\t\t\tif (len < cp - name &&\n+\n+\t\t\tlen = sp.buf + sp.len - np;\n+\t\t\tif (len < second - name &&\n \t\t\t !strncmp(np, name, len) &&\n \t\t\t isspace(name[len])) {\n \t\t\t\t/* Good */\n-\t\t\t\tmemmove(sp, np, len + 1);\n-\t\t\t\treturn sp;\n+\t\t\t\tstrbuf_remove(&sp, 0, np - sp.buf);\n+\t\t\t\treturn sp.buf;\n \t\t\t}\n-\t\t\tgoto free_second_and_fail;\n+\n+\t\tfree_and_fail2:\n+\t\t\tstrbuf_release(&sp);\n+\t\t\treturn NULL;\n \t\t}\n \t}\n \ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex a18ecc4..e6264c4 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -270,26 +270,27 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (read_from_stdin) {\n-\t\tstruct strbuf buf;\n+\t\tstruct strbuf buf, nbuf;\n+\n \t\tif (all)\n \t\t\tdie(\"git-checkout-index: don't mix '--all' and '--stdin'\");\n+\n \t\tstrbuf_init(&buf, 0);\n-\t\twhile (1) {\n-\t\t\tchar *path_name;\n+\t\tstrbuf_init(&nbuf, 0);\n+\t\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n \t\t\tconst char *p;\n-\t\t\tif (strbuf_getline(&buf, stdin, line_termination) == EOF)\n-\t\t\t\tbreak;\n-\t\t\tif (line_termination && buf.buf[0] == '\"')\n-\t\t\t\tpath_name = unquote_c_style(buf.buf, NULL);\n-\t\t\telse\n-\t\t\t\tpath_name = buf.buf;\n-\t\t\tp = prefix_path(prefix, prefix_length, path_name);\n+\t\t\tif (line_termination && buf.buf[0] == '\"') {\n+\t\t\t\tstrbuf_reset(&nbuf);\n+\t\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n+\t\t\t\t\tdie(\"line is badly quoted\");\n+\t\t\t\tstrbuf_swap(&buf, &nbuf);\n+\t\t\t}\n+\t\t\tp = prefix_path(prefix, prefix_length, buf.buf);\n \t\t\tcheckout_file(p, prefix_length);\n-\t\t\tif (p < path_name || p > path_name + strlen(path_name))\n+\t\t\tif (p < buf.buf || p > buf.buf + buf.len)\n \t\t\t\tfree((char *)p);\n-\t\t\tif (path_name != buf.buf)\n-\t\t\t\tfree(path_name);\n \t\t}\n+\t\tstrbuf_release(&nbuf);\n \t\tstrbuf_release(&buf);\n \t}\n \ndiff --git a/builtin-update-index.c b/builtin-update-index.c\nindex acd5ab5..c76879e 100644\n--- a/builtin-update-index.c\n+++ b/builtin-update-index.c\n@@ -295,8 +295,11 @@ static void update_one(const char *path, const char *prefix, int prefix_length)\n static void read_index_info(int line_termination)\n {\n \tstruct strbuf buf;\n+\tstruct strbuf uq;\n+\n \tstrbuf_init(&buf, 0);\n-\twhile (1) {\n+\tstrbuf_init(&uq, 0);\n+\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n \t\tchar *ptr, *tab;\n \t\tchar *path_name;\n \t\tunsigned char sha1[20];\n@@ -320,9 +323,6 @@ static void read_index_info(int line_termination)\n \t\t * This format is to put higher order stages into the\n \t\t * index file and matches git-ls-files --stage output.\n \t\t */\n-\t\tif (strbuf_getline(&buf, stdin, line_termination) == EOF)\n-\t\t\tbreak;\n-\n \t\terrno = 0;\n \t\tul = strtoul(buf.buf, &ptr, 8);\n \t\tif (ptr == buf.buf || *ptr != ' '\n@@ -347,15 +347,17 @@ static void read_index_info(int line_termination)\n \t\tif (get_sha1_hex(tab - 40, sha1) || tab[-41] != ' ')\n \t\t\tgoto bad_line;\n \n-\t\tif (line_termination && ptr[0] == '\"')\n-\t\t\tpath_name = unquote_c_style(ptr, NULL);\n-\t\telse\n-\t\t\tpath_name = ptr;\n+\t\tpath_name = ptr;\n+\t\tif (line_termination && path_name[0] == '\"') {\n+\t\t\tstrbuf_reset(&uq);\n+\t\t\tif (unquote_c_style(&uq, path_name, NULL)) {\n+\t\t\t\tdie(\"git-update-index: bad quoting of path name\");\n+\t\t\t}\n+\t\t\tpath_name = uq.buf;\n+\t\t}\n \n \t\tif (!verify_path(path_name)) {\n \t\t\tfprintf(stderr, \"Ignoring path %s\\n\", path_name);\n-\t\t\tif (path_name != ptr)\n-\t\t\t\tfree(path_name);\n \t\t\tcontinue;\n \t\t}\n \n@@ -383,6 +385,7 @@ static void read_index_info(int line_termination)\n \t\tdie(\"malformed index info %s\", buf.buf);\n \t}\n \tstrbuf_release(&buf);\n+\tstrbuf_release(&uq);\n }\n \n static const char update_index_usage[] =\n@@ -705,26 +708,26 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tfree((char*)p);\n \t}\n \tif (read_from_stdin) {\n-\t\tstruct strbuf buf;\n+\t\tstruct strbuf buf, nbuf;\n+\n \t\tstrbuf_init(&buf, 0);\n-\t\twhile (1) {\n-\t\t\tchar *path_name;\n+\t\tstrbuf_init(&nbuf, 0);\n+\t\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n \t\t\tconst char *p;\n-\t\t\tif (strbuf_getline(&buf, stdin, line_termination) == EOF)\n-\t\t\t\tbreak;\n-\t\t\tif (line_termination && buf.buf[0] == '\"')\n-\t\t\t\tpath_name = unquote_c_style(buf.buf, NULL);\n-\t\t\telse\n-\t\t\t\tpath_name = buf.buf;\n-\t\t\tp = prefix_path(prefix, prefix_length, path_name);\n+\t\t\tif (line_termination && buf.buf[0] == '\"') {\n+\t\t\t\tstrbuf_reset(&nbuf);\n+\t\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n+\t\t\t\t\tdie(\"line is badly quoted\");\n+\t\t\t\tstrbuf_swap(&buf, &nbuf);\n+\t\t\t}\n+\t\t\tp = prefix_path(prefix, prefix_length, buf.buf);\n \t\t\tupdate_one(p, NULL, 0);\n \t\t\tif (set_executable_bit)\n \t\t\t\tchmod_path(set_executable_bit, p);\n-\t\t\tif (p < path_name || p > path_name + strlen(path_name))\n-\t\t\t\tfree((char*) p);\n-\t\t\tif (path_name != buf.buf)\n-\t\t\t\tfree(path_name);\n+\t\t\tif (p < buf.buf || p > buf.buf + buf.len)\n+\t\t\t\tfree((char *)p);\n \t\t}\n+\t\tstrbuf_release(&nbuf);\n \t\tstrbuf_release(&buf);\n \t}\n \ndiff --git a/fast-import.c b/fast-import.c\nindex eddae22..a870a44 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1759,7 +1759,7 @@ static void load_branch(struct branch *b)\n static void file_change_m(struct branch *b)\n {\n \tconst char *p = command_buf.buf + 2;\n-\tchar *p_uq;\n+\tstatic struct strbuf uq = STRBUF_INIT;\n \tconst char *endp;\n \tstruct object_entry *oe = oe;\n \tunsigned char sha1[20];\n@@ -1797,18 +1797,20 @@ static void file_change_m(struct branch *b)\n \tif (*p++ != ' ')\n \t\tdie(\"Missing space after SHA1: %s\", command_buf.buf);\n \n-\tp_uq = unquote_c_style(p, &endp);\n-\tif (p_uq) {\n+\tstrbuf_reset(&uq);\n+\tif (!unquote_c_style(&uq, p, &endp)) {\n \t\tif (*endp)\n \t\t\tdie(\"Garbage after path in: %s\", command_buf.buf);\n-\t\tp = p_uq;\n+\t\tp = uq.buf;\n \t}\n \n \tif (inline_data) {\n \t\tstatic struct strbuf buf = STRBUF_INIT;\n \n-\t\tif (!p_uq)\n-\t\t\tp = p_uq = xstrdup(p);\n+\t\tif (p != uq.buf) {\n+\t\t\tstrbuf_addstr(&uq, p);\n+\t\t\tp = uq.buf;\n+\t\t}\n \t\tread_next_command();\n \t\tcmd_data(&buf);\n \t\tstore_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);\n@@ -1826,56 +1828,54 @@ static void file_change_m(struct branch *b)\n \t}\n \n \ttree_content_set(&b->branch_tree, p, sha1, S_IFREG | mode, NULL);\n-\tfree(p_uq);\n }\n \n static void file_change_d(struct branch *b)\n {\n \tconst char *p = command_buf.buf + 2;\n-\tchar *p_uq;\n+\tstatic struct strbuf uq = STRBUF_INIT;\n \tconst char *endp;\n \n-\tp_uq = unquote_c_style(p, &endp);\n-\tif (p_uq) {\n+\tstrbuf_reset(&uq);\n+\tif (!unquote_c_style(&uq, p, &endp)) {\n \t\tif (*endp)\n \t\t\tdie(\"Garbage after path in: %s\", command_buf.buf);\n-\t\tp = p_uq;\n+\t\tp = uq.buf;\n \t}\n \ttree_content_remove(&b->branch_tree, p, NULL);\n-\tfree(p_uq);\n }\n \n static void file_change_cr(struct branch *b, int rename)\n {\n \tconst char *s, *d;\n-\tchar *s_uq, *d_uq;\n+\tstatic struct strbuf s_uq = STRBUF_INIT;\n+\tstatic struct strbuf d_uq = STRBUF_INIT;\n \tconst char *endp;\n \tstruct tree_entry leaf;\n \n \ts = command_buf.buf + 2;\n-\ts_uq = unquote_c_style(s, &endp);\n-\tif (s_uq) {\n+\tstrbuf_reset(&s_uq);\n+\tif (!unquote_c_style(&s_uq, s, &endp)) {\n \t\tif (*endp != ' ')\n \t\t\tdie(\"Missing space after source: %s\", command_buf.buf);\n-\t}\n-\telse {\n+\t} else {\n \t\tendp = strchr(s, ' ');\n \t\tif (!endp)\n \t\t\tdie(\"Missing space after source: %s\", command_buf.buf);\n-\t\ts_uq = xmemdupz(s, endp - s);\n+\t\tstrbuf_add(&s_uq, s, endp - s);\n \t}\n-\ts = s_uq;\n+\ts = s_uq.buf;\n \n \tendp++;\n \tif (!*endp)\n \t\tdie(\"Missing dest: %s\", command_buf.buf);\n \n \td = endp;\n-\td_uq = unquote_c_style(d, &endp);\n-\tif (d_uq) {\n+\tstrbuf_reset(&d_uq);\n+\tif (!unquote_c_style(&d_uq, d, &endp)) {\n \t\tif (*endp)\n \t\t\tdie(\"Garbage after dest in: %s\", command_buf.buf);\n-\t\td = d_uq;\n+\t\td = d_uq.buf;\n \t}\n \n \tmemset(&leaf, 0, sizeof(leaf));\n@@ -1889,9 +1889,6 @@ static void file_change_cr(struct branch *b, int rename)\n \t\tleaf.versions[1].sha1,\n \t\tleaf.versions[1].mode,\n \t\tleaf.tree);\n-\n-\tfree(s_uq);\n-\tfree(d_uq);\n }\n \n static void file_change_deleteall(struct branch *b)\ndiff --git a/mktree.c b/mktree.c\nindex 9c137de..e0da110 100644\n--- a/mktree.c\n+++ b/mktree.c\n@@ -66,6 +66,7 @@ static const char mktree_usage[] = \"git-mktree [-z]\";\n int main(int ac, char **av)\n {\n \tstruct strbuf sb;\n+\tstruct strbuf p_uq;\n \tunsigned char sha1[20];\n \tint line_termination = '\\n';\n \n@@ -82,14 +83,13 @@ int main(int ac, char **av)\n \t}\n \n \tstrbuf_init(&sb, 0);\n-\twhile (1) {\n+\tstrbuf_init(&p_uq, 0);\n+\twhile (strbuf_getline(&sb, stdin, line_termination) != EOF) {\n \t\tchar *ptr, *ntr;\n \t\tunsigned mode;\n \t\tenum object_type type;\n \t\tchar *path;\n \n-\t\tif (strbuf_getline(&sb, stdin, line_termination) == EOF)\n-\t\t\tbreak;\n \t\tptr = sb.buf;\n \t\t/* Input is non-recursive ls-tree output format\n \t\t * mode SP type SP sha1 TAB name\n@@ -109,18 +109,21 @@ int main(int ac, char **av)\n \t\t*ntr++ = 0; /* now at the beginning of SHA1 */\n \t\tif (type != type_from_string(ptr))\n \t\t\tdie(\"object type %s mismatch (%s)\", ptr, typename(type));\n-\t\tntr += 41; /* at the beginning of name */\n-\t\tif (line_termination && ntr[0] == '\"')\n-\t\t\tpath = unquote_c_style(ntr, NULL);\n-\t\telse\n-\t\t\tpath = ntr;\n \n-\t\tappend_to_tree(mode, sha1, path);\n+\t\tpath = ntr + 41; /* at the beginning of name */\n+\t\tif (line_termination && path[0] == '\"') {\n+\t\t\tstrbuf_reset(&p_uq);\n+\t\t\tif (unquote_c_style(&p_uq, path, NULL)) {\n+\t\t\t\tdie(\"invalid quoting\");\n+\t\t\t}\n+\t\t\tpath = p_uq.buf;\n+\t\t}\n \n-\t\tif (path != ntr)\n-\t\t\tfree(path);\n+\t\tappend_to_tree(mode, sha1, path);\n \t}\n+\tstrbuf_release(&p_uq);\n \tstrbuf_release(&sb);\n+\n \twrite_tree(sha1);\n \tputs(sha1_to_hex(sha1));\n \texit(0);\ndiff --git a/quote.c b/quote.c\nindex 4df3262..67c6527 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -201,68 +201,62 @@ int quote_c_style(const char *name, char *outbuf, FILE *outfp, int no_dq)\n * should free when done. Updates endp pointer to point at\n * one past the ending double quote if given.\n */\n-\n-char *unquote_c_style(const char *quoted, const char **endp)\n+int unquote_c_style(struct strbuf *sb, const char *quoted, const char **endp)\n {\n-\tconst char *sp;\n-\tchar *name = NULL, *outp = NULL;\n-\tint count = 0, ch, ac;\n-\n-#undef EMIT\n-#define EMIT(c) (outp ? (*outp++ = (c)) : (count++))\n+\tsize_t oldlen = sb->len, len;\n+\tint ch, ac;\n \n \tif (*quoted++ != '\"')\n-\t\treturn NULL;\n+\t\treturn -1;\n+\n+\tfor (;;) {\n+\t\tlen = strcspn(quoted, \"\\\"\\\\\");\n+\t\tstrbuf_add(sb, quoted, len);\n+\t\tquoted += len;\n+\n+\t\tswitch (*quoted++) {\n+\t\t case '\"':\n+\t\t\tif (endp)\n+\t\t\t\t*endp = quoted + 1;\n+\t\t\treturn 0;\n+\t\t case '\\\\':\n+\t\t\tbreak;\n+\t\t default:\n+\t\t\tgoto error;\n+\t\t}\n+\n+\t\tswitch ((ch = *quoted++)) {\n+\t\tcase 'a': ch = '\\a'; break;\n+\t\tcase 'b': ch = '\\b'; break;\n+\t\tcase 'f': ch = '\\f'; break;\n+\t\tcase 'n': ch = '\\n'; break;\n+\t\tcase 'r': ch = '\\r'; break;\n+\t\tcase 't': ch = '\\t'; break;\n+\t\tcase 'v': ch = '\\v'; break;\n \n-\twhile (1) {\n-\t\t/* first pass counts and allocates, second pass fills */\n-\t\tfor (sp = quoted; (ch = *sp++) != '\"'; ) {\n-\t\t\tif (ch == '\\\\') {\n-\t\t\t\tswitch (ch = *sp++) {\n-\t\t\t\tcase 'a': ch = '\\a'; break;\n-\t\t\t\tcase 'b': ch = '\\b'; break;\n-\t\t\t\tcase 'f': ch = '\\f'; break;\n-\t\t\t\tcase 'n': ch = '\\n'; break;\n-\t\t\t\tcase 'r': ch = '\\r'; break;\n-\t\t\t\tcase 't': ch = '\\t'; break;\n-\t\t\t\tcase 'v': ch = '\\v'; break;\n-\n-\t\t\t\tcase '\\\\': case '\"':\n-\t\t\t\t\tbreak; /* verbatim */\n-\n-\t\t\t\tcase '0':\n-\t\t\t\tcase '1':\n-\t\t\t\tcase '2':\n-\t\t\t\tcase '3':\n-\t\t\t\tcase '4':\n-\t\t\t\tcase '5':\n-\t\t\t\tcase '6':\n-\t\t\t\tcase '7':\n-\t\t\t\t\t/* octal */\n+\t\tcase '\\\\': case '\"':\n+\t\t\tbreak; /* verbatim */\n+\n+\t\t/* octal values with first digit over 4 overflow */\n+\t\tcase '0': case '1': case '2': case '3':\n \t\t\t\t\tac = ((ch - '0') << 6);\n-\t\t\t\t\tif ((ch = *sp++) < '0' || '7' < ch)\n-\t\t\t\t\t\treturn NULL;\n+\t\t\tif ((ch = *quoted++) < '0' || '7' < ch)\n+\t\t\t\tgoto error;\n \t\t\t\t\tac |= ((ch - '0') << 3);\n-\t\t\t\t\tif ((ch = *sp++) < '0' || '7' < ch)\n-\t\t\t\t\t\treturn NULL;\n+\t\t\tif ((ch = *quoted++) < '0' || '7' < ch)\n+\t\t\t\tgoto error;\n \t\t\t\t\tac |= (ch - '0');\n \t\t\t\t\tch = ac;\n \t\t\t\t\tbreak;\n \t\t\t\tdefault:\n-\t\t\t\t\treturn NULL; /* malformed */\n-\t\t\t\t}\n+\t\t\tgoto error;\n \t\t\t}\n-\t\t\tEMIT(ch);\n+\t\tstrbuf_addch(sb, ch);\n \t\t}\n \n-\t\tif (name) {\n-\t\t\t*outp = 0;\n-\t\t\tif (endp)\n-\t\t\t\t*endp = sp;\n-\t\t\treturn name;\n-\t\t}\n-\t\toutp = name = xmalloc(count + 1);\n-\t}\n+ error:\n+\tstrbuf_setlen(sb, oldlen);\n+\treturn -1;\n }\n \n void write_name_quoted(const char *prefix, int prefix_len,\ndiff --git a/quote.h b/quote.h\nindex 78e8d3e..6407c4d 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -40,9 +40,9 @@ extern void sq_quote_argv(struct strbuf *, const char **argv, int count,\n */\n extern char *sq_dequote(char *);\n \n+extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n extern int quote_c_style(const char *name, char *outbuf, FILE *outfp,\n \t\t\t int nodq);\n-extern char *unquote_c_style(const char *quoted, const char **endp);\n \n extern void write_name_quoted(const char *prefix, int prefix_len,\n \t\t\t const char *name, int quote, FILE *out);\n-- \n1.5.3.1\n"},{"id":"53519","messageId":"20070918224123.3251C344AB3@madism.org","threadId":"9932","inReplyTo":"20070918223947.GB4535@artemis.corp","subject":"[PATCH 5/5] Avoid duplicating memory, and use xmemdupz instead of xstrdup.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-18T21:48:22Z","receivedAt":"2007-09-18T21:48:22Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"Signed-off-by: Pierre Habouzit \n---\n fetch.c | 23 +++++++++--------------\n 1 files changed, 9 insertions(+), 14 deletions(-)\n\ndiff --git a/fetch.c b/fetch.c\nindex b1c1f07..479ec02 100644\n--- a/fetch.c\n+++ b/fetch.c\n@@ -218,24 +218,19 @@ int pull_targets_stdin(char ***target, const char ***write_ref)\n \tstruct strbuf buf;\n \t*target = NULL; *write_ref = NULL;\n \tstrbuf_init(&buf, 0);\n-\twhile (1) {\n-\t\tchar *rf_one = NULL;\n-\t\tchar *tg_one;\n-\n-\t\tif (strbuf_getline(&buf, stdin, '\\n') == EOF)\n-\t\t\tbreak;\n-\t\ttg_one = buf.buf;\n-\t\trf_one = strchr(tg_one, '\\t');\n-\t\tif (rf_one)\n-\t\t\t*rf_one++ = 0;\n+\twhile (strbuf_getline(&buf, stdin, '\\n') != EOF) {\n+\t\tchar *rf_one = memchr(buf.buf, '\\t', buf.len);\n \n \t\tif (targets >= targets_alloc) {\n-\t\t\ttargets_alloc = targets_alloc ? targets_alloc * 2 : 64;\n-\t\t\t*target = xrealloc(*target, targets_alloc * sizeof(**target));\n+\t\t\tALLOC_GROW(target, targets, targets_alloc);\n \t\t\t*write_ref = xrealloc(*write_ref, targets_alloc * sizeof(**write_ref));\n \t\t}\n-\t\t(*target)[targets] = xstrdup(tg_one);\n-\t\t(*write_ref)[targets] = rf_one ? xstrdup(rf_one) : NULL;\n+\t\tif (rf_one) {\n+\t\t\t(*write_ref)[targets] = xmemdupz(rf_one, buf.len - (rf_one - buf.buf));\n+\t\t} else {\n+\t\t\t(*write_ref)[targets] = NULL;\n+\t\t}\n+\t\t(*target)[targets] = strbuf_detach(&buf);\n \t\ttargets++;\n \t}\n \tstrbuf_release(&buf);\n-- \n1.5.3.1\n"},{"id":"53520","messageId":"20070918224122.2B55D344AB3@madism.org","threadId":"9932","inReplyTo":"20070918223947.GB4535@artemis.corp","subject":"[PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-18T22:00:51Z","receivedAt":"2007-09-18T22:00:51Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"* quote_c_style works on a strbuf instead of a wild buffer.\n* quote_c_style is now clever enough to not add double quotes if not needed.\n\n* write_name_quoted inherits those advantages, but also take a different\n set of arguments. Now instead of asking for quotes or not, you pass a\n \"terminator\". If it's \\0 then we assume you don't want to escape, else C\n escaping is performed. In any case, the terminator is also appended to the\n stream. It also no longer takes the prefix/prefix_len arguments, as it's\n seldomly used, and makes some optimizations harder.\n\n* write_name_quotedpfx is created to work like write_name_quoted and take\n the prefix/prefix_len arguments.\n\nThanks to those API changes, diff.c has somehow lost weight, thanks to the\nremoval of functions that were wrappers around the old write_name_quoted\ntrying to give it a semantics like the new one, but performing a lot of\nallocations for this goal. Now we always write directly to the stream, no\nintermediate allocation is performed.\n\nSigned-off-by: Pierre Habouzit \n---\n builtin-apply.c | 83 +++++--------\n builtin-blame.c | 3 +-\n builtin-check-attr.c | 2 +-\n builtin-checkout-index.c | 4 +-\n builtin-ls-files.c | 13 +--\n builtin-ls-tree.c | 6 +-\n combine-diff.c | 16 +--\n diff.c | 303 +++++++++++++++++-----------------------------\n quote.c | 198 +++++++++++++++++-------------\n quote.h | 8 +-\n 10 files changed, 268 insertions(+), 368 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex cffbe52..0328863 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -163,15 +163,14 @@ static void say_patch_name(FILE *output, const char *pre, struct patch *patch, c\n \tfputs(pre, output);\n \tif (patch->old_name && patch->new_name &&\n \t strcmp(patch->old_name, patch->new_name)) {\n-\t\twrite_name_quoted(NULL, 0, patch->old_name, 1, output);\n+\t\tquote_c_style(patch->old_name, NULL, output, 0);\n \t\tfputs(\" => \", output);\n-\t\twrite_name_quoted(NULL, 0, patch->new_name, 1, output);\n-\t}\n-\telse {\n+\t\tquote_c_style(patch->new_name, NULL, output, 0);\n+\t} else {\n \t\tconst char *n = patch->new_name;\n \t\tif (!n)\n \t\t\tn = patch->old_name;\n-\t\twrite_name_quoted(NULL, 0, n, 1, output);\n+\t\tquote_c_style(n, NULL, output, 0);\n \t}\n \tfputs(post, output);\n }\n@@ -1378,61 +1377,50 @@ static const char minuses[]= \"--------------------------------------------------\n \n static void show_stats(struct patch *patch)\n {\n-\tconst char *prefix = \"\";\n-\tchar *name = patch->new_name;\n-\tchar *qname = NULL;\n-\tint len, max, add, del, total;\n-\n-\tif (!name)\n-\t\tname = patch->old_name;\n+\tstruct strbuf qname;\n+\tchar *cp = patch->new_name ? patch->new_name : patch->old_name;\n+\tint max, add, del;\n \n-\tif (0 < (len = quote_c_style(name, NULL, NULL, 0))) {\n-\t\tqname = xmalloc(len + 1);\n-\t\tquote_c_style(name, qname, NULL, 0);\n-\t\tname = qname;\n-\t}\n+\tstrbuf_init(&qname, 0);\n+\tquote_c_style(cp, &qname, NULL, 0);\n \n \t/*\n \t * \"scale\" the filename\n \t */\n-\tlen = strlen(name);\n \tmax = max_len;\n \tif (max > 50)\n \t\tmax = 50;\n-\tif (len > max) {\n-\t\tchar *slash;\n-\t\tprefix = \"...\";\n-\t\tmax -= 3;\n-\t\tname += len - max;\n-\t\tslash = strchr(name, '/');\n-\t\tif (slash)\n-\t\t\tname = slash;\n+\n+\tif (qname.len > max) {\n+\t\tcp = strchr(qname.buf + qname.len + 3 - max, '/');\n+\t\tif (cp)\n+\t\t\tcp = qname.buf + qname.len + 3 - max;\n+\t\tstrbuf_splice(&qname, 0, cp - qname.buf, \"...\", 3);\n+\t}\n+\n+\tif (patch->is_binary) {\n+\t\tprintf(\" %-*s | Bin\\n\", max, qname.buf);\n+\t\tstrbuf_release(&qname);\n+\t\treturn;\n \t}\n-\tlen = max;\n+\n+\tprintf(\" %-*s |\", max, qname.buf);\n+\tstrbuf_release(&qname);\n \n \t/*\n \t * scale the add/delete\n \t */\n-\tmax = max_change;\n-\tif (max + len > 70)\n-\t\tmax = 70 - len;\n-\n+\tmax = max + max_change > 70 ? 70 - max : max_change;\n \tadd = patch->lines_added;\n \tdel = patch->lines_deleted;\n-\ttotal = add + del;\n \n \tif (max_change > 0) {\n-\t\ttotal = (total * max + max_change / 2) / max_change;\n+\t\tint total = ((add + del) * max + max_change / 2) / max_change;\n \t\tadd = (add * max + max_change / 2) / max_change;\n \t\tdel = total - add;\n \t}\n-\tif (patch->is_binary)\n-\t\tprintf(\" %s%-*s | Bin\\n\", prefix, len, name);\n-\telse\n-\t\tprintf(\" %s%-*s |%5d %.*s%.*s\\n\", prefix,\n-\t\t len, name, patch->lines_added + patch->lines_deleted,\n-\t\t add, pluses, del, minuses);\n-\tfree(qname);\n+\tprintf(\"%5d %.*s%.*s\\n\", patch->lines_added + patch->lines_deleted,\n+\t\tadd, pluses, del, minuses);\n }\n \n static int read_old_data(struct stat *st, const char *path, struct strbuf *buf)\n@@ -2197,11 +2185,7 @@ static void show_index_list(struct patch *list)\n \t\t\tsha1_ptr = sha1;\n \n \t\tprintf(\"%06o %s\t\",patch->old_mode, sha1_to_hex(sha1_ptr));\n-\t\tif (line_termination && quote_c_style(name, NULL, NULL, 0))\n-\t\t\tquote_c_style(name, NULL, stdout, 0);\n-\t\telse\n-\t\t\tfputs(name, stdout);\n-\t\tputchar(line_termination);\n+\t\twrite_name_quoted(name, stdout, line_termination);\n \t}\n }\n \n@@ -2227,13 +2211,8 @@ static void numstat_patch_list(struct patch *patch)\n \t\tif (patch->is_binary)\n \t\t\tprintf(\"-\\t-\\t\");\n \t\telse\n-\t\t\tprintf(\"%d\\t%d\\t\",\n-\t\t\t patch->lines_added, patch->lines_deleted);\n-\t\tif (line_termination && quote_c_style(name, NULL, NULL, 0))\n-\t\t\tquote_c_style(name, NULL, stdout, 0);\n-\t\telse\n-\t\t\tfputs(name, stdout);\n-\t\tputchar(line_termination);\n+\t\t\tprintf(\"%d\\t%d\\t\", patch->lines_added, patch->lines_deleted);\n+\t\twrite_name_quoted(name, stdout, line_termination);\n \t}\n }\n \ndiff --git a/builtin-blame.c b/builtin-blame.c\nindex e364b6c..16c0ca8 100644\n--- a/builtin-blame.c\n+++ b/builtin-blame.c\n@@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,\n static void write_filename_info(const char *path)\n {\n \tprintf(\"filename \");\n-\twrite_name_quoted(NULL, 0, path, 1, stdout);\n-\tputchar('\\n');\n+\twrite_name_quoted(path, stdout, '\\n');\n }\n \n /*\ndiff --git a/builtin-check-attr.c b/builtin-check-attr.c\nindex d949733..6afdfa1 100644\n--- a/builtin-check-attr.c\n+++ b/builtin-check-attr.c\n@@ -56,7 +56,7 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)\n \t\t\telse if (ATTR_UNSET(value))\n \t\t\t\tvalue = \"unspecified\";\n \n-\t\t\twrite_name_quoted(\"\", 0, argv[i], 1, stdout);\n+\t\t\tquote_c_style(argv[i], NULL, stdout, 0);\n \t\t\tprintf(\": %s: %s\\n\", argv[j+1], value);\n \t\t}\n \t}\ndiff --git a/builtin-checkout-index.c b/builtin-checkout-index.c\nindex e6264c4..70d619d 100644\n--- a/builtin-checkout-index.c\n+++ b/builtin-checkout-index.c\n@@ -66,9 +66,7 @@ static void write_tempfile_record(const char *name, int prefix_length)\n \t\tfputs(topath[checkout_stage], stdout);\n \n \tputchar('\\t');\n-\twrite_name_quoted(\"\", 0, name + prefix_length,\n-\t\tline_termination, stdout);\n-\tputchar(line_termination);\n+\twrite_name_quoted(name + prefix_length, stdout, line_termination);\n \n \tfor (i = 0; i < 4; i++) {\n \t\ttopath[i][0] = 0;\ndiff --git a/builtin-ls-files.c b/builtin-ls-files.c\nindex 48dd3f7..2e6f43b 100644\n--- a/builtin-ls-files.c\n+++ b/builtin-ls-files.c\n@@ -84,8 +84,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)\n \t\treturn;\n \n \tfputs(tag, stdout);\n-\twrite_name_quoted(\"\", 0, ent->name + offset, line_terminator, stdout);\n-\tputchar(line_terminator);\n+\twrite_name_quoted(ent->name + offset, stdout, line_terminator);\n }\n \n static void show_other_files(struct dir_struct *dir)\n@@ -208,21 +207,15 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)\n \n \tif (!show_stage) {\n \t\tfputs(tag, stdout);\n-\t\twrite_name_quoted(\"\", 0, ce->name + offset,\n-\t\t\t\t line_terminator, stdout);\n-\t\tputchar(line_terminator);\n-\t}\n-\telse {\n+\t} else {\n \t\tprintf(\"%s%06o %s %d\\t\",\n \t\t tag,\n \t\t ntohl(ce->ce_mode),\n \t\t abbrev ? find_unique_abbrev(ce->sha1,abbrev)\n \t\t\t\t: sha1_to_hex(ce->sha1),\n \t\t ce_stage(ce));\n-\t\twrite_name_quoted(\"\", 0, ce->name + offset,\n-\t\t\t\t line_terminator, stdout);\n-\t\tputchar(line_terminator);\n \t}\n+\twrite_name_quoted(ce->name + offset, stdout, line_terminator);\n }\n \n static void show_files(struct dir_struct *dir, const char *prefix)\ndiff --git a/builtin-ls-tree.c b/builtin-ls-tree.c\nindex cb4be4f..7abe333 100644\n--- a/builtin-ls-tree.c\n+++ b/builtin-ls-tree.c\n@@ -112,10 +112,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n \t\t\t abbrev ? find_unique_abbrev(sha1, abbrev)\n \t\t\t : sha1_to_hex(sha1));\n \t}\n-\twrite_name_quoted(base + chomp_prefix, baselen - chomp_prefix,\n-\t\t\t pathname,\n-\t\t\t line_termination, stdout);\n-\tputchar(line_termination);\n+\twrite_name_quotedpfx(base + chomp_prefix, baselen - chomp_prefix,\n+\t\t\t pathname, stdout, line_termination);\n \treturn retval;\n }\n \ndiff --git a/combine-diff.c b/combine-diff.c\nindex ef62234..fe5a2a1 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -650,10 +650,7 @@ static void dump_quoted_path(const char *prefix, const char *path,\n \t\t\t const char *c_meta, const char *c_reset)\n {\n \tprintf(\"%s%s\", c_meta, prefix);\n-\tif (quote_c_style(path, NULL, NULL, 0))\n-\t\tquote_c_style(path, NULL, stdout, 0);\n-\telse\n-\t\tprintf(\"%s\", path);\n+\tquote_c_style(path, NULL, stdout, 0);\n \tprintf(\"%s\\n\", c_reset);\n }\n \n@@ -900,16 +897,7 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \t\tputchar(inter_name_termination);\n \t}\n \n-\tif (line_termination) {\n-\t\tif (quote_c_style(p->path, NULL, NULL, 0))\n-\t\t\tquote_c_style(p->path, NULL, stdout, 0);\n-\t\telse\n-\t\t\tprintf(\"%s\", p->path);\n-\t\tputchar(line_termination);\n-\t}\n-\telse {\n-\t\tprintf(\"%s%c\", p->path, line_termination);\n-\t}\n+\twrite_name_quoted(p->path, stdout, line_termination);\n }\n \n void show_combined_diff(struct combine_diff_path *p,\ndiff --git a/diff.c b/diff.c\nindex 2216d75..fb6d077 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -181,44 +181,23 @@ int git_diff_ui_config(const char *var, const char *value)\n \treturn git_default_config(var, value);\n }\n \n-static char *quote_one(const char *str)\n-{\n-\tint needlen;\n-\tchar *xp;\n-\n-\tif (!str)\n-\t\treturn NULL;\n-\tneedlen = quote_c_style(str, NULL, NULL, 0);\n-\tif (!needlen)\n-\t\treturn xstrdup(str);\n-\txp = xmalloc(needlen + 1);\n-\tquote_c_style(str, xp, NULL, 0);\n-\treturn xp;\n-}\n-\n static char *quote_two(const char *one, const char *two)\n {\n \tint need_one = quote_c_style(one, NULL, NULL, 1);\n \tint need_two = quote_c_style(two, NULL, NULL, 1);\n-\tchar *xp;\n+\tstruct strbuf res;\n \n+\tstrbuf_init(&res, 0);\n \tif (need_one + need_two) {\n-\t\tif (!need_one) need_one = strlen(one);\n-\t\tif (!need_two) need_one = strlen(two);\n-\n-\t\txp = xmalloc(need_one + need_two + 3);\n-\t\txp[0] = '\"';\n-\t\tquote_c_style(one, xp + 1, NULL, 1);\n-\t\tquote_c_style(two, xp + need_one + 1, NULL, 1);\n-\t\tstrcpy(xp + need_one + need_two + 1, \"\\\"\");\n-\t\treturn xp;\n+\t\tstrbuf_addch(&res, '\"');\n+\t\tquote_c_style(one, &res, NULL, 1);\n+\t\tquote_c_style(two, &res, NULL, 1);\n+\t\tstrbuf_addch(&res, '\"');\n+\t} else {\n+\t\tstrbuf_addstr(&res, one);\n+\t\tstrbuf_addstr(&res, two);\n \t}\n-\tneed_one = strlen(one);\n-\tneed_two = strlen(two);\n-\txp = xmalloc(need_one + need_two + 1);\n-\tstrcpy(xp, one);\n-\tstrcpy(xp + need_one, two);\n-\treturn xp;\n+\treturn res.buf;\n }\n \n static const char *external_diff(void)\n@@ -670,27 +649,20 @@ static char *pprint_rename(const char *a, const char *b)\n {\n \tconst char *old = a;\n \tconst char *new = b;\n-\tchar *name = NULL;\n+\tstruct strbuf name;\n \tint pfx_length, sfx_length;\n \tint len_a = strlen(a);\n \tint len_b = strlen(b);\n+\tint a_midlen, b_midlen;\n \tint qlen_a = quote_c_style(a, NULL, NULL, 0);\n \tint qlen_b = quote_c_style(b, NULL, NULL, 0);\n \n+\tstrbuf_init(&name, 0);\n \tif (qlen_a || qlen_b) {\n-\t\tif (qlen_a) len_a = qlen_a;\n-\t\tif (qlen_b) len_b = qlen_b;\n-\t\tname = xmalloc( len_a + len_b + 5 );\n-\t\tif (qlen_a)\n-\t\t\tquote_c_style(a, name, NULL, 0);\n-\t\telse\n-\t\t\tmemcpy(name, a, len_a);\n-\t\tmemcpy(name + len_a, \" => \", 4);\n-\t\tif (qlen_b)\n-\t\t\tquote_c_style(b, name + len_a + 4, NULL, 0);\n-\t\telse\n-\t\t\tmemcpy(name + len_a + 4, b, len_b + 1);\n-\t\treturn name;\n+\t\tquote_c_style(a, &name, NULL, 0);\n+\t\tstrbuf_addstr(&name, \" => \");\n+\t\tquote_c_style(b, &name, NULL, 0);\n+\t\treturn name.buf;\n \t}\n \n \t/* Find common prefix */\n@@ -719,24 +691,26 @@ static char *pprint_rename(const char *a, const char *b)\n \t * pfx{sfx-a => sfx-b}\n \t * name-a => name-b\n \t */\n+\ta_midlen = len_a - pfx_length - sfx_length;\n+\tb_midlen = len_b - pfx_length - sfx_length;\n+\tif (a_midlen < 0)\n+\t\ta_midlen = 0;\n+\tif (b_midlen < 0)\n+\t\tb_midlen = 0;\n+\n+\tstrbuf_grow(&name, pfx_length + a_midlen + b_midlen + sfx_length + 7);\n \tif (pfx_length + sfx_length) {\n-\t\tint a_midlen = len_a - pfx_length - sfx_length;\n-\t\tint b_midlen = len_b - pfx_length - sfx_length;\n-\t\tif (a_midlen < 0) a_midlen = 0;\n-\t\tif (b_midlen < 0) b_midlen = 0;\n-\n-\t\tname = xmalloc(pfx_length + a_midlen + b_midlen + sfx_length + 7);\n-\t\tsprintf(name, \"%.*s{%.*s => %.*s}%s\",\n-\t\t\tpfx_length, a,\n-\t\t\ta_midlen, a + pfx_length,\n-\t\t\tb_midlen, b + pfx_length,\n-\t\t\ta + len_a - sfx_length);\n+\t\tstrbuf_add(&name, a, pfx_length);\n+\t\tstrbuf_addch(&name, '{');\n \t}\n-\telse {\n-\t\tname = xmalloc(len_a + len_b + 5);\n-\t\tsprintf(name, \"%s => %s\", a, b);\n+\tstrbuf_add(&name, a + pfx_length, a_midlen);\n+\tstrbuf_addstr(&name, \" => \");\n+\tstrbuf_add(&name, b + pfx_length, b_midlen);\n+\tif (pfx_length + sfx_length) {\n+\t\tstrbuf_addch(&name, '}');\n+\t\tstrbuf_add(&name, a + len_a - sfx_length, sfx_length);\n \t}\n-\treturn name;\n+\treturn name.buf;\n }\n \n struct diffstat_t {\n@@ -849,12 +823,13 @@ static void show_stats(struct diffstat_t* data, struct diff_options *options)\n \t\tint change = file->added + file->deleted;\n \n \t\tif (!file->is_renamed) { /* renames are already quoted by pprint_rename */\n-\t\t\tlen = quote_c_style(file->name, NULL, NULL, 0);\n-\t\t\tif (len) {\n-\t\t\t\tchar *qname = xmalloc(len + 1);\n-\t\t\t\tquote_c_style(file->name, qname, NULL, 0);\n+\t\t\tstruct strbuf buf;\n+\t\t\tstrbuf_init(&buf, 0);\n+\t\t\tif (quote_c_style(file->name, &buf, NULL, 0)) {\n \t\t\t\tfree(file->name);\n-\t\t\t\tfile->name = qname;\n+\t\t\t\tfile->name = buf.buf;\n+\t\t\t} else {\n+\t\t\t\tstrbuf_release(&buf);\n \t\t\t}\n \t\t}\n \n@@ -992,12 +967,12 @@ static void show_numstat(struct diffstat_t* data, struct diff_options *options)\n \t\t\tprintf(\"-\\t-\\t\");\n \t\telse\n \t\t\tprintf(\"%d\\t%d\\t\", file->added, file->deleted);\n-\t\tif (options->line_termination && !file->is_renamed &&\n-\t\t quote_c_style(file->name, NULL, NULL, 0))\n-\t\t\tquote_c_style(file->name, NULL, stdout, 0);\n-\t\telse\n+\t\tif (!file->is_renamed) {\n+\t\t\twrite_name_quoted(file->name, stdout, options->line_termination);\n+\t\t} else {\n \t\t\tfputs(file->name, stdout);\n-\t\tputchar(options->line_termination);\n+\t\t\tputchar(options->line_termination);\n+\t\t}\n \t}\n }\n \n@@ -1941,50 +1916,46 @@ static int similarity_index(struct diff_filepair *p)\n static void run_diff(struct diff_filepair *p, struct diff_options *o)\n {\n \tconst char *pgm = external_diff();\n-\tchar msg[PATH_MAX*2+300], *xfrm_msg;\n-\tstruct diff_filespec *one;\n-\tstruct diff_filespec *two;\n+\tstruct strbuf msg;\n+\tchar *xfrm_msg;\n+\tstruct diff_filespec *one = p->one;\n+\tstruct diff_filespec *two = p->two;\n \tconst char *name;\n \tconst char *other;\n-\tchar *name_munged, *other_munged;\n \tint complete_rewrite = 0;\n-\tint len;\n+\n \n \tif (DIFF_PAIR_UNMERGED(p)) {\n-\t\t/* unmerged */\n \t\trun_diff_cmd(pgm, p->one->path, NULL, NULL, NULL, NULL, o, 0);\n \t\treturn;\n \t}\n \n-\tname = p->one->path;\n+\tname = p->one->path;\n \tother = (strcmp(name, p->two->path) ? p->two->path : NULL);\n-\tname_munged = quote_one(name);\n-\tother_munged = quote_one(other);\n-\tone = p->one; two = p->two;\n-\n \tdiff_fill_sha1_info(one);\n \tdiff_fill_sha1_info(two);\n \n-\tlen = 0;\n+\tstrbuf_init(&msg, PATH_MAX * 2 + 300);\n \tswitch (p->status) {\n \tcase DIFF_STATUS_COPIED:\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\"similarity index %d%%\\n\"\n-\t\t\t\t\"copy from %s\\n\"\n-\t\t\t\t\"copy to %s\\n\",\n-\t\t\t\tsimilarity_index(p), name_munged, other_munged);\n+\t\tstrbuf_addf(&msg, \"similarity index %d%%\", similarity_index(p));\n+\t\tstrbuf_addstr(&msg, \"\\ncopy from \");\n+\t\tquote_c_style(name, &msg, NULL, 0);\n+\t\tstrbuf_addstr(&msg, \"\\ncopy to \");\n+\t\tquote_c_style(other, &msg, NULL, 0);\n+\t\tstrbuf_addch(&msg, '\\n');\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\"similarity index %d%%\\n\"\n-\t\t\t\t\"rename from %s\\n\"\n-\t\t\t\t\"rename to %s\\n\",\n-\t\t\t\tsimilarity_index(p), name_munged, other_munged);\n+\t\tstrbuf_addf(&msg, \"similarity index %d%%\", similarity_index(p));\n+\t\tstrbuf_addstr(&msg, \"\\nrename from \");\n+\t\tquote_c_style(name, &msg, NULL, 0);\n+\t\tstrbuf_addstr(&msg, \"\\nrename to \");\n+\t\tquote_c_style(other, &msg, NULL, 0);\n+\t\tstrbuf_addch(&msg, '\\n');\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n \t\tif (p->score) {\n-\t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\t\"dissimilarity index %d%%\\n\",\n+\t\t\tstrbuf_addf(&msg, \"dissimilarity index %d%%\\n\",\n \t\t\t\t\tsimilarity_index(p));\n \t\t\tcomplete_rewrite = 1;\n \t\t\tbreak;\n@@ -2004,19 +1975,17 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \t\t\t (!fill_mmfile(&mf, two) && diff_filespec_is_binary(two)))\n \t\t\t\tabbrev = 40;\n \t\t}\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\"index %.*s..%.*s\",\n+\t\tstrbuf_addf(&msg, \"index %.*s..%.*s\",\n \t\t\t\tabbrev, sha1_to_hex(one->sha1),\n \t\t\t\tabbrev, sha1_to_hex(two->sha1));\n \t\tif (one->mode == two->mode)\n-\t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\t\" %06o\", one->mode);\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len, \"\\n\");\n+\t\t\tstrbuf_addf(&msg, \" %06o\", one->mode);\n+\t\tstrbuf_addch(&msg, '\\n');\n \t}\n \n-\tif (len)\n-\t\tmsg[--len] = 0;\n-\txfrm_msg = len ? msg : NULL;\n+\tif (msg.len)\n+\t\tstrbuf_setlen(&msg, msg.len - 1);\n+\txfrm_msg = msg.len ? msg.buf : NULL;\n \n \tif (!pgm &&\n \t DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&\n@@ -2035,8 +2004,7 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \t\trun_diff_cmd(pgm, name, other, one, two, xfrm_msg, o,\n \t\t\t complete_rewrite);\n \n-\tfree(name_munged);\n-\tfree(other_munged);\n+\tstrbuf_release(&msg);\n }\n \n static void run_diffstat(struct diff_filepair *p, struct diff_options *o,\n@@ -2492,72 +2460,30 @@ const char *diff_unique_abbrev(const unsigned char *sha1, int len)\n \treturn sha1_to_hex(sha1);\n }\n \n-static void diff_flush_raw(struct diff_filepair *p,\n-\t\t\t struct diff_options *options)\n+static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)\n {\n-\tint two_paths;\n-\tchar status[10];\n-\tint abbrev = options->abbrev;\n-\tconst char *path_one, *path_two;\n-\tint inter_name_termination = '\\t';\n-\tint line_termination = options->line_termination;\n-\n-\tif (!line_termination)\n-\t\tinter_name_termination = 0;\n+\tint line_termination = opt->line_termination;\n+\tint inter_name_termination = line_termination ? '\\t' : '\\0';\n \n-\tpath_one = p->one->path;\n-\tpath_two = p->two->path;\n-\tif (line_termination) {\n-\t\tpath_one = quote_one(path_one);\n-\t\tpath_two = quote_one(path_two);\n+\tif (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {\n+\t\tprintf(\":%06o %06o %s \", p->one->mode, p->two->mode,\n+\t\t diff_unique_abbrev(p->one->sha1, opt->abbrev));\n+\t\tprintf(\"%s \", diff_unique_abbrev(p->two->sha1, opt->abbrev));\n \t}\n-\n-\tif (p->score)\n-\t\tsprintf(status, \"%c%03d\", p->status, similarity_index(p));\n-\telse {\n-\t\tstatus[0] = p->status;\n-\t\tstatus[1] = 0;\n-\t}\n-\tswitch (p->status) {\n-\tcase DIFF_STATUS_COPIED:\n-\tcase DIFF_STATUS_RENAMED:\n-\t\ttwo_paths = 1;\n-\t\tbreak;\n-\tcase DIFF_STATUS_ADDED:\n-\tcase DIFF_STATUS_DELETED:\n-\t\ttwo_paths = 0;\n-\t\tbreak;\n-\tdefault:\n-\t\ttwo_paths = 0;\n-\t\tbreak;\n-\t}\n-\tif (!(options->output_format & DIFF_FORMAT_NAME_STATUS)) {\n-\t\tprintf(\":%06o %06o %s \",\n-\t\t p->one->mode, p->two->mode,\n-\t\t diff_unique_abbrev(p->one->sha1, abbrev));\n-\t\tprintf(\"%s \",\n-\t\t diff_unique_abbrev(p->two->sha1, abbrev));\n+\tif (p->score) {\n+\t\tprintf(\"%c%03d%c\", p->status, similarity_index(p),\n+\t\t\t inter_name_termination);\n+\t} else {\n+\t\tprintf(\"%c%c\", p->status, inter_name_termination);\n \t}\n-\tprintf(\"%s%c%s\", status, inter_name_termination,\n-\t\t\ttwo_paths || p->one->mode ? path_one : path_two);\n-\tif (two_paths)\n-\t\tprintf(\"%c%s\", inter_name_termination, path_two);\n-\tputchar(line_termination);\n-\tif (path_one != p->one->path)\n-\t\tfree((void*)path_one);\n-\tif (path_two != p->two->path)\n-\t\tfree((void*)path_two);\n-}\n \n-static void diff_flush_name(struct diff_filepair *p, struct diff_options *opt)\n-{\n-\tchar *path = p->two->path;\n-\n-\tif (opt->line_termination)\n-\t\tpath = quote_one(p->two->path);\n-\tprintf(\"%s%c\", path, opt->line_termination);\n-\tif (p->two->path != path)\n-\t\tfree(path);\n+\tif (p->status == DIFF_STATUS_COPIED || p->status == DIFF_STATUS_RENAMED) {\n+\t\twrite_name_quoted(p->one->path, stdout, inter_name_termination);\n+\t\twrite_name_quoted(p->two->path, stdout, line_termination);\n+\t} else {\n+\t\tconst char *path = p->one->mode ? p->one->path : p->two->path;\n+\t\twrite_name_quoted(path, stdout, line_termination);\n+\t}\n }\n \n int diff_unmodified_pair(struct diff_filepair *p)\n@@ -2567,14 +2493,11 @@ int diff_unmodified_pair(struct diff_filepair *p)\n \t * let transformers to produce diff_filepairs any way they want,\n \t * and filter and clean them up here before producing the output.\n \t */\n-\tstruct diff_filespec *one, *two;\n+\tstruct diff_filespec *one = p->one, *two = p->two;\n \n \tif (DIFF_PAIR_UNMERGED(p))\n \t\treturn 0; /* unmerged is interesting */\n \n-\tone = p->one;\n-\ttwo = p->two;\n-\n \t/* deletion, addition, mode or type change\n \t * and rename are all interesting.\n \t */\n@@ -2763,32 +2686,27 @@ static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)\n \telse if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))\n \t\tdiff_flush_raw(p, opt);\n \telse if (fmt & DIFF_FORMAT_NAME)\n-\t\tdiff_flush_name(p, opt);\n+\t\twrite_name_quoted(p->two->path, stdout, opt->line_termination);\n }\n \n static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)\n {\n-\tchar *name = quote_one(fs->path);\n \tif (fs->mode)\n-\t\tprintf(\" %s mode %06o %s\\n\", newdelete, fs->mode, name);\n+\t\tprintf(\" %s mode %06o \", newdelete, fs->mode);\n \telse\n-\t\tprintf(\" %s %s\\n\", newdelete, name);\n-\tfree(name);\n+\t\tprintf(\" %s \", newdelete);\n+\twrite_name_quoted(fs->path, stdout, '\\n');\n }\n \n \n static void show_mode_change(struct diff_filepair *p, int show_name)\n {\n \tif (p->one->mode && p->two->mode && p->one->mode != p->two->mode) {\n+\t\tprintf(\" mode change %06o => %06o%c\", p->one->mode, p->two->mode,\n+\t\t\tshow_name ? ' ' : '\\n');\n \t\tif (show_name) {\n-\t\t\tchar *name = quote_one(p->two->path);\n-\t\t\tprintf(\" mode change %06o => %06o %s\\n\",\n-\t\t\t p->one->mode, p->two->mode, name);\n-\t\t\tfree(name);\n+\t\t\twrite_name_quoted(p->two->path, stdout, '\\n');\n \t\t}\n-\t\telse\n-\t\t\tprintf(\" mode change %06o => %06o\\n\",\n-\t\t\t p->one->mode, p->two->mode);\n \t}\n }\n \n@@ -2818,12 +2736,11 @@ static void diff_summary(struct diff_filepair *p)\n \t\tbreak;\n \tdefault:\n \t\tif (p->score) {\n-\t\t\tchar *name = quote_one(p->two->path);\n-\t\t\tprintf(\" rewrite %s (%d%%)\\n\", name,\n-\t\t\t similarity_index(p));\n-\t\t\tfree(name);\n-\t\t\tshow_mode_change(p, 0);\n-\t\t} else\tshow_mode_change(p, 1);\n+\t\t\tputs(\" rewrite \");\n+\t\t\twrite_name_quoted(p->two->path, stdout, ' ');\n+\t\t\tprintf(\"(%d%%)\\n\", similarity_index(p));\n+\t\t}\n+\t\tshow_mode_change(p, !p->score);\n \t\tbreak;\n \t}\n }\n@@ -2837,14 +2754,14 @@ struct patch_id_t {\n static int remove_space(char *line, int len)\n {\n \tint i;\n- char *dst = line;\n- unsigned char c;\n+\tchar *dst = line;\n+\tunsigned char c;\n \n- for (i = 0; i < len; i++)\n- if (!isspace((c = line[i])))\n- *dst++ = c;\n+\tfor (i = 0; i < len; i++)\n+\t\tif (!isspace((c = line[i])))\n+\t\t\t*dst++ = c;\n \n- return dst - line;\n+\treturn dst - line;\n }\n \n static void patch_id_consume(void *priv, char *line, unsigned long len)\ndiff --git a/quote.c b/quote.c\nindex 67c6527..a8a755a 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -114,83 +114,142 @@ char *sq_dequote(char *arg)\n \t}\n }\n \n+/* 1 means: quote as octal\n+ * 0 means: quote as octal if (quote_path_fully)\n+ * -1 means: never quote\n+ * c: quote as \"\\\\c\"\n+ */\n+#define X8(x) x, x, x, x, x, x, x, x\n+#define X16(x) X8(x), X8(x)\n+static signed char const sq_lookup[256] = {\n+\t/* 0 1 2 3 4 5 6 7 */\n+\t/* 0x00 */ 1, 1, 1, 1, 1, 1, 'a', 1,\n+\t/* 0x08 */ 'b', 't', 'n', 'v', 'f', 'r', 1, 1,\n+\t/* 0x10 */ X16(1),\n+\t/* 0x20 */ -1, -1, '\"', -1, -1, -1, -1, -1,\n+\t/* 0x28 */ X16(-1), X16(-1), X16(-1),\n+\t/* 0x58 */ -1, -1, -1, -1,'\\\\', -1, -1, -1,\n+\t/* 0x60 */ X16(-1), X16(-1),\n+\t/* 0x80 */ /* set to 0 */\n+};\n+\n+static inline int sq_must_quote(char c) {\n+\treturn sq_lookup[(unsigned char)c] + quote_path_fully > 0;\n+}\n+\n+/* returns the longest prefix not needing a quote up to maxlen if positive.\n+ This stops at the first \\0 because it's marked as a character needing an\n+ escape */\n+static size_t next_quote_pos(const char *s, ssize_t maxlen)\n+{\n+\tsize_t len;\n+\tif (maxlen < 0) {\n+\t\tfor (len = 0; !sq_must_quote(s[len]); len++);\n+\t} else {\n+\t\tfor (len = 0; len < maxlen && !sq_must_quote(s[len]); len++);\n+\t}\n+\treturn len;\n+}\n+\n /*\n * C-style name quoting.\n *\n- * Does one of three things:\n- *\n * (1) if outbuf and outfp are both NULL, inspect the input name and\n * counts the number of bytes that are needed to hold c_style\n * quoted version of name, counting the double quotes around\n * it but not terminating NUL, and returns it. However, if name\n * does not need c_style quoting, it returns 0.\n *\n- * (2) if outbuf is not NULL, it must point at a buffer large enough\n- * to hold the c_style quoted version of name, enclosing double\n- * quotes, and terminating NUL. Fills outbuf with c_style quoted\n- * version of name enclosed in double-quote pair. Return value\n- * is undefined.\n- *\n- * (3) if outfp is not NULL, outputs c_style quoted version of name,\n- * but not enclosed in double-quote pair. Return value is undefined.\n+ * (2) if sb or fp are not NULL, it emits the c_style quoted version\n+ * of name, enclosed with double quotes if asked and needed only.\n+ * Return value is the same as in (1).\n */\n-\n-static int quote_c_style_counted(const char *name, int namelen,\n-\t\t\t\t char *outbuf, FILE *outfp, int no_dq)\n+static size_t quote_c_style_counted(const char *name, ssize_t maxlen,\n+ struct strbuf *sb, FILE *fp, int no_dq)\n {\n #undef EMIT\n-#define EMIT(c) \\\n-\t(outbuf ? (*outbuf++ = (c)) : outfp ? fputc(c, outfp) : (count++))\n+#define EMIT(c) \\\n+\tdo { \\\n+\t\tif (sb) strbuf_addch(sb, (c)); \\\n+\t\tif (fp) fputc((c), fp); \\\n+\t\tcount++; \\\n+\t} while (0)\n+#define EMITBUF(s, l) \\\n+\tdo { \\\n+\t\tif (sb) strbuf_add(sb, (s), (l)); \\\n+\t\tif (fp) fwrite((s), (l), 1, fp); \\\n+\t\tcount += (l); \\\n+\t} while (0)\n+\n+\tsize_t len, count = 0;\n+\tconst char *p = name;\n \n-#define EMITQ() EMIT('\\\\')\n-\n-\tconst char *sp;\n-\tunsigned char ch;\n-\tint count = 0, needquote = 0;\n+\tfor (;;) {\n+\t\tint ch;\n \n-\tif (!no_dq)\n-\t\tEMIT('\"');\n-\tfor (sp = name; sp < name + namelen; sp++) {\n-\t\tch = *sp;\n-\t\tif (!ch)\n+\t\tlen = next_quote_pos(p, maxlen);\n+\t\tif (len == maxlen || !p[len])\n \t\t\tbreak;\n-\t\tif ((ch < ' ') || (ch == '\"') || (ch == '\\\\') ||\n-\t\t (quote_path_fully && (ch >= 0177))) {\n-\t\t\tneedquote = 1;\n-\t\t\tswitch (ch) {\n-\t\t\tcase '\\a': EMITQ(); ch = 'a'; break;\n-\t\t\tcase '\\b': EMITQ(); ch = 'b'; break;\n-\t\t\tcase '\\f': EMITQ(); ch = 'f'; break;\n-\t\t\tcase '\\n': EMITQ(); ch = 'n'; break;\n-\t\t\tcase '\\r': EMITQ(); ch = 'r'; break;\n-\t\t\tcase '\\t': EMITQ(); ch = 't'; break;\n-\t\t\tcase '\\v': EMITQ(); ch = 'v'; break;\n-\n-\t\t\tcase '\\\\': /* fallthru */\n-\t\t\tcase '\"': EMITQ(); break;\n-\t\t\tdefault:\n-\t\t\t\t/* octal */\n-\t\t\t\tEMITQ();\n-\t\t\t\tEMIT(((ch >> 6) & 03) + '0');\n-\t\t\t\tEMIT(((ch >> 3) & 07) + '0');\n-\t\t\t\tch = (ch & 07) + '0';\n-\t\t\t\tbreak;\n-\t\t\t}\n+\n+\t\tif (!no_dq && p == name)\n+\t\t\tEMIT('\"');\n+\n+\t\tEMITBUF(p, len);\n+\t\tEMIT('\\\\');\n+\t\tp += len;\n+\t\tch = (unsigned char)*p++;\n+\t\tif (sq_lookup[ch] >= ' ') {\n+\t\t\tEMIT(sq_lookup[ch]);\n+\t\t} else {\n+\t\t\tEMIT(((ch >> 6) & 03) + '0');\n+\t\t\tEMIT(((ch >> 3) & 07) + '0');\n+\t\t\tEMIT(((ch >> 0) & 07) + '0');\n \t\t}\n-\t\tEMIT(ch);\n \t}\n+\n+\tEMITBUF(p, len);\n+\tif (p == name) /* no ending quote needed */\n+\t\treturn 0;\n+\n \tif (!no_dq)\n \t\tEMIT('\"');\n-\tif (outbuf)\n-\t\t*outbuf = 0;\n+\treturn count;\n+}\n \n-\treturn needquote ? count : 0;\n+size_t quote_c_style(const char *name, struct strbuf *sb, FILE *fp, int nodq)\n+{\n+\treturn quote_c_style_counted(name, -1, sb, fp, nodq);\n }\n \n-int quote_c_style(const char *name, char *outbuf, FILE *outfp, int no_dq)\n+void write_name_quoted(const char *name, FILE *fp, int terminator)\n {\n-\tint cnt = strlen(name);\n-\treturn quote_c_style_counted(name, cnt, outbuf, outfp, no_dq);\n+\tif (terminator) {\n+\t\tquote_c_style(name, NULL, fp, 0);\n+\t} else {\n+\t\tfputs(name, fp);\n+\t}\n+\tfputc(terminator, fp);\n+}\n+\n+extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n+ const char *name, FILE *fp, int terminator)\n+{\n+\tint needquote = 0;\n+\n+\tif (terminator) {\n+\t\tneedquote = next_quote_pos(pfx, pfxlen) < pfxlen\n+\t\t\t|| name[next_quote_pos(name, -1)];\n+\t}\n+\tif (needquote) {\n+\t\tfputc('\"', fp);\n+\t\tquote_c_style_counted(pfx, pfxlen, NULL, fp, 1);\n+\t\tquote_c_style(name, NULL, fp, 1);\n+\t\tfputc('\"', fp);\n+\t} else {\n+\t\tfwrite(pfx, pfxlen, 1, fp);\n+\t\tfputs(name, fp);\n+\t}\n+\tfputc(terminator, fp);\n }\n \n /*\n@@ -259,37 +318,6 @@ int unquote_c_style(struct strbuf *sb, const char *quoted, const char **endp)\n \treturn -1;\n }\n \n-void write_name_quoted(const char *prefix, int prefix_len,\n-\t\t const char *name, int quote, FILE *out)\n-{\n-\tint needquote;\n-\n-\tif (!quote) {\n-\tno_quote:\n-\t\tif (prefix_len)\n-\t\t\tfprintf(out, \"%.*s\", prefix_len, prefix);\n-\t\tfputs(name, out);\n-\t\treturn;\n-\t}\n-\n-\tneedquote = 0;\n-\tif (prefix_len)\n-\t\tneedquote = quote_c_style_counted(prefix, prefix_len,\n-\t\t\t\t\t\t NULL, NULL, 0);\n-\tif (!needquote)\n-\t\tneedquote = quote_c_style(name, NULL, NULL, 0);\n-\tif (needquote) {\n-\t\tfputc('\"', out);\n-\t\tif (prefix_len)\n-\t\t\tquote_c_style_counted(prefix, prefix_len,\n-\t\t\t\t\t NULL, out, 1);\n-\t\tquote_c_style(name, NULL, out, 1);\n-\t\tfputc('\"', out);\n-\t}\n-\telse\n-\t\tgoto no_quote;\n-}\n-\n /* quoting as a string literal for other languages */\n \n void perl_quote_print(FILE *stream, const char *src)\ndiff --git a/quote.h b/quote.h\nindex 6407c4d..4287990 100644\n--- a/quote.h\n+++ b/quote.h\n@@ -41,11 +41,11 @@ extern void sq_quote_argv(struct strbuf *, const char **argv, int count,\n extern char *sq_dequote(char *);\n \n extern int unquote_c_style(struct strbuf *, const char *quoted, const char **endp);\n-extern int quote_c_style(const char *name, char *outbuf, FILE *outfp,\n-\t\t\t int nodq);\n+extern size_t quote_c_style(const char *name, struct strbuf *, FILE *, int no_dq);\n \n-extern void write_name_quoted(const char *prefix, int prefix_len,\n-\t\t\t const char *name, int quote, FILE *out);\n+extern void write_name_quoted(const char *name, FILE *, int terminator);\n+extern void write_name_quotedpfx(const char *pfx, size_t pfxlen,\n+ const char *name, FILE *, int terminator);\n \n /* quoting as a string literal for other languages */\n extern void perl_quote_print(FILE *stream, const char *src);\n-- \n1.5.3.1\n"},{"id":"53516","messageId":"20070918223947.GB4535@artemis.corp","threadId":"9932","inReplyTo":null,"subject":"let's refactor quoting ...","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-18T22:39:47Z","receivedAt":"2007-09-18T22:39:47Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":" Here comes a series dedicated to quote. I just can't resist the simple\npleasure to show:\n\n$ git diff --shortstat HEAD~5.. ^strbuf*; git diff --shortstat HEAD~5.. strbuf*\n 19 files changed, 523 insertions(+), 716 deletions(-)\n 2 files changed, 41 insertions(+), 16 deletions(-)\n\n So it's an overall ~200 sloc reductions for a gain of 30 lines in\nthe strbuf module.\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53532","messageId":"20070919000757.GC4535@artemis.corp","threadId":"9932","inReplyTo":"20070918224122.2B55D344AB3@madism.org","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T00:07:57Z","receivedAt":"2007-09-19T00:07:57Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Sep 18, 2007 at 10:00:51PM +0000, Pierre Habouzit wrote:\n> +\t\tcp = strchr(qname.buf + qname.len + 3 - max, '/');\n> +\t\tif (cp)\n> +\t\t\tcp = qname.buf + qname.len + 3 - max;\n\n OMG, this is supposed to be if (!cp) of course...\n\n I wonder how this passed the testsuite.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53533","messageId":"20070919001457.GD4535@artemis.corp","threadId":"9932","inReplyTo":"20070918224121.24C3B344AB3@madism.org","subject":"Re: [PATCH 3/5] Rework unquote_c_style to work on a strbuf.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T00:14:57Z","receivedAt":"2007-09-19T00:14:57Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Tue, Sep 18, 2007 at 09:22:47PM +0000, Pierre Habouzit wrote:\n> +\t\t\tfor (cp = name.buf; p_value; p_value--) {\n> +\t\t\t\tcp = strchr(name.buf, '/');\n\n And I forgot to fix this, as cp = strchr(cp...) like the patch I\nproposed for integration in maint.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53540","messageId":"7vabrjtqw7.fsf@gitster.siamese.dyndns.org","threadId":"9932","inReplyTo":"20070919000757.GC4535@artemis.corp","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-19T00:55:52Z","receivedAt":"2007-09-19T00:55:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> On Tue, Sep 18, 2007 at 10:00:51PM +0000, Pierre Habouzit wrote:\n>> +\t\tcp = strchr(qname.buf + qname.len + 3 - max, '/');\n>> +\t\tif (cp)\n>> +\t\t\tcp = qname.buf + qname.len + 3 - max;\n>\n> OMG, this is supposed to be if (!cp) of course...\n>\n> I wonder how this passed the testsuite.\n\nYou would need a new test, I guess, before a huge rewrite.\n"},{"id":"53542","messageId":"20070919011433.GE4535@artemis.corp","threadId":"9932","inReplyTo":"7vabrjtqw7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T01:14:33Z","receivedAt":"2007-09-19T01:14:33Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 12:55:52AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > On Tue, Sep 18, 2007 at 10:00:51PM +0000, Pierre Habouzit wrote:\n> >> +\t\tcp = strchr(qname.buf + qname.len + 3 - max, '/');\n> >> +\t\tif (cp)\n> >> +\t\t\tcp = qname.buf + qname.len + 3 - max;\n> >\n> > OMG, this is supposed to be if (!cp) of course...\n> >\n> > I wonder how this passed the testsuite.\n> \n> You would need a new test, I guess, before a huge rewrite.\n\n OTOH this is in the code that generates the diffstats, it's not _that_\nsurprising that we don't have extensive tests about that, as it's not\ncritical in git afaict ;)\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53552","messageId":"46F0C3AB.8010801@op5.se","threadId":"9932","inReplyTo":"20070918224122.2B55D344AB3@madism.org","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-09-19T06:37:31Z","receivedAt":"2007-09-19T06:37:31Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Pierre Habouzit wrote:\n> \n> diff --git a/builtin-blame.c b/builtin-blame.c\n> index e364b6c..16c0ca8 100644\n> --- a/builtin-blame.c\n> +++ b/builtin-blame.c\n> @@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,\n> static void write_filename_info(const char *path)\n> {\n> \tprintf(\"filename \");\n> -\twrite_name_quoted(NULL, 0, path, 1, stdout);\n> -\tputchar('\\n');\n> +\twrite_name_quoted(path, stdout, '\\n');\n> }\n> \n\nThis looks like a candidate for a macro. I'm not sure if gcc optimizes\nsibling calls in void functions with -O2, and it doesn't inline without\n-O3.\n\n> \n> -static void diff_flush_raw(struct diff_filepair *p,\n> -\t\t\t struct diff_options *options)\n> +static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)\n\nParameter rename? I'd have thought the patch was big enough as it is ;-)\n\n\nOther than that, the diffstat calls this a good patch, and given the fact that\nall your previous series passed all tests, I assume this one does too.\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"53554","messageId":"20070919080030.GA28205@artemis.corp","threadId":"9932","inReplyTo":"46F0C3AB.8010801@op5.se","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T08:00:30Z","receivedAt":"2007-09-19T08:00:30Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 06:37:31AM +0000, Andreas Ericsson wrote:\n> Pierre Habouzit wrote:\n> > diff --git a/builtin-blame.c b/builtin-blame.c\n> >index e364b6c..16c0ca8 100644\n> >--- a/builtin-blame.c\n> >+++ b/builtin-blame.c\n> >@@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,\n> > static void write_filename_info(const char *path)\n> > {\n> > \tprintf(\"filename \");\n> >-\twrite_name_quoted(NULL, 0, path, 1, stdout);\n> >-\tputchar('\\n');\n> >+\twrite_name_quoted(path, stdout, '\\n');\n> > }\n> > \n> \n> This looks like a candidate for a macro. I'm not sure if gcc optimizes\n> sibling calls in void functions with -O2, and it doesn't inline without\n> -O3.\n\n Well, there is little point. write_name_quoted behaviour changes if\nthe last argument is \\0 or non-\\0 (see patch comment and quote.c code),\nso it does not really matter to inline the \"putchar\" IMHO.\n\n> > -static void diff_flush_raw(struct diff_filepair *p,\n> >-\t\t\t struct diff_options *options)\n> >+static void diff_flush_raw(struct diff_filepair *p, struct diff_options \n> >*opt)\n> \n> Parameter rename? I'd have thought the patch was big enough as it is ;-)\n\n I'm anal when it comes to code: the rule of the least surprise should\napply, and consistency is fundamental. And it happens that diff_options\nare always called `opt' in diff.c, except in that place (and it allows\nto write the prototype of the function on one line).\n\n> Other than that, the diffstat calls this a good patch, and given the\n> fact that all your previous series passed all tests, I assume this one\n> does too.\n\n Yes, before submitting a series I check the testsuite passes at each\nstep, so that it doesn't break git-bisect in obvious ways.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53557","messageId":"46F0D8E2.5090706@op5.se","threadId":"9932","inReplyTo":"20070919080030.GA28205@artemis.corp","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-09-19T08:08:02Z","receivedAt":"2007-09-19T08:08:02Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Pierre Habouzit wrote:\n> On Wed, Sep 19, 2007 at 06:37:31AM +0000, Andreas Ericsson wrote:\n>> Pierre Habouzit wrote:\n>>> diff --git a/builtin-blame.c b/builtin-blame.c\n>>> index e364b6c..16c0ca8 100644\n>>> --- a/builtin-blame.c\n>>> +++ b/builtin-blame.c\n>>> @@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,\n>>> static void write_filename_info(const char *path)\n>>> {\n>>> \tprintf(\"filename \");\n>>> -\twrite_name_quoted(NULL, 0, path, 1, stdout);\n>>> -\tputchar('\\n');\n>>> +\twrite_name_quoted(path, stdout, '\\n');\n>>> }\n>>>\n>> This looks like a candidate for a macro. I'm not sure if gcc optimizes\n>> sibling calls in void functions with -O2, and it doesn't inline without\n>> -O3.\n> \n> Well, there is little point. write_name_quoted behaviour changes if\n> the last argument is \\0 or non-\\0 (see patch comment and quote.c code),\n> so it does not really matter to inline the \"putchar\" IMHO.\n> \n>>> -static void diff_flush_raw(struct diff_filepair *p,\n>>> -\t\t\t struct diff_options *options)\n>>> +static void diff_flush_raw(struct diff_filepair *p, struct diff_options \n>>> *opt)\n>> Parameter rename? I'd have thought the patch was big enough as it is ;-)\n> \n> I'm anal when it comes to code: the rule of the least surprise should\n> apply, and consistency is fundamental. And it happens that diff_options\n> are always called `opt' in diff.c, except in that place (and it allows\n> to write the prototype of the function on one line).\n> \n\nThen perhaps a separate patch for this would have been prudent? I'm not\nagainst the change per se and I understand the reasoning behind it, but\nit seems to go against Documentation/SubmittingPatches (submit one change\nat a time).\n\n-- \nAndreas Ericsson andreas.ericsson@op5.se\nOP5 AB www.op5.se\nTel: +46 8-230225 Fax: +46 8-230231\n"},{"id":"53559","messageId":"7v4phrqdow.fsf@gitster.siamese.dyndns.org","threadId":"9932","inReplyTo":"20070918224121.24C3B344AB3@madism.org","subject":"Re: [PATCH 3/5] Rework unquote_c_style to work on a strbuf.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-19T08:09:19Z","receivedAt":"2007-09-19T08:09:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> If the gain is not obvious in the diffstat, the resulting code is more\n> readable, _and_ in checkout-index/update-index we now reuse the same buffer\n> to unquote strings instead of always freeing/mallocing.\n>\n> This also is more coherent with the next patch that reworks quoting\n> functions.\n>\n> The quoting function is also made more efficient scanning for backslashes\n> and treating portions of strings without a backslash at once.\n>\n> Signed-off-by: Pierre Habouzit \n> ---\n> builtin-apply.c | 125 +++++++++++++++++++++++-----------------------\n> builtin-checkout-index.c | 27 +++++-----\n> builtin-update-index.c | 51 ++++++++++---------\n> fast-import.c | 47 ++++++++---------\n> mktree.c | 25 +++++----\n> quote.c | 92 ++++++++++++++++------------------\n> quote.h | 2 +-\n> 7 files changed, 184 insertions(+), 185 deletions(-)\n> ...\n> diff --git a/quote.c b/quote.c\n> index 4df3262..67c6527 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -201,68 +201,62 @@ int quote_c_style(const char *name, char *outbuf, FILE *outfp, int no_dq)\n> * should free when done. Updates endp pointer to point at\n> * one past the ending double quote if given.\n> */\n\nYou need to update the comment above which talks about the input\nand return values. You no longer return an allocated memory\nwhich the caller should free. You return something else.\n"},{"id":"53560","messageId":"7v3axbqdnw.fsf@gitster.siamese.dyndns.org","threadId":"9932","inReplyTo":"20070918224120.1DC44344AB3@madism.org","subject":"Re: [PATCH 2/5] sq_quote_argv and add_to_string rework with strbuf's.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-19T08:09:55Z","receivedAt":"2007-09-19T08:09:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> * sq_quote_buf is made public, and works on a strbuf.\n> * sq_quote_argv also works on a strbuf.\n> * make sq_quote_argv take a \"maxlen\" argument to check the buffer won't grow\n> too big.\n>\n> Signed-off-by: Pierre Habouzit \n> ---\n> connect.c | 21 ++++++--------\n> git.c | 16 +++-------\n> quote.c | 91 ++++++++++++++++---------------------------------------------\n> quote.h | 9 ++----\n> rsh.c | 33 ++++++----------------\n> trace.c | 35 +++++++-----------------\n> 6 files changed, 60 insertions(+), 145 deletions(-)\n> ...\n> diff --git a/quote.c b/quote.c\n> index d88bf75..4df3262 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -20,29 +20,26 @@ static inline int need_bs_quote(char c)\n> \treturn (c == '\\'' || c == '!');\n> }\n> \n> -static size_t sq_quote_buf(char *dst, size_t n, const char *src)\n> +void sq_quote_buf(struct strbuf *dst, const char *src)\n> {\n\nYou got rid of use of EMIT() macro which is local to this\nfunction, so you need to remove the #undef/#define in front of\nthe function as well.\n"},{"id":"53561","messageId":"20070919082111.GB28205@artemis.corp","threadId":"9932","inReplyTo":"46F0D8E2.5090706@op5.se","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T08:21:11Z","receivedAt":"2007-09-19T08:21:11Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 08:08:02AM +0000, Andreas Ericsson wrote:\n> Then perhaps a separate patch for this would have been prudent? I'm not\n> against the change per se and I understand the reasoning behind it, but\n> it seems to go against Documentation/SubmittingPatches (submit one change\n> at a time).\n\n Yes, the thing is, I wrote it in one piece, and had a _very_ hard time\nsplitting it. The aggregated patches had almost no chunks, and editing\ndiffs by hand isn't what I like to do :)\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53562","messageId":"20070919082239.GC28205@artemis.corp","threadId":"9932","inReplyTo":"7v4phrqdow.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 3/5] Rework unquote_c_style to work on a strbuf.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T08:22:39Z","receivedAt":"2007-09-19T08:22:39Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 08:09:19AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > If the gain is not obvious in the diffstat, the resulting code is more\n> > readable, _and_ in checkout-index/update-index we now reuse the same buffer\n> > to unquote strings instead of always freeing/mallocing.\n> >\n> > This also is more coherent with the next patch that reworks quoting\n> > functions.\n> >\n> > The quoting function is also made more efficient scanning for backslashes\n> > and treating portions of strings without a backslash at once.\n> >\n> > Signed-off-by: Pierre Habouzit \n> > ---\n> > builtin-apply.c | 125 +++++++++++++++++++++++-----------------------\n> > builtin-checkout-index.c | 27 +++++-----\n> > builtin-update-index.c | 51 ++++++++++---------\n> > fast-import.c | 47 ++++++++---------\n> > mktree.c | 25 +++++----\n> > quote.c | 92 ++++++++++++++++------------------\n> > quote.h | 2 +-\n> > 7 files changed, 184 insertions(+), 185 deletions(-)\n> > ...\n> > diff --git a/quote.c b/quote.c\n> > index 4df3262..67c6527 100644\n> > --- a/quote.c\n> > +++ b/quote.c\n> > @@ -201,68 +201,62 @@ int quote_c_style(const char *name, char *outbuf, FILE *outfp, int no_dq)\n> > * should free when done. Updates endp pointer to point at\n> > * one past the ending double quote if given.\n> > */\n> \n> You need to update the comment above which talks about the input\n> and return values. You no longer return an allocated memory\n> which the caller should free. You return something else.\n\n Oh my, okay I'll do that. I intend to send an updated series at some\npoint anyways, because of the two mistakes I already spotted, and\nbecause next conflicts with this series (in not too hard ways to deal\nwith but still, that would save you some useless merges).\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53563","messageId":"20070919082313.GD28205@artemis.corp","threadId":"9932","inReplyTo":"7v3axbqdnw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/5] sq_quote_argv and add_to_string rework with strbuf's.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T08:23:13Z","receivedAt":"2007-09-19T08:23:13Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 08:09:55AM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > * sq_quote_buf is made public, and works on a strbuf.\n> > * sq_quote_argv also works on a strbuf.\n> > * make sq_quote_argv take a \"maxlen\" argument to check the buffer won't grow\n> > too big.\n> >\n> > Signed-off-by: Pierre Habouzit \n> > ---\n> > connect.c | 21 ++++++--------\n> > git.c | 16 +++-------\n> > quote.c | 91 ++++++++++++++++---------------------------------------------\n> > quote.h | 9 ++----\n> > rsh.c | 33 ++++++----------------\n> > trace.c | 35 +++++++-----------------\n> > 6 files changed, 60 insertions(+), 145 deletions(-)\n> > ...\n> > diff --git a/quote.c b/quote.c\n> > index d88bf75..4df3262 100644\n> > --- a/quote.c\n> > +++ b/quote.c\n> > @@ -20,29 +20,26 @@ static inline int need_bs_quote(char c)\n> > \treturn (c == '\\'' || c == '!');\n> > }\n> > \n> > -static size_t sq_quote_buf(char *dst, size_t n, const char *src)\n> > +void sq_quote_buf(struct strbuf *dst, const char *src)\n> > {\n> \n> You got rid of use of EMIT() macro which is local to this\n> function, so you need to remove the #undef/#define in front of\n> the function as well.\n\n Isn't it used by the function before ? hmm I'll check then.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53565","messageId":"86r6kv2h64.fsf@lola.quinscape.zz","threadId":"9932","inReplyTo":"20070919082111.GB28205@artemis.corp","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-19T08:28:03Z","receivedAt":"2007-09-19T08:28:03Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Pierre Habouzit writes:\n\n> On Wed, Sep 19, 2007 at 08:08:02AM +0000, Andreas Ericsson wrote:\n>> Then perhaps a separate patch for this would have been prudent? I'm not\n>> against the change per se and I understand the reasoning behind it, but\n>> it seems to go against Documentation/SubmittingPatches (submit one change\n>> at a time).\n>\n> Yes, the thing is, I wrote it in one piece, and had a _very_ hard time\n> splitting it. The aggregated patches had almost no chunks, and editing\n> diffs by hand isn't what I like to do :)\n\nUse Emacs for it. After loading the patch in a file, type\n\nEsc x diff-mode RET\n\nIf you now move to a place in the middle of a hunk and type C-c C-s,\nthe hunk is split at that point into two hunks. C-c C-k kills the\ncurrent hunk. C-x C-s saves the file, C-x C-c exits Emacs.\n\nIn that manner throwing selected material out of a patch is rather\nstraightforward, even when it is in the middle of a hunk.\n\n-- \nDavid Kastrup\n"},{"id":"53566","messageId":"7v1wcvqcsg.fsf@gitster.siamese.dyndns.org","threadId":"9932","inReplyTo":"20070918224122.2B55D344AB3@madism.org","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-19T08:28:47Z","receivedAt":"2007-09-19T08:28:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> ...\n> Signed-off-by: Pierre Habouzit \n> ---\n> builtin-apply.c | 83 +++++--------\n> builtin-blame.c | 3 +-\n> builtin-check-attr.c | 2 +-\n> builtin-checkout-index.c | 4 +-\n> builtin-ls-files.c | 13 +--\n> builtin-ls-tree.c | 6 +-\n> combine-diff.c | 16 +--\n> diff.c | 303 +++++++++++++++++-----------------------------\n> quote.c | 198 +++++++++++++++++-------------\n> quote.h | 8 +-\n> 10 files changed, 268 insertions(+), 368 deletions(-)\n> ...\n> diff --git a/builtin-apply.c b/builtin-apply.c\n> index cffbe52..0328863 100644\n> --- a/builtin-apply.c\n> +++ b/builtin-apply.c\n> @@ -1378,61 +1377,50 @@ static const char minuses[]= \"--------------------------------------------------\n> \n> static void show_stats(struct patch *patch)\n> {\n> -\tconst char *prefix = \"\";\n> -\tchar *name = patch->new_name;\n> -\tchar *qname = NULL;\n> -\tint len, max, add, del, total;\n> -\n> -\tif (!name)\n> -\t\tname = patch->old_name;\n> +\tstruct strbuf qname;\n> +\tchar *cp = patch->new_name ? patch->new_name : patch->old_name;\n> +\tint max, add, del;\n> \n> -\tif (0 < (len = quote_c_style(name, NULL, NULL, 0))) {\n> -\t\tqname = xmalloc(len + 1);\n> -\t\tquote_c_style(name, qname, NULL, 0);\n> -\t\tname = qname;\n> -\t}\n> +\tstrbuf_init(&qname, 0);\n> +\tquote_c_style(cp, &qname, NULL, 0);\n> \n> \t/*\n> \t * \"scale\" the filename\n> \t */\n> -\tlen = strlen(name);\n> \tmax = max_len;\n> \tif (max > 50)\n> \t\tmax = 50;\n> -\tif (len > max) {\n> -\t\tchar *slash;\n> -\t\tprefix = \"...\";\n> -\t\tmax -= 3;\n> -\t\tname += len - max;\n> -\t\tslash = strchr(name, '/');\n> -\t\tif (slash)\n> -\t\t\tname = slash;\n> +\n> +\tif (qname.len > max) {\n> +\t\tcp = strchr(qname.buf + qname.len + 3 - max, '/');\n> +\t\tif (cp)\n> +\t\t\tcp = qname.buf + qname.len + 3 - max;\n> +\t\tstrbuf_splice(&qname, 0, cp - qname.buf, \"...\", 3);\n> +\t}\n\nAt this point, you have max that is larger by 3 than what old\ncode had. That would make the next two printf() you added as\nexpected. This affects scaling of add/delete code. Is this\nintentional? I _think_ the change is correct (there is no\nreason that name display being cliped should affect the length\nof the bar graph), but that should have been documented as a\nseparate bugfix in the commit log.\n\n> diff --git a/quote.c b/quote.c\n> index 67c6527..a8a755a 100644\n> --- a/quote.c\n> +++ b/quote.c\n> @@ -114,83 +114,142 @@ char *sq_dequote(char *arg)\n> \t}\n> }\n> \n> +/* 1 means: quote as octal\n> + * 0 means: quote as octal if (quote_path_fully)\n> + * -1 means: never quote\n> + * c: quote as \"\\\\c\"\n> + */\n> +#define X8(x) x, x, x, x, x, x, x, x\n> +#define X16(x) X8(x), X8(x)\n> +static signed char const sq_lookup[256] = {\n> +\t/* 0 1 2 3 4 5 6 7 */\n> +\t/* 0x00 */ 1, 1, 1, 1, 1, 1, 'a', 1,\n\nIsn't BEL == 0x07, not 0x06?\n\n> +\t/* 0x08 */ 'b', 't', 'n', 'v', 'f', 'r', 1, 1,\n> +\t/* 0x10 */ X16(1),\n> +\t/* 0x20 */ -1, -1, '\"', -1, -1, -1, -1, -1,\n> +\t/* 0x28 */ X16(-1), X16(-1), X16(-1),\n> +\t/* 0x58 */ -1, -1, -1, -1,'\\\\', -1, -1, -1,\n> +\t/* 0x60 */ X16(-1), X16(-1),\n\nShouldn't you quote DEL == 0177 here?\n\n> +\t/* 0x80 */ /* set to 0 */\n> +};\n> +\n> +static inline int sq_must_quote(char c) {\n> +\treturn sq_lookup[(unsigned char)c] + quote_path_fully > 0;\n> +}\n> +\n> +/* returns the longest prefix not needing a quote up to maxlen if positive.\n> + This stops at the first \\0 because it's marked as a character needing an\n> + escape */\n> +static size_t next_quote_pos(const char *s, ssize_t maxlen)\n> +{\n> +\tsize_t len;\n> +\tif (maxlen < 0) {\n> +\t\tfor (len = 0; !sq_must_quote(s[len]); len++);\n> +\t} else {\n> +\t\tfor (len = 0; len < maxlen && !sq_must_quote(s[len]); len++);\n> +\t}\n> +\treturn len;\n> +}\n> +\n> /*\n> * C-style name quoting.\n> *\n> - * Does one of three things:\n> - *\n> * (1) if outbuf and outfp are both NULL, inspect the input name and\n> * counts the number of bytes that are needed to hold c_style\n> * quoted version of name, counting the double quotes around\n> * it but not terminating NUL, and returns it. However, if name\n> * does not need c_style quoting, it returns 0.\n> *\n\nYou need to update this comment; you do not have outbuf nor\noutfp anymore, you have something else.\n"},{"id":"53567","messageId":"7vwsunoy3d.fsf@gitster.siamese.dyndns.org","threadId":"9932","inReplyTo":"86r6kv2h64.fsf@lola.quinscape.zz","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-19T08:31:34Z","receivedAt":"2007-09-19T08:31:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Kastrup writes:\n\n> Pierre Habouzit writes:\n>\n>> On Wed, Sep 19, 2007 at 08:08:02AM +0000, Andreas Ericsson wrote:\n>>> Then perhaps a separate patch for this would have been prudent? I'm not\n>>> against the change per se and I understand the reasoning behind it, but\n>>> it seems to go against Documentation/SubmittingPatches (submit one change\n>>> at a time).\n>>\n>> Yes, the thing is, I wrote it in one piece, and had a _very_ hard time\n>> splitting it. The aggregated patches had almost no chunks, and editing\n>> diffs by hand isn't what I like to do :)\n>\n> Use Emacs for it. After loading the patch in a file, type\n>\n> Esc x diff-mode RET\n>\n> If you now move to a place in the middle of a hunk and type C-c C-s,\n> the hunk is split at that point into two hunks. C-c C-k kills the\n> current hunk. C-x C-s saves the file, C-x C-c exits Emacs.\n>\n> In that manner throwing selected material out of a patch is rather\n> straightforward, even when it is in the middle of a hunk.\n\nBe careful when you edit format-patch output. It seems that\ndiff-mode tends to mistake the trailing \"signature separator\" at\nthe end as if it is a removal of a line from the preimage, and\nediting the last hunk ends up miscalculating the number of lines\nin it. It might have been fixed in the latest version but I was\nburned by it number of times.\n"},{"id":"53569","messageId":"86fy1b2goz.fsf@lola.quinscape.zz","threadId":"9932","inReplyTo":"7vwsunoy3d.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-09-19T08:38:20Z","receivedAt":"2007-09-19T08:38:20Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\n[Also posted to the list via gmane, so reply there if appropriate]\n\nJunio C Hamano writes:\n\n> David Kastrup writes:\n>>\n>> Esc x diff-mode RET\n>\n> Be careful when you edit format-patch output. It seems that\n> diff-mode tends to mistake the trailing \"signature separator\" at\n> the end as if it is a removal of a line from the preimage, and\n> editing the last hunk ends up miscalculating the number of lines\n> in it. It might have been fixed in the latest version but I was\n> burned by it number of times.\n\nWould you be able to prepare an example and submit it using\nM-x report-emacs-bug RET (probably using an attachment)?\n\nEmacs 22.2 is likely to come out in a few months mainly as a bug fix,\nincremental change and maintenance release to 22.1, and this would be\nvery much the kind of bug that warrants getting fixed, in particular\nsince it appears likely that Emacs 22.2 will come with git support in\nVC.\n\nIf there is a good test case known to fail in your Emacs version, it\nwould be quite easy to verify that it is or gets fixed in 22.2\n\nThanks,\n\n-- \nDavid Kastrup\n"},{"id":"53570","messageId":"20070919084703.GG28205@artemis.corp","threadId":"9932","inReplyTo":"7v1wcvqcsg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T08:47:03Z","receivedAt":"2007-09-19T08:47:03Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 08:28:47AM +0000, Junio C Hamano wrote:\n> At this point, you have max that is larger by 3 than what old\n> code had. That would make the next two printf() you added as\n> expected. This affects scaling of add/delete code. Is this\n> intentional? I _think_ the change is correct (there is no\n> reason that name display being cliped should affect the length\n> of the bar graph), but that should have been documented as a\n> separate bugfix in the commit log.\n\n Indeed, in fact I didn't noticed that difference, I'll document that.\n\n> \n> > diff --git a/quote.c b/quote.c\n> > index 67c6527..a8a755a 100644\n> > --- a/quote.c\n> > +++ b/quote.c\n> > @@ -114,83 +114,142 @@ char *sq_dequote(char *arg)\n> > \t}\n> > }\n> > \n> > +/* 1 means: quote as octal\n> > + * 0 means: quote as octal if (quote_path_fully)\n> > + * -1 means: never quote\n> > + * c: quote as \"\\\\c\"\n> > + */\n> > +#define X8(x) x, x, x, x, x, x, x, x\n> > +#define X16(x) X8(x), X8(x)\n> > +static signed char const sq_lookup[256] = {\n> > +\t/* 0 1 2 3 4 5 6 7 */\n> > +\t/* 0x00 */ 1, 1, 1, 1, 1, 1, 'a', 1,\n> \n> Isn't BEL == 0x07, not 0x06?\n\n indeed.\n\n> > +\t/* 0x08 */ 'b', 't', 'n', 'v', 'f', 'r', 1, 1,\n> > +\t/* 0x10 */ X16(1),\n> > +\t/* 0x20 */ -1, -1, '\"', -1, -1, -1, -1, -1,\n> > +\t/* 0x28 */ X16(-1), X16(-1), X16(-1),\n> > +\t/* 0x58 */ -1, -1, -1, -1,'\\\\', -1, -1, -1,\n> > +\t/* 0x60 */ X16(-1), X16(-1),\n> \n> Shouldn't you quote DEL == 0177 here?\n\n indeed again.\n\n> > /*\n> > * C-style name quoting.\n> > *\n> > - * Does one of three things:\n> > - *\n> > * (1) if outbuf and outfp are both NULL, inspect the input name and\n> > * counts the number of bytes that are needed to hold c_style\n> > * quoted version of name, counting the double quotes around\n> > * it but not terminating NUL, and returns it. However, if name\n> > * does not need c_style quoting, it returns 0.\n> > *\n> \n> You need to update this comment; you do not have outbuf nor\n> outfp anymore, you have something else.\n\n heh, well outfp is still here, but I'll fix the part about outbuf.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53579","messageId":"20070919144604.7deca4f7.froese@gmx.de","threadId":"9932","inReplyTo":"20070918224119.17650344AB3@madism.org","subject":"Re: [PATCH 1/5] strbuf API additions and enhancements.","fromName":"Edgar Toernig","fromEmail":"froese@gmx.de","sentAt":"2007-09-19T12:46:04Z","receivedAt":"2007-09-19T12:46:04Z","isPatch":true,"sender":{"key":"froese@gmx.de","avatar":null},"body":"Pierre Habouzit wrote:\n>\n> +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)\n> +{\n> +\tint len;\n> +\n> +\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n> +\tif (len < 0) {\n> +\t\tlen = 0;\n> +\t}\n> +\tif (len > strbuf_avail(sb)) {\n> +\t\tstrbuf_grow(sb, len);\n> +\t\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n> +\t\tif (len > strbuf_avail(sb)) {\n> +\t\t\tdie(\"this should not happen, your snprintf is broken\");\n> +\t\t}\n> +\t}\n> +\tstrbuf_setlen(sb, sb->len + len);\n> +}\n\nThe second vsnprintf won't work as the first one consumed all args\nfrom va_list ap. You need to va_copy the ap. But iirc va_copy poses\ncompatibility issues. Unless va_copy is made available somehow,\nI would suggest to let the caller know that the buffer was too small\n(but isn't any more) and it has to call the function again:\n\nint strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)\n{\n\tint len;\n\n\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n\tif (len < 0)\n\t\treturn 0;\n\tif (len > strbuf_avail(sb)) {\n\t\tstrbuf_grow(sb, len);\n\t\treturn -1;\n\t}\n\tstrbuf_setlen(sb, sb->len + len);\n\treturn 0;\n}\n\nThe caller:\n\n\tdo {\n\t\tva_start(ap, fmt);\n\t\tagain = strbuf_addvf(sb, fmt, ap);\n\t\tva_end(ap);\n\t} while (again);\n\nva_copy would be nicer though...\n\nCiao, ET.\n"},{"id":"53582","messageId":"20070919133647.GA17192@artemis.corp","threadId":"9932","inReplyTo":"20070919144604.7deca4f7.froese@gmx.de","subject":"Re: [PATCH 1/5] strbuf API additions and enhancements.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-19T13:36:47Z","receivedAt":"2007-09-19T13:36:47Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Sep 19, 2007 at 12:46:04PM +0000, Edgar Toernig wrote:\n> Pierre Habouzit wrote:\n> >\n> > +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)\n> > +{\n> > +\tint len;\n> > +\n> > +\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n> > +\tif (len < 0) {\n> > +\t\tlen = 0;\n> > +\t}\n> > +\tif (len > strbuf_avail(sb)) {\n> > +\t\tstrbuf_grow(sb, len);\n> > +\t\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n> > +\t\tif (len > strbuf_avail(sb)) {\n> > +\t\t\tdie(\"this should not happen, your snprintf is broken\");\n> > +\t\t}\n> > +\t}\n> > +\tstrbuf_setlen(sb, sb->len + len);\n> > +}\n> \n> The second vsnprintf won't work as the first one consumed all args\n> from va_list ap. You need to va_copy the ap. But iirc va_copy poses\n> compatibility issues. Unless va_copy is made available somehow,\n> I would suggest to let the caller know that the buffer was too small\n> (but isn't any more) and it has to call the function again:\n\n That's what I thought, and then nfvasprintf in trace.c suffers from\nthe same issue, as I copied the code from there.\n\n> \tdo {\n> \t\tva_start(ap, fmt);\n> \t\tagain = strbuf_addvf(sb, fmt, ap);\n> \t\tva_end(ap);\n> \t} while (again);\n\n in fact doing it twice is enough but either way I don't like to impose\nthat to the caller :/ I mean it's totally stupid to have to do that on a\nstrbuf. of course we could provide a macro doing that ...\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"53591","messageId":"7vmyvipk6t.fsf@gitster.siamese.dyndns.org","threadId":"9932","inReplyTo":"20070919133647.GA17192@artemis.corp","subject":"Re: [PATCH 1/5] strbuf API additions and enhancements.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-19T18:46:34Z","receivedAt":"2007-09-19T18:46:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> ... in fact doing it twice is enough but either way I don't like to impose\n> that to the caller :/\n\nI do not think so either. We had a similar issue resolved with\n4bf53833.\n"},{"id":"53628","messageId":"46F21097.5030901@eudaptics.com","threadId":"9932","inReplyTo":"20070919144604.7deca4f7.froese@gmx.de","subject":"Re: [PATCH 1/5] strbuf API additions and enhancements.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-09-20T06:17:59Z","receivedAt":"2007-09-20T06:17:59Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Edgar Toernig schrieb:\n> Pierre Habouzit wrote:\n>> +void strbuf_addvf(struct strbuf *sb, const char *fmt, va_list ap)\n>> +{\n>> +\tint len;\n>> +\n>> +\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n>> +\tif (len < 0) {\n>> +\t\tlen = 0;\n>> +\t}\n>> +\tif (len > strbuf_avail(sb)) {\n>> +\t\tstrbuf_grow(sb, len);\n>> +\t\tlen = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);\n>> +\t\tif (len > strbuf_avail(sb)) {\n>> +\t\t\tdie(\"this should not happen, your snprintf is broken\");\n>> +\t\t}\n>> +\t}\n>> +\tstrbuf_setlen(sb, sb->len + len);\n>> +}\n> \n> The second vsnprintf won't work as the first one consumed all args\n> from va_list ap. You need to va_copy the ap.\n\nYour analysis is not correct. The second vsnprintf receives the same \nargument pointer as the first, and, hence, consumes the same set of arguments.\n\nYou have to use va_copy in a variadic function, ie. if you are using \nva_start+va_end in the same function, but not in a function with a fixed \nlist of arguments like this one.\n\n-- Hannes\n"},{"id":"53631","messageId":"87lkb1iz0i.fsf@Astalo.kon.iki.fi","threadId":"9932","inReplyTo":"46F21097.5030901@eudaptics.com","subject":"Re: [PATCH 1/5] strbuf API additions and enhancements.","fromName":"Kalle Olavi Niemitalo","fromEmail":"kon@iki.fi","sentAt":"2007-09-20T07:20:29Z","receivedAt":"2007-09-20T07:20:29Z","isPatch":true,"sender":{"key":"kon@iki.fi","avatar":null},"body":"Johannes Sixt writes:\n\n> Edgar Toernig schrieb:\n>> The second vsnprintf won't work as the first one consumed all args\n>> from va_list ap. You need to va_copy the ap.\n>\n> Your analysis is not correct. The second vsnprintf receives the same\n> argument pointer as the first, and, hence, consumes the same set of\n> arguments.\n\nC99 7.9.16.2p2 has a footnote: \"As the functions vfprintf,\nvfscanf, vprintf, vscanf, vsnprintf, vsprintf, and vsscanf invoke\nthe va_arg macro, the value of arg after the return is\nindeterminate.\"\n\nNormative text in 7.15p3 confirms this: \"The object ap may be\npassed as an argument to another function; if that function\ninvokes the va_arg macro with parameter ap, the value of ap in\nthe calling function is indeterminate and shall be passed to the\nva_end macro prior to any further reference to ap.\"\n\nTherefore va_copy is needed here, at least in principle.\n"},{"id":"53655","messageId":"20070920161007.GA22876@sigill.intra.peff.net","threadId":"9932","inReplyTo":"87lkb1iz0i.fsf@Astalo.kon.iki.fi","subject":"Re: [PATCH 1/5] strbuf API additions and enhancements.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-09-20T16:10:07Z","receivedAt":"2007-09-20T16:10:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 20, 2007 at 10:20:29AM +0300, Kalle Olavi Niemitalo wrote:\n\n> Normative text in 7.15p3 confirms this: \"The object ap may be\n> passed as an argument to another function; if that function\n> invokes the va_arg macro with parameter ap, the value of ap in\n> the calling function is indeterminate and shall be passed to the\n> va_end macro prior to any further reference to ap.\"\n> \n> Therefore va_copy is needed here, at least in principle.\n\nNot just in principle; a few months ago, I ran afoul of the same issue\nusing gcc + glibc6, so it is a real problem for our target platforms\n(sorry, I don't have a test case anymore, but I recall getting\nundefined-ish behavior from my print statements).\n\n-Peff\n"}]}