{"thread":{"id":"36875","subject":"[PATCH v2 00/19] add strbuf_set operations","startedAt":"2014-06-09T22:19:19Z","lastAt":"2014-06-12T23:59:53Z","messageCount":28,"participants":["Jeremiah Mahler","Eric Sunshine","Michael Haggerty"],"isPatch":true,"patchVersion":2,"patchTotal":19},"messages":[{"id":"243651","messageId":"cover.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":null,"subject":"[PATCH v2 00/19] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:19Z","receivedAt":"2014-06-09T22:19:19Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Version 2 of the patch set to add strbuf_set operations.\n\nIncludes suggestions from Eric Sunshine [1]:\n\n - New operations and their documentation placed in one patch.\n\n - Less ambiguous documentation: \"Replace the buffer content with [...]\"\n\n - Use imperative mood in log messages.\n\n - Don't use strbuf_set operations in cases where strbuf_add is used to\n build up a buffer in multiple steps. Multiple adds suggest that\n\tre-ordering is possible whereas a strbuf_set at the beginning\n\tsuggests that it isn't. This is confusing.\n\nUsing strbuf_set before a sequence of adds can be confusing. But using\nstrbuf_add when nothing important was being added to can also be\nconfusing. strbuf_set can be used to make these cases more clear.\n\n struct strbuf mybuf = STRBUF_INIT;\n\n strbuf_add(&mybuf, ...); /* Was something there before? */\n\n strbuf_set(&mybuf, ...); /* Replace everything. */\n\nSeveral single line replacements were made for this reason.\n\nAdditional files have also been converted compared to version 1 [1].\n\n[1]: http://marc.info/?l=git&m=140230874124060&w=2\n\nJeremiah Mahler (19):\n add strbuf_set operations\n sha1_name: simplify via strbuf_set()\n fast-import: simplify via strbuf_set()\n builtin/remote: simplify via strbuf_set()\n branch: simplify via strbuf_set()\n builtin/branch: simplify via strbuf_set()\n builtin/checkout: simplify via strbuf_set()\n builtin/mailinfo: simplify via strbuf_set()\n builtin/tag: simplify via strbuf_set()\n date: simplify via strbuf_set()\n diffcore-order: simplify via strbuf_set()\n http-backend: simplify via strbuf_set()\n http: simplify via strbuf_set()\n ident: simplify via strbuf_set()\n remote-curl: simplify via strbuf_set()\n submodule: simplify via strbuf_set()\n transport: simplify via strbuf_set()\n vcs-svn/svndump: simplify via strbuf_set()\n wt-status: simplify via strbuf_set()\n\n Documentation/technical/api-strbuf.txt | 18 +++++++++++\n branch.c | 6 ++--\n builtin/branch.c | 3 +-\n builtin/checkout.c | 18 ++++-------\n builtin/mailinfo.c | 18 ++++-------\n builtin/remote.c | 59 ++++++++++++----------------------\n builtin/tag.c | 3 +-\n date.c | 3 +-\n diffcore-order.c | 3 +-\n fast-import.c | 6 ++--\n http-backend.c | 6 ++--\n http.c | 3 +-\n ident.c | 6 ++--\n remote-curl.c | 3 +-\n sha1_name.c | 15 +++------\n strbuf.c | 21 ++++++++++++\n strbuf.h | 14 ++++++++\n submodule.c | 5 ++-\n transport.c | 3 +-\n vcs-svn/svndump.c | 3 +-\n wt-status.c | 3 +-\n 21 files changed, 110 insertions(+), 109 deletions(-)\n\n-- \n2.0.0.592.gf55b190\n"},{"id":"243652","messageId":"6fe33498512fc2ca1678517e51dc2e94a4260ff4.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 01/19] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:20Z","receivedAt":"2014-06-09T22:19:20Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Currently, the data in a strbuf is modified using add operations. To\nset the buffer to some data a reset must be performed before an add.\n\n strbuf_reset(buf);\n strbuf_add(buf, cb.buf.buf, cb.buf.len);\n\nAnd this is a common sequence of operations with 70 occurrences found in\nthe current source code. This includes all the different variations\n(add, addf, addstr, addbuf, addch).\n\n FILES=`find ./ -name '*.c'`\n CNT=$(pcregrep -M \"strbuf_reset.*\\n.*strbuf_add\" $FILES | wc -l)\n CNT=$(echo \"$CNT / 2\" | bc)\n echo $CNT\n 70\n\nThese patches add strbuf_set operations which allow this common sequence\nto be performed in one line instead of two.\n\n strbuf_set(buf, cb.buf.buf, cb.buf.len);\n\nSigned-off-by: Jeremiah Mahler \n---\n Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n strbuf.c | 21 +++++++++++++++++++++\n strbuf.h | 14 ++++++++++++++\n 3 files changed, 53 insertions(+)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex 077a709..b7e23da 100644\n--- a/Documentation/technical/api-strbuf.txt\n+++ b/Documentation/technical/api-strbuf.txt\n@@ -149,6 +149,24 @@ Functions\n \tthan zero if the first buffer is found, respectively, to be less than,\n \tto match, or be greater than the second buffer.\n \n+* Setting the buffer\n+\n+`strbuf_set`::\n+\n+\tReplace the buffer content with data of a given length.\n+\n+`strbuf_setstr`::\n+\n+\tReplace the buffer content with data from a NUL-terminated string.\n+\n+`strbuf_setf`::\n+\n+\tReplace the buffer content with a formatted string.\n+\n+`strbuf_setbuf`::\n+\n+\tReplace the buffer content with data from another buffer.\n+\n * Adding data to the buffer\n \n NOTE: All of the functions in this section will grow the buffer as necessary.\ndiff --git a/strbuf.c b/strbuf.c\nindex ac62982..9d64b00 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -189,6 +189,27 @@ 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_set(struct strbuf *sb, const void *data, size_t len)\n+{\n+\tstrbuf_reset(sb);\n+\tstrbuf_add(sb, data, len);\n+}\n+\n+void strbuf_setf(struct strbuf *sb, const char *fmt, ...)\n+{\n+\tva_list ap;\n+\tstrbuf_reset(sb);\n+\tva_start(ap, fmt);\n+\tstrbuf_vaddf(sb, fmt, ap);\n+\tva_end(ap);\n+}\n+\n+void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)\n+{\n+\tstrbuf_reset(sb);\n+\tstrbuf_add(sb, sb2->buf, sb2->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);\ndiff --git a/strbuf.h b/strbuf.h\nindex e9ad03e..b339f08 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -101,6 +101,20 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,\n */\n extern void strbuf_list_free(struct strbuf **);\n \n+/*----- set buffer to data -----*/\n+\n+extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);\n+\n+static inline void strbuf_setstr(struct strbuf *sb, const char *s)\n+{\n+\tstrbuf_set(sb, s, strlen(s));\n+}\n+\n+__attribute__((format (printf,2,3)))\n+extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);\n+\n+extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);\n+\n /*----- add data in your buffer -----*/\n static inline void strbuf_addch(struct strbuf *sb, int c)\n {\n-- \n2.0.0.592.gf55b190\n"},{"id":"243653","messageId":"724e8529e76a5868787311c10ed025a4e80e25cc.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 02/19] sha1_name: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:21Z","receivedAt":"2014-06-09T22:19:21Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n sha1_name.c | 15 +++++----------\n 1 file changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 2b6322f..f88b66c 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -920,8 +920,7 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,\n \t\treturn 0;\n \tif (--(cb->remaining) == 0) {\n \t\tlen = target - match;\n-\t\tstrbuf_reset(&cb->buf);\n-\t\tstrbuf_add(&cb->buf, match, len);\n+\t\tstrbuf_set(&cb->buf, match, len);\n \t\treturn 1; /* we are done */\n \t}\n \treturn 0;\n@@ -957,8 +956,7 @@ static int interpret_nth_prior_checkout(const char *name, int namelen,\n \n \tretval = 0;\n \tif (0 < for_each_reflog_ent_reverse(\"HEAD\", grab_nth_branch_switch, &cb)) {\n-\t\tstrbuf_reset(buf);\n-\t\tstrbuf_add(buf, cb.buf.buf, cb.buf.len);\n+\t\tstrbuf_set(buf, cb.buf.buf, cb.buf.len);\n \t\tretval = brace - name + 1;\n \t}\n \n@@ -1025,8 +1023,7 @@ static int interpret_empty_at(const char *name, int namelen, int len, struct str\n \tif (next != name + 1)\n \t\treturn -1;\n \n-\tstrbuf_reset(buf);\n-\tstrbuf_add(buf, \"HEAD\", 4);\n+\tstrbuf_set(buf, \"HEAD\", 4);\n \treturn 1;\n }\n \n@@ -1044,8 +1041,7 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu\n \t\tstrbuf_setlen(buf, used);\n \t\treturn len;\n \t}\n-\tstrbuf_reset(buf);\n-\tstrbuf_addbuf(buf, &tmp);\n+\tstrbuf_setbuf(buf, &tmp);\n \tstrbuf_release(&tmp);\n \t/* tweak for size of {-N} versus expanded ref name */\n \treturn ret - used + len;\n@@ -1054,8 +1050,7 @@ static int reinterpret(const char *name, int namelen, int len, struct strbuf *bu\n static void set_shortened_ref(struct strbuf *buf, const char *ref)\n {\n \tchar *s = shorten_unambiguous_ref(ref, 0);\n-\tstrbuf_reset(buf);\n-\tstrbuf_addstr(buf, s);\n+\tstrbuf_setstr(buf, s);\n \tfree(s);\n }\n \n-- \n2.0.0.592.gf55b190\n"},{"id":"243654","messageId":"3c2074297916c6f5f8deb923432af22edd1dccd2.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 03/19] fast-import: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:22Z","receivedAt":"2014-06-09T22:19:22Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n fast-import.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex e8ec34d..cfe9404 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2898,8 +2898,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n \t * Output based on batch_one_object() from cat-file.c.\n \t */\n \tif (type <= 0) {\n-\t\tstrbuf_reset(&line);\n-\t\tstrbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n+\t\tstrbuf_setf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n \t\tcat_blob_write(line.buf, line.len);\n \t\tstrbuf_release(&line);\n \t\tfree(buf);\n@@ -2910,8 +2909,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n \tif (type != OBJ_BLOB)\n \t\tdie(\"Object %s is a %s but a blob was expected.\",\n \t\t sha1_to_hex(sha1), typename(type));\n-\tstrbuf_reset(&line);\n-\tstrbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n+\tstrbuf_setf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n \t\t\t\t\t\ttypename(type), size);\n \tcat_blob_write(line.buf, line.len);\n \tstrbuf_release(&line);\n-- \n2.0.0.592.gf55b190\n"},{"id":"243655","messageId":"93f9d508bddd51115797da346e58880818d0dd6f.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 04/19] builtin/remote: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:23Z","receivedAt":"2014-06-09T22:19:23Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n builtin/remote.c | 59 ++++++++++++++++++++------------------------------------\n 1 file changed, 21 insertions(+), 38 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex c9b67ff..f2d809d 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -193,8 +193,7 @@ static int add(int argc, const char **argv)\n \t\treturn 1;\n \n \tif (!mirror || mirror & MIRROR_FETCH) {\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"remote.%s.fetch\", name);\n+\t\tstrbuf_setf(&buf, \"remote.%s.fetch\", name);\n \t\tif (track.nr == 0)\n \t\t\tstring_list_append(&track, \"*\");\n \t\tfor (i = 0; i < track.nr; i++) {\n@@ -205,15 +204,13 @@ static int add(int argc, const char **argv)\n \t}\n \n \tif (mirror & MIRROR_PUSH) {\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"remote.%s.mirror\", name);\n+\t\tstrbuf_setf(&buf, \"remote.%s.mirror\", name);\n \t\tif (git_config_set(buf.buf, \"true\"))\n \t\t\treturn 1;\n \t}\n \n \tif (fetch_tags != TAGS_DEFAULT) {\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"remote.%s.tagopt\", name);\n+\t\tstrbuf_setf(&buf, \"remote.%s.tagopt\", name);\n \t\tif (git_config_set(buf.buf,\n \t\t\tfetch_tags == TAGS_SET ? \"--tags\" : \"--no-tags\"))\n \t\t\treturn 1;\n@@ -223,11 +220,9 @@ static int add(int argc, const char **argv)\n \t\treturn 1;\n \n \tif (master) {\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"refs/remotes/%s/HEAD\", name);\n+\t\tstrbuf_setf(&buf, \"refs/remotes/%s/HEAD\", name);\n \n-\t\tstrbuf_reset(&buf2);\n-\t\tstrbuf_addf(&buf2, \"refs/remotes/%s/%s\", name, master);\n+\t\tstrbuf_setf(&buf2, \"refs/remotes/%s/%s\", name, master);\n \n \t\tif (create_symref(buf.buf, buf2.buf, \"remote add\"))\n \t\t\treturn error(_(\"Could not setup master '%s'\"), master);\n@@ -584,19 +579,17 @@ static int migrate_file(struct remote *remote)\n \tint i;\n \tconst char *path = NULL;\n \n-\tstrbuf_addf(&buf, \"remote.%s.url\", remote->name);\n+\tstrbuf_setf(&buf, \"remote.%s.url\", remote->name);\n \tfor (i = 0; i < remote->url_nr; i++)\n \t\tif (git_config_set_multivar(buf.buf, remote->url[i], \"^$\", 0))\n \t\t\treturn error(_(\"Could not append '%s' to '%s'\"),\n \t\t\t\t\tremote->url[i], buf.buf);\n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"remote.%s.push\", remote->name);\n+\tstrbuf_setf(&buf, \"remote.%s.push\", remote->name);\n \tfor (i = 0; i < remote->push_refspec_nr; i++)\n \t\tif (git_config_set_multivar(buf.buf, remote->push_refspec[i], \"^$\", 0))\n \t\t\treturn error(_(\"Could not append '%s' to '%s'\"),\n \t\t\t\t\tremote->push_refspec[i], buf.buf);\n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"remote.%s.fetch\", remote->name);\n+\tstrbuf_setf(&buf, \"remote.%s.fetch\", remote->name);\n \tfor (i = 0; i < remote->fetch_refspec_nr; i++)\n \t\tif (git_config_set_multivar(buf.buf, remote->fetch_refspec[i], \"^$\", 0))\n \t\t\treturn error(_(\"Could not append '%s' to '%s'\"),\n@@ -640,27 +633,24 @@ static int mv(int argc, const char **argv)\n \tif (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr))\n \t\tdie(_(\"remote %s already exists.\"), rename.new);\n \n-\tstrbuf_addf(&buf, \"refs/heads/test:refs/remotes/%s/test\", rename.new);\n+\tstrbuf_setf(&buf, \"refs/heads/test:refs/remotes/%s/test\", rename.new);\n \tif (!valid_fetch_refspec(buf.buf))\n \t\tdie(_(\"'%s' is not a valid remote name\"), rename.new);\n \n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"remote.%s\", rename.old);\n-\tstrbuf_addf(&buf2, \"remote.%s\", rename.new);\n+\tstrbuf_setf(&buf, \"remote.%s\", rename.old);\n+\tstrbuf_setf(&buf2, \"remote.%s\", rename.new);\n \tif (git_config_rename_section(buf.buf, buf2.buf) < 1)\n \t\treturn error(_(\"Could not rename config section '%s' to '%s'\"),\n \t\t\t\tbuf.buf, buf2.buf);\n \n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"remote.%s.fetch\", rename.new);\n+\tstrbuf_setf(&buf, \"remote.%s.fetch\", rename.new);\n \tif (git_config_set_multivar(buf.buf, NULL, NULL, 1))\n \t\treturn error(_(\"Could not remove config section '%s'\"), buf.buf);\n-\tstrbuf_addf(&old_remote_context, \":refs/remotes/%s/\", rename.old);\n+\tstrbuf_setf(&old_remote_context, \":refs/remotes/%s/\", rename.old);\n \tfor (i = 0; i < oldremote->fetch_refspec_nr; i++) {\n \t\tchar *ptr;\n \n-\t\tstrbuf_reset(&buf2);\n-\t\tstrbuf_addstr(&buf2, oldremote->fetch_refspec[i]);\n+\t\tstrbuf_setstr(&buf2, oldremote->fetch_refspec[i]);\n \t\tptr = strstr(buf2.buf, old_remote_context.buf);\n \t\tif (ptr) {\n \t\t\trefspec_updated = 1;\n@@ -683,8 +673,7 @@ static int mv(int argc, const char **argv)\n \t\tstruct string_list_item *item = branch_list.items + i;\n \t\tstruct branch_info *info = item->util;\n \t\tif (info->remote_name && !strcmp(info->remote_name, rename.old)) {\n-\t\t\tstrbuf_reset(&buf);\n-\t\t\tstrbuf_addf(&buf, \"branch.%s.remote\", item->string);\n+\t\t\tstrbuf_setf(&buf, \"branch.%s.remote\", item->string);\n \t\t\tif (git_config_set(buf.buf, rename.new)) {\n \t\t\t\treturn error(_(\"Could not set '%s'\"), buf.buf);\n \t\t\t}\n@@ -715,12 +704,10 @@ static int mv(int argc, const char **argv)\n \n \t\tif (item->util)\n \t\t\tcontinue;\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addstr(&buf, item->string);\n+\t\tstrbuf_setstr(&buf, item->string);\n \t\tstrbuf_splice(&buf, strlen(\"refs/remotes/\"), strlen(rename.old),\n \t\t\t\trename.new, strlen(rename.new));\n-\t\tstrbuf_reset(&buf2);\n-\t\tstrbuf_addf(&buf2, \"remote: renamed %s to %s\",\n+\t\tstrbuf_setf(&buf2, \"remote: renamed %s to %s\",\n \t\t\t\titem->string, buf.buf);\n \t\tif (rename_ref(item->string, buf.buf, buf2.buf))\n \t\t\tdie(_(\"renaming '%s' failed\"), item->string);\n@@ -730,16 +717,13 @@ static int mv(int argc, const char **argv)\n \n \t\tif (!item->util)\n \t\t\tcontinue;\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addstr(&buf, item->string);\n+\t\tstrbuf_setstr(&buf, item->string);\n \t\tstrbuf_splice(&buf, strlen(\"refs/remotes/\"), strlen(rename.old),\n \t\t\t\trename.new, strlen(rename.new));\n-\t\tstrbuf_reset(&buf2);\n-\t\tstrbuf_addstr(&buf2, item->util);\n+\t\tstrbuf_setstr(&buf2, item->util);\n \t\tstrbuf_splice(&buf2, strlen(\"refs/remotes/\"), strlen(rename.old),\n \t\t\t\trename.new, strlen(rename.new));\n-\t\tstrbuf_reset(&buf3);\n-\t\tstrbuf_addf(&buf3, \"remote: renamed %s to %s\",\n+\t\tstrbuf_setf(&buf3, \"remote: renamed %s to %s\",\n \t\t\t\titem->string, buf.buf);\n \t\tif (create_symref(buf.buf, buf2.buf, buf3.buf))\n \t\t\tdie(_(\"creating '%s' failed\"), buf.buf);\n@@ -804,8 +788,7 @@ static int rm(int argc, const char **argv)\n \t\tif (info->remote_name && !strcmp(info->remote_name, remote->name)) {\n \t\t\tconst char *keys[] = { \"remote\", \"merge\", NULL }, **k;\n \t\t\tfor (k = keys; *k; k++) {\n-\t\t\t\tstrbuf_reset(&buf);\n-\t\t\t\tstrbuf_addf(&buf, \"branch.%s.%s\",\n+\t\t\t\tstrbuf_setf(&buf, \"branch.%s.%s\",\n \t\t\t\t\t\titem->string, *k);\n \t\t\t\tif (git_config_set(buf.buf, NULL)) {\n \t\t\t\t\tstrbuf_release(&buf);\n-- \n2.0.0.592.gf55b190\n"},{"id":"243657","messageId":"6380bb5aabeb990729d821707d80d9b4f01137de.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 05/19] branch: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:24Z","receivedAt":"2014-06-09T22:19:24Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n branch.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 660097b..bc7cc7e 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -65,13 +65,11 @@ void install_branch_config(int flag, const char *local, const char *origin, cons\n \tstrbuf_addf(&key, \"branch.%s.remote\", local);\n \tgit_config_set(key.buf, origin ? origin : \".\");\n \n-\tstrbuf_reset(&key);\n-\tstrbuf_addf(&key, \"branch.%s.merge\", local);\n+\tstrbuf_setf(&key, \"branch.%s.merge\", local);\n \tgit_config_set(key.buf, remote);\n \n \tif (rebasing) {\n-\t\tstrbuf_reset(&key);\n-\t\tstrbuf_addf(&key, \"branch.%s.rebase\", local);\n+\t\tstrbuf_setf(&key, \"branch.%s.rebase\", local);\n \t\tgit_config_set(key.buf, \"true\");\n \t}\n \tstrbuf_release(&key);\n-- \n2.0.0.592.gf55b190\n"},{"id":"243656","messageId":"85cfb1d4cafce87e134d4329ccecad745eb5c39e.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 06/19] builtin/branch: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:25Z","receivedAt":"2014-06-09T22:19:25Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n builtin/branch.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 2d1c57c..ad641b6 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -984,8 +984,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \n \t\tstrbuf_addf(&buf, \"branch.%s.remote\", branch->name);\n \t\tgit_config_set_multivar(buf.buf, NULL, NULL, 1);\n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf(&buf, \"branch.%s.merge\", branch->name);\n+\t\tstrbuf_setf(&buf, \"branch.%s.merge\", branch->name);\n \t\tgit_config_set_multivar(buf.buf, NULL, NULL, 1);\n \t\tstrbuf_release(&buf);\n \t} else if (argc > 0 && argc <= 2) {\n-- \n2.0.0.592.gf55b190\n"},{"id":"243658","messageId":"4a675aa5439b7f1cc932e3c28cfdad051dd0eb5b.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 07/19] builtin/checkout: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:26Z","receivedAt":"2014-06-09T22:19:26Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n builtin/checkout.c | 18 ++++++------------\n 1 file changed, 6 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 9cbe7d1..38fc0ce 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -912,8 +912,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts,\n \t\t\t sb_git.buf);\n \tjunk_work_tree = path;\n \n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n+\tstrbuf_setf(&sb, \"%s/gitdir\", sb_repo.buf);\n \twrite_file(sb.buf, 1, \"%s\\n\", real_path(sb_git.buf));\n \twrite_file(sb_git.buf, 1, \"gitdir: %s/repos/%s\\n\",\n \t\t real_path(get_git_common_dir()), name);\n@@ -923,11 +922,9 @@ static int prepare_linked_checkout(const struct checkout_opts *opts,\n \t * value would do because this value will be ignored and\n \t * replaced at the next (real) checkout.\n \t */\n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%s/HEAD\", sb_repo.buf);\n+\tstrbuf_setf(&sb, \"%s/HEAD\", sb_repo.buf);\n \twrite_file(sb.buf, 1, \"%s\\n\", sha1_to_hex(new->commit->object.sha1));\n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%s/commondir\", sb_repo.buf);\n+\tstrbuf_setf(&sb, \"%s/commondir\", sb_repo.buf);\n \twrite_file(sb.buf, 1, \"../..\\n\");\n \n \tif (!opts->quiet)\n@@ -942,8 +939,7 @@ static int prepare_linked_checkout(const struct checkout_opts *opts,\n \tret = run_command(&cp);\n \tif (!ret)\n \t\tis_junk = 0;\n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%s/locked\", sb_repo.buf);\n+\tstrbuf_setf(&sb, \"%s/locked\", sb_repo.buf);\n \tunlink_or_warn(sb.buf);\n \tstrbuf_release(&sb);\n \tstrbuf_release(&sb_repo);\n@@ -1048,8 +1044,7 @@ static void check_linked_checkouts(struct branch_info *new)\n \t\treturn;\n \t}\n \n-\tstrbuf_reset(&path);\n-\tstrbuf_addf(&path, \"%s/HEAD\", get_git_common_dir());\n+\tstrbuf_setf(&path, \"%s/HEAD\", get_git_common_dir());\n \t/*\n \t * $GIT_COMMON_DIR/HEAD is practically outside\n \t * $GIT_DIR so resolve_ref_unsafe() won't work (it\n@@ -1064,8 +1059,7 @@ static void check_linked_checkouts(struct branch_info *new)\n \twhile ((d = readdir(dir)) != NULL) {\n \t\tif (!strcmp(d->d_name, \".\") || !strcmp(d->d_name, \"..\"))\n \t\t\tcontinue;\n-\t\tstrbuf_reset(&path);\n-\t\tstrbuf_addf(&path, \"%s/repos/%s/HEAD\",\n+\t\tstrbuf_setf(&path, \"%s/repos/%s/HEAD\",\n \t\t\t get_git_common_dir(), d->d_name);\n \t\tif (check_linked_checkout(new, d->d_name, path.buf))\n \t\t\tbreak;\n-- \n2.0.0.592.gf55b190\n"},{"id":"243659","messageId":"ec9b2997b3a74581ec59af6354d276181b5481f5.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 08/19] builtin/mailinfo: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:27Z","receivedAt":"2014-06-09T22:19:27Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n builtin/mailinfo.c | 18 ++++++------------\n 1 file changed, 6 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex cf11c8d..596071e 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -40,8 +40,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf *name, struct strbuf\n \t\tsrc = email;\n \telse if (name == out)\n \t\treturn;\n-\tstrbuf_reset(out);\n-\tstrbuf_addbuf(out, src);\n+\tstrbuf_setbuf(out, src);\n }\n \n static void parse_bogus_from(const struct strbuf *line)\n@@ -62,11 +61,9 @@ static void parse_bogus_from(const struct strbuf *line)\n \tif (!ket)\n \t\treturn;\n \n-\tstrbuf_reset(&email);\n-\tstrbuf_add(&email, bra + 1, ket - bra - 1);\n+\tstrbuf_set(&email, bra + 1, ket - bra - 1);\n \n-\tstrbuf_reset(&name);\n-\tstrbuf_add(&name, line->buf, bra - line->buf);\n+\tstrbuf_set(&name, line->buf, bra - line->buf);\n \tstrbuf_trim(&name);\n \tget_sane_name(&name, &name, &email);\n }\n@@ -108,8 +105,7 @@ static void handle_from(const struct strbuf *from)\n \t\tat--;\n \t}\n \tel = strcspn(at, \" \\n\\t\\r\\v\\f>\");\n-\tstrbuf_reset(&email);\n-\tstrbuf_add(&email, at, el);\n+\tstrbuf_set(&email, at, el);\n \tstrbuf_remove(&f, at - f.buf, el + (at[el] ? 1 : 0));\n \n \t/* The remainder is name. It could be\n@@ -567,8 +563,7 @@ static int decode_header_bq(struct strbuf *it)\n \t\tin = ep + 2;\n \t}\n \tstrbuf_addstr(&outbuf, in);\n-\tstrbuf_reset(it);\n-\tstrbuf_addbuf(it, &outbuf);\n+\tstrbuf_setbuf(it, &outbuf);\n decode_header_bq_out:\n \tstrbuf_release(&outbuf);\n \tstrbuf_release(&charset_q);\n@@ -602,8 +597,7 @@ static void decode_transfer_encoding(struct strbuf *line)\n \tdefault:\n \t\treturn;\n \t}\n-\tstrbuf_reset(line);\n-\tstrbuf_addbuf(line, ret);\n+\tstrbuf_setbuf(line, ret);\n \tstrbuf_release(ret);\n \tfree(ret);\n }\n-- \n2.0.0.592.gf55b190\n"},{"id":"243660","messageId":"2931e950153369cbdc6ee583cd987129dd608726.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 09/19] builtin/tag: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:28Z","receivedAt":"2014-06-09T22:19:28Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n builtin/tag.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex c6e8a71..b545c21 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -496,8 +496,7 @@ static int strbuf_check_tag_ref(struct strbuf *sb, const char *name)\n \tif (name[0] == '-')\n \t\treturn -1;\n \n-\tstrbuf_reset(sb);\n-\tstrbuf_addf(sb, \"refs/tags/%s\", name);\n+\tstrbuf_setf(sb, \"refs/tags/%s\", name);\n \n \treturn check_refname_format(sb->buf, 0);\n }\n-- \n2.0.0.592.gf55b190\n"},{"id":"243661","messageId":"a4423dad33c44ba6badc9b44018a5de0ef4f5995.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 10/19] date: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:29Z","receivedAt":"2014-06-09T22:19:29Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n date.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 782de95..0b723a4 100644\n--- a/date.c\n+++ b/date.c\n@@ -166,8 +166,7 @@ const char *show_date(unsigned long time, int tz, enum date_mode mode)\n \tstatic struct strbuf timebuf = STRBUF_INIT;\n \n \tif (mode == DATE_RAW) {\n-\t\tstrbuf_reset(&timebuf);\n-\t\tstrbuf_addf(&timebuf, \"%lu %+05d\", time, tz);\n+\t\tstrbuf_setf(&timebuf, \"%lu %+05d\", time, tz);\n \t\treturn timebuf.buf;\n \t}\n \n-- \n2.0.0.592.gf55b190\n"},{"id":"243662","messageId":"20febeddeb08ba7c811731d2f9a416716d78bf55.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 11/19] diffcore-order: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:30Z","receivedAt":"2014-06-09T22:19:30Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n diffcore-order.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/diffcore-order.c b/diffcore-order.c\nindex 97dd3d0..5a93971 100644\n--- a/diffcore-order.c\n+++ b/diffcore-order.c\n@@ -63,8 +63,7 @@ static int match_order(const char *path)\n \tstatic struct strbuf p = STRBUF_INIT;\n \n \tfor (i = 0; i < order_cnt; i++) {\n-\t\tstrbuf_reset(&p);\n-\t\tstrbuf_addstr(&p, path);\n+\t\tstrbuf_setstr(&p, path);\n \t\twhile (p.buf[0]) {\n \t\t\tchar *cp;\n \t\t\tif (!wildmatch(order[i], p.buf, 0, NULL))\n-- \n2.0.0.592.gf55b190\n"},{"id":"243663","messageId":"1b2452210b15aedeb3006ae8952220086f9420e5.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 12/19] http-backend: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:31Z","receivedAt":"2014-06-09T22:19:31Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n http-backend.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex d2c0a62..25c7435 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -489,14 +489,12 @@ static void service_rpc(char *service_name)\n \tstruct rpc_service *svc = select_service(service_name);\n \tstruct strbuf buf = STRBUF_INIT;\n \n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"application/x-git-%s-request\", svc->name);\n+\tstrbuf_setf(&buf, \"application/x-git-%s-request\", svc->name);\n \tcheck_content_type(buf.buf);\n \n \thdr_nocache();\n \n-\tstrbuf_reset(&buf);\n-\tstrbuf_addf(&buf, \"application/x-git-%s-result\", svc->name);\n+\tstrbuf_setf(&buf, \"application/x-git-%s-result\", svc->name);\n \thdr_str(content_type, buf.buf);\n \n \tend_headers();\n-- \n2.0.0.592.gf55b190\n"},{"id":"243664","messageId":"72828866118c7e86e07a5cbaf4b69f17d8a71d1c.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 13/19] http: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:32Z","receivedAt":"2014-06-09T22:19:32Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n http.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 2b4f6a3..626fed7 100644\n--- a/http.c\n+++ b/http.c\n@@ -1098,8 +1098,7 @@ static int update_url_from_redirect(struct strbuf *base,\n \t strcmp(tail, got->buf + got->len - tail_len))\n \t\treturn 0; /* insane redirect scheme */\n \n-\tstrbuf_reset(base);\n-\tstrbuf_add(base, got->buf, got->len - tail_len);\n+\tstrbuf_set(base, got->buf, got->len - tail_len);\n \treturn 1;\n }\n \n-- \n2.0.0.592.gf55b190\n"},{"id":"243665","messageId":"a2787757af5e33b84346d1ebd2ae7084682a086e.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 14/19] ident: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:33Z","receivedAt":"2014-06-09T22:19:33Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n ident.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/ident.c b/ident.c\nindex 1d9b6e7..523e249 100644\n--- a/ident.c\n+++ b/ident.c\n@@ -397,8 +397,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \tif (!strcmp(var, \"user.name\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tstrbuf_reset(&git_default_name);\n-\t\tstrbuf_addstr(&git_default_name, value);\n+\t\tstrbuf_setstr(&git_default_name, value);\n \t\tcommitter_ident_explicitly_given |= IDENT_NAME_GIVEN;\n \t\tauthor_ident_explicitly_given |= IDENT_NAME_GIVEN;\n \t\treturn 0;\n@@ -407,8 +406,7 @@ int git_ident_config(const char *var, const char *value, void *data)\n \tif (!strcmp(var, \"user.email\")) {\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n-\t\tstrbuf_reset(&git_default_email);\n-\t\tstrbuf_addstr(&git_default_email, value);\n+\t\tstrbuf_setstr(&git_default_email, value);\n \t\tcommitter_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\tauthor_ident_explicitly_given |= IDENT_MAIL_GIVEN;\n \t\treturn 0;\n-- \n2.0.0.592.gf55b190\n"},{"id":"243666","messageId":"f455f552910af710f2766d272db61ae62e4a7b8f.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 15/19] remote-curl: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:34Z","receivedAt":"2014-06-09T22:19:34Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n remote-curl.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 4493b38..49d27f5 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -288,8 +288,7 @@ static struct discovery* discover_refs(const char *service, int for_push)\n \t\t */\n \t\tline = packet_read_line_buf(&last->buf, &last->len, NULL);\n \n-\t\tstrbuf_reset(&exp);\n-\t\tstrbuf_addf(&exp, \"# service=%s\", service);\n+\t\tstrbuf_setf(&exp, \"# service=%s\", service);\n \t\tif (strcmp(line, exp.buf))\n \t\t\tdie(\"invalid server response; got '%s'\", line);\n \t\tstrbuf_release(&exp);\n-- \n2.0.0.592.gf55b190\n"},{"id":"243667","messageId":"95dbf96f18d28535eef05cd384656cb5122a37d7.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 16/19] submodule: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:35Z","receivedAt":"2014-06-09T22:19:35Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n submodule.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 3402af6..878cc48 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1324,13 +1324,12 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir)\n \tconst char *real_work_tree = xstrdup(real_path(work_tree));\n \n \t/* Update gitfile */\n-\tstrbuf_addf(&file_name, \"%s/.git\", work_tree);\n+\tstrbuf_setf(&file_name, \"%s/.git\", work_tree);\n \twrite_file(file_name.buf, 1, \"gitdir: %s\\n\",\n \t\t relative_path(real_git_dir, real_work_tree, &rel_path));\n \n \t/* Update core.worktree setting */\n-\tstrbuf_reset(&file_name);\n-\tstrbuf_addf(&file_name, \"%s/config\", real_git_dir);\n+\tstrbuf_setf(&file_name, \"%s/config\", real_git_dir);\n \tif (git_config_set_in_file(file_name.buf, \"core.worktree\",\n \t\t\t\t relative_path(real_work_tree, real_git_dir,\n \t\t\t\t\t\t &rel_path)))\n-- \n2.0.0.592.gf55b190\n"},{"id":"243668","messageId":"090cdfb1c698b40b55828793c031f3723f2cb216.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 17/19] transport: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:36Z","receivedAt":"2014-06-09T22:19:36Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n transport.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 172b3d8..e8f5dfa 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1092,8 +1092,7 @@ static int run_pre_push_hook(struct transport *transport,\n \t\tif (r->status == REF_STATUS_REJECT_STALE) continue;\n \t\tif (r->status == REF_STATUS_UPTODATE) continue;\n \n-\t\tstrbuf_reset(&buf);\n-\t\tstrbuf_addf( &buf, \"%s %s %s %s\\n\",\n+\t\tstrbuf_setf( &buf, \"%s %s %s %s\\n\",\n \t\t\t r->peer_ref->name, sha1_to_hex(r->new_sha1),\n \t\t\t r->name, sha1_to_hex(r->old_sha1));\n \n-- \n2.0.0.592.gf55b190\n"},{"id":"243669","messageId":"1ec2ae837728a223871090fe386bdfc99da84aba.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 18/19] vcs-svn/svndump: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:37Z","receivedAt":"2014-06-09T22:19:37Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n vcs-svn/svndump.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c\nindex 31d1d83..7fbf5d8 100644\n--- a/vcs-svn/svndump.c\n+++ b/vcs-svn/svndump.c\n@@ -352,8 +352,7 @@ void svndump_read(const char *url, const char *local_ref, const char *notes_ref)\n \t\tcase sizeof(\"UUID\"):\n \t\t\tif (constcmp(t, \"UUID\"))\n \t\t\t\tcontinue;\n-\t\t\tstrbuf_reset(&dump_ctx.uuid);\n-\t\t\tstrbuf_addstr(&dump_ctx.uuid, val);\n+\t\t\tstrbuf_setstr(&dump_ctx.uuid, val);\n \t\t\tbreak;\n \t\tcase sizeof(\"Revision-number\"):\n \t\t\tif (constcmp(t, \"Revision-number\"))\n-- \n2.0.0.592.gf55b190\n"},{"id":"243670","messageId":"f55b190bd776a8b2b0dd248de73c04071fa26fd2.1402348696.git.jmmahler@gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"[PATCH v2 19/19] wt-status: simplify via strbuf_set()","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:19:38Z","receivedAt":"2014-06-09T22:19:38Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplify cases where a strbuf_reset is immediately followed by a\nstrbuf_add by using strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler \n---\n wt-status.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/wt-status.c b/wt-status.c\nindex 318a191..a89cd73 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1168,8 +1168,7 @@ static char *read_and_strip_branch(const char *path)\n \telse if (!get_sha1_hex(sb.buf, sha1)) {\n \t\tconst char *abbrev;\n \t\tabbrev = find_unique_abbrev(sha1, DEFAULT_ABBREV);\n-\t\tstrbuf_reset(&sb);\n-\t\tstrbuf_addstr(&sb, abbrev);\n+\t\tstrbuf_setstr(&sb, abbrev);\n \t} else if (!strcmp(sb.buf, \"detached HEAD\")) /* rebase */\n \t\tgoto got_nothing;\n \telse\t\t\t/* bisect */\n-- \n2.0.0.592.gf55b190\n"},{"id":"243882","messageId":"CAPig+cRJJ6byQi6+=+8r3Ohyt9FvMHwQViq+MNR1DcD2t8w+NQ@mail.gmail.com","threadId":"36875","inReplyTo":"cover.1402348696.git.jmmahler@gmail.com","subject":"Re: [PATCH v2 00/19] add strbuf_set operations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-11T08:09:17Z","receivedAt":"2014-06-11T08:09:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 9, 2014 at 6:19 PM, Jeremiah Mahler wrote:\n> Version 2 of the patch set to add strbuf_set operations.\n>\n> Includes suggestions from Eric Sunshine [1]:\n>\n> [...snip...]\n>\n> Using strbuf_set before a sequence of adds can be confusing. But using\n> strbuf_add when nothing important was being added to can also be\n> confusing. strbuf_set can be used to make these cases more clear.\n>\n> struct strbuf mybuf = STRBUF_INIT;\n>\n> strbuf_add(&mybuf, ...); /* Was something there before? */\n>\n> strbuf_set(&mybuf, ...); /* Replace everything. */\n>\n> Several single line replacements were made for this reason.\n>\n> Additional files have also been converted compared to version 1 [1].\n>\n> [1]: http://marc.info/?l=git&m=140230874124060&w=2\n\nFood for thought: This patch series has now likely entered the\nterritory of unnecessary code churn. Quoting from\nDocumentation/CodingGuidelines:\n\n Fixing style violations while working on a real change as a\n preparatory clean-up step is good, but otherwise avoid useless\n code churn for the sake of conforming to the style.\n\n \"Once it _is_ in the tree, it's not really worth the patch noise\n to go and fix it up.\"\n Cf. http://article.gmane.org/gmane.linux.kernel/943020\n\nRewriting code merely for the sake of replacing strbuf_reset() +\nstrbuf_add() with strbuf_set(), while not exactly a style cleanup,\nfalls into the same camp. (The patch which introduces strbuf_set(),\nhowever, is not churn.) Each change you make can potentially conflict\nwith other topics already in-flight (see [1], for example), thus\nmaking more work for Junio.\n\nAlso, these sorts of apparently innocuous \"mechanical\" changes can\nhave hidden costs [2], so it's useful to weigh carefully how lengthy\nyou want to make this patch series.\n\nHaving said that, patch 4/19, particularly the changes to\nbuiltin/remote.c:mv(), in which multiple strbufs are reused\nrepeatedly, does seem to make the code a bit easier to read and likely\nreduces cognitive load.\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/245133/focus=245147\n[2]: http://thread.gmane.org/gmane.comp.version-control.git/245133/focus=245144\n\n> Jeremiah Mahler (19):\n> add strbuf_set operations\n> sha1_name: simplify via strbuf_set()\n> fast-import: simplify via strbuf_set()\n> builtin/remote: simplify via strbuf_set()\n> branch: simplify via strbuf_set()\n> builtin/branch: simplify via strbuf_set()\n> builtin/checkout: simplify via strbuf_set()\n> builtin/mailinfo: simplify via strbuf_set()\n> builtin/tag: simplify via strbuf_set()\n> date: simplify via strbuf_set()\n> diffcore-order: simplify via strbuf_set()\n> http-backend: simplify via strbuf_set()\n> http: simplify via strbuf_set()\n> ident: simplify via strbuf_set()\n> remote-curl: simplify via strbuf_set()\n> submodule: simplify via strbuf_set()\n> transport: simplify via strbuf_set()\n> vcs-svn/svndump: simplify via strbuf_set()\n> wt-status: simplify via strbuf_set()\n>\n> Documentation/technical/api-strbuf.txt | 18 +++++++++++\n> branch.c | 6 ++--\n> builtin/branch.c | 3 +-\n> builtin/checkout.c | 18 ++++-------\n> builtin/mailinfo.c | 18 ++++-------\n> builtin/remote.c | 59 ++++++++++++----------------------\n> builtin/tag.c | 3 +-\n> date.c | 3 +-\n> diffcore-order.c | 3 +-\n> fast-import.c | 6 ++--\n> http-backend.c | 6 ++--\n> http.c | 3 +-\n> ident.c | 6 ++--\n> remote-curl.c | 3 +-\n> sha1_name.c | 15 +++------\n> strbuf.c | 21 ++++++++++++\n> strbuf.h | 14 ++++++++\n> submodule.c | 5 ++-\n> transport.c | 3 +-\n> vcs-svn/svndump.c | 3 +-\n> wt-status.c | 3 +-\n> 21 files changed, 110 insertions(+), 109 deletions(-)\n>\n> --\n> 2.0.0.592.gf55b190\n"},{"id":"243884","messageId":"CAPig+cTY22Or8vtZQpP5aahJzBs3wKmdX0tuxEpt0sswfp94bA@mail.gmail.com","threadId":"36875","inReplyTo":"6fe33498512fc2ca1678517e51dc2e94a4260ff4.1402348696.git.jmmahler@gmail.com","subject":"Re: [PATCH v2 01/19] add strbuf_set operations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-11T08:42:56Z","receivedAt":"2014-06-11T08:42:56Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 9, 2014 at 6:19 PM, Jeremiah Mahler wrote:\n> Currently, the data in a strbuf is modified using add operations. To\n> set the buffer to some data a reset must be performed before an add.\n>\n> strbuf_reset(buf);\n> strbuf_add(buf, cb.buf.buf, cb.buf.len);\n>\n> And this is a common sequence of operations with 70 occurrences found in\n> the current source code. This includes all the different variations\n> (add, addf, addstr, addbuf, addch).\n>\n> FILES=`find ./ -name '*.c'`\n> CNT=$(pcregrep -M \"strbuf_reset.*\\n.*strbuf_add\" $FILES | wc -l)\n> CNT=$(echo \"$CNT / 2\" | bc)\n> echo $CNT\n> 70\n>\n> These patches add strbuf_set operations which allow this common sequence\n> to be performed in one line instead of two.\n>\n> strbuf_set(buf, cb.buf.buf, cb.buf.len);\n\nThis commit message is effectively the cover letter for the entire\npatch series; it doesn't say specifically what _this_ patch is doing.\n\nJustification for the change could be stronger. Rather than merely\npointing out that a sequence of operations occurs N times in the\nproject, explain why strbuf_set() is superior. For instance, you might\nsay something about how strbuf_set() conveys the operation being\nperformed more concisely and clearly than strbuf_reset() +\nstrbuf_add() (and thus may reduce cognitive load, though that's\nsubjective). The bit about performing the operation in one line\ninstead of two is minor, at best, and may not be worth mentioning at\nall (since it's implied).\n\nIt's also redundant to say \"this patch\" in the commit message, thus\nshould be avoided.\n\nMore below.\n\n> Signed-off-by: Jeremiah Mahler \n> ---\n> Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n> strbuf.c | 21 +++++++++++++++++++++\n> strbuf.h | 14 ++++++++++++++\n> 3 files changed, 53 insertions(+)\n>\n> diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\n> index 077a709..b7e23da 100644\n> --- a/Documentation/technical/api-strbuf.txt\n> +++ b/Documentation/technical/api-strbuf.txt\n> @@ -149,6 +149,24 @@ Functions\n> than zero if the first buffer is found, respectively, to be less than,\n> to match, or be greater than the second buffer.\n>\n> +* Setting the buffer\n> +\n> +`strbuf_set`::\n> +\n> + Replace the buffer content with data of a given length.\n\nI know that you copied the wording I suggested in my v1 review, but\nnow that I see it in context, I find the redundancy level rather high.\nThe header already says \"Setting the buffer\", so repeating \"the\nbuffer\" in each function description doesn't add much. It might make\nsense to reword as:\n\n Replace content with [...].\n\nMore below.\n\n> +`strbuf_setstr`::\n> +\n> + Replace the buffer content with data from a NUL-terminated string.\n> +\n> +`strbuf_setf`::\n> +\n> + Replace the buffer content with a formatted string.\n> +\n> +`strbuf_setbuf`::\n> +\n> + Replace the buffer content with data from another buffer.\n> +\n> * Adding data to the buffer\n>\n> NOTE: All of the functions in this section will grow the buffer as necessary.\n> diff --git a/strbuf.c b/strbuf.c\n> index ac62982..9d64b00 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n> strbuf_setlen(sb, sb->len + dlen - len);\n> }\n>\n> +void strbuf_set(struct strbuf *sb, const void *data, size_t len)\n> +{\n> + strbuf_reset(sb);\n> + strbuf_add(sb, data, len);\n> +}\n> +\n> +void strbuf_setf(struct strbuf *sb, const char *fmt, ...)\n> +{\n> + va_list ap;\n> + strbuf_reset(sb);\n> + va_start(ap, fmt);\n> + strbuf_vaddf(sb, fmt, ap);\n> + va_end(ap);\n> +}\n> +\n> +void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)\n> +{\n> + strbuf_reset(sb);\n> + strbuf_add(sb, sb2->buf, sb2->len);\n> +}\n> +\n> void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)\n> {\n> strbuf_splice(sb, pos, 0, data, len);\n> diff --git a/strbuf.h b/strbuf.h\n> index e9ad03e..b339f08 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -101,6 +101,20 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,\n> */\n> extern void strbuf_list_free(struct strbuf **);\n>\n> +/*----- set buffer to data -----*/\n> +\n\nMinor: Existing divider lines in this header are not followed by a blank line.\n\n> +extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);\n> +\n> +static inline void strbuf_setstr(struct strbuf *sb, const char *s)\n> +{\n> + strbuf_set(sb, s, strlen(s));\n> +}\n> +\n> +__attribute__((format (printf,2,3)))\n> +extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);\n> +\n> +extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);\n> +\n> /*----- add data in your buffer -----*/\n> static inline void strbuf_addch(struct strbuf *sb, int c)\n> {\n> --\n> 2.0.0.592.gf55b190\n\nFinal word: The comments in this review do not necessarily require a\nre-roll. Junio may or may not want to pick up the series as is. If he\ndoesn't, or if you want to polish it further, then perhaps take the\nreview comments into consideration when rerolling.\n"},{"id":"243904","messageId":"5398460F.3040900@alum.mit.edu","threadId":"36875","inReplyTo":"6fe33498512fc2ca1678517e51dc2e94a4260ff4.1402348696.git.jmmahler@gmail.com","subject":"Re: [PATCH v2 01/19] add strbuf_set operations","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-06-11T12:05:35Z","receivedAt":"2014-06-11T12:05:35Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/10/2014 12:19 AM, Jeremiah Mahler wrote:\n> Currently, the data in a strbuf is modified using add operations. To\n> set the buffer to some data a reset must be performed before an add.\n> \n> strbuf_reset(buf);\n> strbuf_add(buf, cb.buf.buf, cb.buf.len);\n> \n> And this is a common sequence of operations with 70 occurrences found in\n> the current source code. This includes all the different variations\n> (add, addf, addstr, addbuf, addch).\n> \n> FILES=`find ./ -name '*.c'`\n> CNT=$(pcregrep -M \"strbuf_reset.*\\n.*strbuf_add\" $FILES | wc -l)\n> CNT=$(echo \"$CNT / 2\" | bc)\n> echo $CNT\n> 70\n> \n> These patches add strbuf_set operations which allow this common sequence\n> to be performed in one line instead of two.\n> \n> strbuf_set(buf, cb.buf.buf, cb.buf.len);\n> \n> Signed-off-by: Jeremiah Mahler \n> ---\n> Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n> strbuf.c | 21 +++++++++++++++++++++\n> strbuf.h | 14 ++++++++++++++\n> 3 files changed, 53 insertions(+)\n> \n> diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\n> index 077a709..b7e23da 100644\n> --- a/Documentation/technical/api-strbuf.txt\n> +++ b/Documentation/technical/api-strbuf.txt\n> @@ -149,6 +149,24 @@ Functions\n> \tthan zero if the first buffer is found, respectively, to be less than,\n> \tto match, or be greater than the second buffer.\n> \n> +* Setting the buffer\n> +\n> +`strbuf_set`::\n> +\n> +\tReplace the buffer content with data of a given length.\n> +\n> +`strbuf_setstr`::\n> +\n> +\tReplace the buffer content with data from a NUL-terminated string.\n> +\n> +`strbuf_setf`::\n> +\n> +\tReplace the buffer content with a formatted string.\n> +\n> +`strbuf_setbuf`::\n> +\n> +\tReplace the buffer content with data from another buffer.\n> +\n> * Adding data to the buffer\n> \n> NOTE: All of the functions in this section will grow the buffer as necessary.\n> diff --git a/strbuf.c b/strbuf.c\n> index ac62982..9d64b00 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -189,6 +189,27 @@ 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_set(struct strbuf *sb, const void *data, size_t len)\n> +{\n> +\tstrbuf_reset(sb);\n> +\tstrbuf_add(sb, data, len);\n> +}\n> +\n\nI never know how much intelligence to attribute to modern compilers, but\nit seems like even after optimization this function will be doing more\nwork than necessary:\n\n strbuf_reset(sb)\n -> strbuf_setlen(sb, 0)\n -> sb->len = 0\n -> sb->buf[len] = '\\0'\n -> strbuf_add(sb, data, len)\n -> strbuf_grow(sb, len)\n -> ...lots of stuff...\n -> memcpy(sb->buf + sb->len, data, len)\n -> strbuf_setlen(sb, sb->len + len)\n -> sb->len = len\n -> sb->buf[len] = '\\0'\n\nIf there were a function like strbuf_grow_to(sb, len):\n\nvoid strbuf_grow_to(struct strbuf *sb, size_t len)\n{\n\tint new_buf = !sb->alloc;\n\tif (unsigned_add_overflows(len, 1))\n\t\tdie(\"you want to use way too much memory\");\n\tif (new_buf)\n\t\tsb->buf = NULL;\n\tALLOC_GROW(sb->buf, len + 1, sb->alloc);\n\tif (new_buf)\n\t\tsb->buf[0] = '\\0';\n}\n\n(strbuf_grow() could call it:\n\nstatic inline void strbuf_grow(struct strbuf *sb, size_t extra)\n{\n\tif (unsigned_add_overflows(sb->len, extra))\n\t\tdie(\"you want to use way too much memory\");\n\tstrbuf_grow_to(sb, sb->len + extra);\n}\n\n), then your function could be minimized to\n\nvoid strbuf_set(struct strbuf *sb, const void *data, size_t len)\n{\n\tstrbuf_grow_to(sb, len);\n\tmemcpy(sb->buf, data, len);\n\tstrbuf_setlen(sb, len);\n}\n\nI think strbuf_grow_to() would be useful in other situations too.\n\nThis is just an idea; I'm not saying that you have to make this change.\n\n> [...]\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"243934","messageId":"20140612070932.GA25353@hudson.localdomain","threadId":"36875","inReplyTo":"CAPig+cRJJ6byQi6+=+8r3Ohyt9FvMHwQViq+MNR1DcD2t8w+NQ@mail.gmail.com","subject":"Re: [PATCH v2 00/19] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-12T07:09:32Z","receivedAt":"2014-06-12T07:09:32Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Eric,\n\nOn Wed, Jun 11, 2014 at 04:09:17AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 9, 2014 at 6:19 PM, Jeremiah Mahler wrote:\n> > Version 2 of the patch set to add strbuf_set operations.\n> >\n> > Includes suggestions from Eric Sunshine [1]:\n> >\n> > [...snip...]\n> >\n...\n> \n> Food for thought: This patch series has now likely entered the\n> territory of unnecessary code churn. Quoting from\n> Documentation/CodingGuidelines:\n> \n> Fixing style violations while working on a real change as a\n> preparatory clean-up step is good, but otherwise avoid useless\n> code churn for the sake of conforming to the style.\n> \n> \"Once it _is_ in the tree, it's not really worth the patch noise\n> to go and fix it up.\"\n> Cf. http://article.gmane.org/gmane.linux.kernel/943020\n> \n> Rewriting code merely for the sake of replacing strbuf_reset() +\n> strbuf_add() with strbuf_set(), while not exactly a style cleanup,\n> falls into the same camp. (The patch which introduces strbuf_set(),\n> however, is not churn.) Each change you make can potentially conflict\n> with other topics already in-flight (see [1], for example), thus\n> making more work for Junio.\n> \n> Also, these sorts of apparently innocuous \"mechanical\" changes can\n> have hidden costs [2], so it's useful to weigh carefully how lengthy\n> you want to make this patch series.\n> \n> Having said that, patch 4/19, particularly the changes to\n> builtin/remote.c:mv(), in which multiple strbufs are reused\n> repeatedly, does seem to make the code a bit easier to read and likely\n> reduces cognitive load.\n> \n> [1]: http://thread.gmane.org/gmane.comp.version-control.git/245133/focus=245147\n> [2]: http://thread.gmane.org/gmane.comp.version-control.git/245133/focus=245144\n> \n\nI did not realize this \"code churn\" aspect at the time but it makes\nsense now. These \"mechanical\" changes can be more trouble than they are\nworth.\n\nI will try to re-focus my changes to only those that have a substantial\nbenefit.\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"243935","messageId":"20140612071003.GB25353@hudson.localdomain","threadId":"36875","inReplyTo":"CAPig+cTY22Or8vtZQpP5aahJzBs3wKmdX0tuxEpt0sswfp94bA@mail.gmail.com","subject":"Re: [PATCH v2 01/19] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-12T07:10:03Z","receivedAt":"2014-06-12T07:10:03Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"On Wed, Jun 11, 2014 at 04:42:56AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 9, 2014 at 6:19 PM, Jeremiah Mahler wrote:\n> > Currently, the data in a strbuf is modified using add operations. To\n> > set the buffer to some data a reset must be performed before an add.\n> >\n> > strbuf_reset(buf);\n> > strbuf_add(buf, cb.buf.buf, cb.buf.len);\n> >\n> > And this is a common sequence of operations with 70 occurrences found in\n> > the current source code. This includes all the different variations\n> > (add, addf, addstr, addbuf, addch).\n> >\n> > FILES=`find ./ -name '*.c'`\n> > CNT=$(pcregrep -M \"strbuf_reset.*\\n.*strbuf_add\" $FILES | wc -l)\n> > CNT=$(echo \"$CNT / 2\" | bc)\n> > echo $CNT\n> > 70\n> >\n> > These patches add strbuf_set operations which allow this common sequence\n> > to be performed in one line instead of two.\n> >\n> > strbuf_set(buf, cb.buf.buf, cb.buf.len);\n> \n> This commit message is effectively the cover letter for the entire\n> patch series; it doesn't say specifically what _this_ patch is doing.\n> \n> Justification for the change could be stronger. Rather than merely\n> pointing out that a sequence of operations occurs N times in the\n> project, explain why strbuf_set() is superior. For instance, you might\n> say something about how strbuf_set() conveys the operation being\n> performed more concisely and clearly than strbuf_reset() +\n> strbuf_add() (and thus may reduce cognitive load, though that's\n> subjective). The bit about performing the operation in one line\n> instead of two is minor, at best, and may not be worth mentioning at\n> all (since it's implied).\n> \nAfter reviewing my argument, I think the biggest benefit is that it can\nmake the code more readable in some cases.\n\n> It's also redundant to say \"this patch\" in the commit message, thus\n> should be avoided.\nGot it.\n\n> \n> More below.\n> \n> > Signed-off-by: Jeremiah Mahler \n> > ---\n> > Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n> > strbuf.c | 21 +++++++++++++++++++++\n> > strbuf.h | 14 ++++++++++++++\n> > 3 files changed, 53 insertions(+)\n> >\n> > diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\n> > index 077a709..b7e23da 100644\n> > --- a/Documentation/technical/api-strbuf.txt\n> > +++ b/Documentation/technical/api-strbuf.txt\n> > @@ -149,6 +149,24 @@ Functions\n> > than zero if the first buffer is found, respectively, to be less than,\n> > to match, or be greater than the second buffer.\n> >\n> > +* Setting the buffer\n> > +\n> > +`strbuf_set`::\n> > +\n> > + Replace the buffer content with data of a given length.\n> \n> I know that you copied the wording I suggested in my v1 review, but\n> now that I see it in context, I find the redundancy level rather high.\n> The header already says \"Setting the buffer\", so repeating \"the\n> buffer\" in each function description doesn't add much. It might make\n> sense to reword as:\n> \n> Replace content with [...].\n> \nFixed.\n\n> More below.\n> \n> > +`strbuf_setstr`::\n> > +\n> > + Replace the buffer content with data from a NUL-terminated string.\n> > +\n> > +`strbuf_setf`::\n> > +\n> > + Replace the buffer content with a formatted string.\n> > +\n> > +`strbuf_setbuf`::\n> > +\n> > + Replace the buffer content with data from another buffer.\n> > +\n> > * Adding data to the buffer\n> >\n> > NOTE: All of the functions in this section will grow the buffer as necessary.\n> > diff --git a/strbuf.c b/strbuf.c\n> > index ac62982..9d64b00 100644\n> > --- a/strbuf.c\n> > +++ b/strbuf.c\n> > @@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,\n> > strbuf_setlen(sb, sb->len + dlen - len);\n> > }\n> >\n> > +void strbuf_set(struct strbuf *sb, const void *data, size_t len)\n> > +{\n> > + strbuf_reset(sb);\n> > + strbuf_add(sb, data, len);\n> > +}\n> > +\n> > +void strbuf_setf(struct strbuf *sb, const char *fmt, ...)\n> > +{\n> > + va_list ap;\n> > + strbuf_reset(sb);\n> > + va_start(ap, fmt);\n> > + strbuf_vaddf(sb, fmt, ap);\n> > + va_end(ap);\n> > +}\n> > +\n> > +void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)\n> > +{\n> > + strbuf_reset(sb);\n> > + strbuf_add(sb, sb2->buf, sb2->len);\n> > +}\n> > +\n> > void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)\n> > {\n> > strbuf_splice(sb, pos, 0, data, len);\n> > diff --git a/strbuf.h b/strbuf.h\n> > index e9ad03e..b339f08 100644\n> > --- a/strbuf.h\n> > +++ b/strbuf.h\n> > @@ -101,6 +101,20 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,\n> > */\n> > extern void strbuf_list_free(struct strbuf **);\n> >\n> > +/*----- set buffer to data -----*/\n> > +\n> \n> Minor: Existing divider lines in this header are not followed by a blank line.\nFixed.\n> \n> > +extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);\n> > +\n> > +static inline void strbuf_setstr(struct strbuf *sb, const char *s)\n> > +{\n> > + strbuf_set(sb, s, strlen(s));\n> > +}\n> > +\n> > +__attribute__((format (printf,2,3)))\n> > +extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);\n> > +\n> > +extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);\n> > +\n> > /*----- add data in your buffer -----*/\n> > static inline void strbuf_addch(struct strbuf *sb, int c)\n> > {\n> > --\n> > 2.0.0.592.gf55b190\n> \n> Final word: The comments in this review do not necessarily require a\n> re-roll. Junio may or may not want to pick up the series as is. If he\n> doesn't, or if you want to polish it further, then perhaps take the\n> review comments into consideration when rerolling.\n\nThey are certainly changes worth making.\nThanks again for your review :-)\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"243936","messageId":"20140612071045.GC25353@hudson.localdomain","threadId":"36875","inReplyTo":"5398460F.3040900@alum.mit.edu","subject":"Re: [PATCH v2 01/19] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-12T07:10:45Z","receivedAt":"2014-06-12T07:10:45Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Michael,\n\nOn Wed, Jun 11, 2014 at 02:05:35PM +0200, Michael Haggerty wrote:\n> On 06/10/2014 12:19 AM, Jeremiah Mahler wrote:\n> > Currently, the data in a strbuf is modified using add operations. To\n> > set the buffer to some data a reset must be performed before an add.\n> > \n...\n> > diff --git a/strbuf.c b/strbuf.c\n> > index ac62982..9d64b00 100644\n> > --- a/strbuf.c\n> > +++ b/strbuf.c\n> > @@ -189,6 +189,27 @@ 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_set(struct strbuf *sb, const void *data, size_t len)\n> > +{\n> > +\tstrbuf_reset(sb);\n> > +\tstrbuf_add(sb, data, len);\n> > +}\n> > +\n> \n> I never know how much intelligence to attribute to modern compilers, but\n> it seems like even after optimization this function will be doing more\n> work than necessary:\n> \n> strbuf_reset(sb)\n> -> strbuf_setlen(sb, 0)\n> -> sb->len = 0\n> -> sb->buf[len] = '\\0'\n> -> strbuf_add(sb, data, len)\n> -> strbuf_grow(sb, len)\n> -> ...lots of stuff...\n> -> memcpy(sb->buf + sb->len, data, len)\n> -> strbuf_setlen(sb, sb->len + len)\n> -> sb->len = len\n> -> sb->buf[len] = '\\0'\n> \n> If there were a function like strbuf_grow_to(sb, len):\n> \n> void strbuf_grow_to(struct strbuf *sb, size_t len)\n> {\n> \tint new_buf = !sb->alloc;\n> \tif (unsigned_add_overflows(len, 1))\n> \t\tdie(\"you want to use way too much memory\");\n> \tif (new_buf)\n> \t\tsb->buf = NULL;\n> \tALLOC_GROW(sb->buf, len + 1, sb->alloc);\n> \tif (new_buf)\n> \t\tsb->buf[0] = '\\0';\n> }\n> \ngrow_to() which could reduce in size, interesting.\n\n> (strbuf_grow() could call it:\n> \n> static inline void strbuf_grow(struct strbuf *sb, size_t extra)\n> {\n> \tif (unsigned_add_overflows(sb->len, extra))\n> \t\tdie(\"you want to use way too much memory\");\n> \tstrbuf_grow_to(sb, sb->len + extra);\n> }\n> \n> ), then your function could be minimized to\n> \n> void strbuf_set(struct strbuf *sb, const void *data, size_t len)\n> {\n> \tstrbuf_grow_to(sb, len);\n> \tmemcpy(sb->buf, data, len);\n> \tstrbuf_setlen(sb, len);\n> }\n> \n> I think strbuf_grow_to() would be useful in other situations too.\n> \n> This is just an idea; I'm not saying that you have to make this change.\n> \nI like your idea. I am leaning towards doing the un-optimized\nstrbuf_set operations first, then optimizing in a later patch.\n\n> > [...]\n> \n> Michael\n> \n> -- \n> Michael Haggerty\n> mhagger@alum.mit.edu\n> http://softwareswirl.blogspot.com/\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"243944","messageId":"53997F3C.9010608@alum.mit.edu","threadId":"36875","inReplyTo":"20140612071045.GC25353@hudson.localdomain","subject":"Re: [PATCH v2 01/19] add strbuf_set operations","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-06-12T10:21:48Z","receivedAt":"2014-06-12T10:21:48Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 06/12/2014 09:10 AM, Jeremiah Mahler wrote:\n> On Wed, Jun 11, 2014 at 02:05:35PM +0200, Michael Haggerty wrote:\n>> [...]\n>> If there were a function like strbuf_grow_to(sb, len):\n>>\n>> void strbuf_grow_to(struct strbuf *sb, size_t len)\n>> {\n>> \tint new_buf = !sb->alloc;\n>> \tif (unsigned_add_overflows(len, 1))\n>> \t\tdie(\"you want to use way too much memory\");\n>> \tif (new_buf)\n>> \t\tsb->buf = NULL;\n>> \tALLOC_GROW(sb->buf, len + 1, sb->alloc);\n>> \tif (new_buf)\n>> \t\tsb->buf[0] = '\\0';\n>> }\n>>\n> grow_to() which could reduce in size, interesting.\n\nI don't understand what you mean by \"could reduce in size\". This\nfunction can only grow, never reduce, the size of the strbuf, because\nALLOC_GROW doesn't do anything unless the requested size is larger than\nthe currently-allocated size.\n\n>> (strbuf_grow() could call it:\n>>\n>> static inline void strbuf_grow(struct strbuf *sb, size_t extra)\n>> {\n>> \tif (unsigned_add_overflows(sb->len, extra))\n>> \t\tdie(\"you want to use way too much memory\");\n>> \tstrbuf_grow_to(sb, sb->len + extra);\n>> }\n>>\n>> ), then your function could be minimized to\n>>\n>> void strbuf_set(struct strbuf *sb, const void *data, size_t len)\n>> {\n>> \tstrbuf_grow_to(sb, len);\n>> \tmemcpy(sb->buf, data, len);\n>> \tstrbuf_setlen(sb, len);\n>> }\n>>\n>> I think strbuf_grow_to() would be useful in other situations too.\n>>\n>> This is just an idea; I'm not saying that you have to make this change.\n>>\n> I like your idea. I am leaning towards doing the un-optimized\n> strbuf_set operations first, then optimizing in a later patch.\n\nThat's a good plan.\n\nIn case you're interested, I sketched what the strbuf_grow_to() changes\nmight look like, and also looked for other places in the code where\nstrbuf_grow_to() could be used instead of strbuf_grow() [1]. This is\nonly a sketch; I haven't even tested the result. Feel free to use what\nyou want from it.\n\nMichael\n\n[1] Branch \"strbuf-grow-to-sketch\" on https://github.com/mhagger/git\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"244045","messageId":"20140612235953.GC17803@hudson.localdomain","threadId":"36875","inReplyTo":"53997F3C.9010608@alum.mit.edu","subject":"Re: [PATCH v2 01/19] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-12T23:59:53Z","receivedAt":"2014-06-12T23:59:53Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Michael,\n\nOn Thu, Jun 12, 2014 at 12:21:48PM +0200, Michael Haggerty wrote:\n> On 06/12/2014 09:10 AM, Jeremiah Mahler wrote:\n> > On Wed, Jun 11, 2014 at 02:05:35PM +0200, Michael Haggerty wrote:\n> >> [...]\n> >> If there were a function like strbuf_grow_to(sb, len):\n> >>\n> >> void strbuf_grow_to(struct strbuf *sb, size_t len)\n> >> {\n> >> \tint new_buf = !sb->alloc;\n> >> \tif (unsigned_add_overflows(len, 1))\n> >> \t\tdie(\"you want to use way too much memory\");\n> >> \tif (new_buf)\n> >> \t\tsb->buf = NULL;\n> >> \tALLOC_GROW(sb->buf, len + 1, sb->alloc);\n> >> \tif (new_buf)\n> >> \t\tsb->buf[0] = '\\0';\n> >> }\n> >>\n> > grow_to() which could reduce in size, interesting.\n> \n> I don't understand what you mean by \"could reduce in size\". This\n> function can only grow, never reduce, the size of the strbuf, because\n> ALLOC_GROW doesn't do anything unless the requested size is larger than\n> the currently-allocated size.\n> \nI got it now. strbuf_grow() adds extra to the current buf.\nstrbuf_grow_to() will add more if len is larger than the current length.\n\n> >> (strbuf_grow() could call it:\n> >>\n> >> static inline void strbuf_grow(struct strbuf *sb, size_t extra)\n> >> {\n> >> \tif (unsigned_add_overflows(sb->len, extra))\n> >> \t\tdie(\"you want to use way too much memory\");\n> >> \tstrbuf_grow_to(sb, sb->len + extra);\n> >> }\n> >>\n> >> ), then your function could be minimized to\n> >>\n> >> void strbuf_set(struct strbuf *sb, const void *data, size_t len)\n> >> {\n> >> \tstrbuf_grow_to(sb, len);\n> >> \tmemcpy(sb->buf, data, len);\n> >> \tstrbuf_setlen(sb, len);\n> >> }\n> >>\n> >> I think strbuf_grow_to() would be useful in other situations too.\n> >>\n> >> This is just an idea; I'm not saying that you have to make this change.\n> >>\n> > I like your idea. I am leaning towards doing the un-optimized\n> > strbuf_set operations first, then optimizing in a later patch.\n> \n> That's a good plan.\n> \n> In case you're interested, I sketched what the strbuf_grow_to() changes\n> might look like, and also looked for other places in the code where\n> strbuf_grow_to() could be used instead of strbuf_grow() [1]. This is\n> only a sketch; I haven't even tested the result. Feel free to use what\n> you want from it.\n> \nI started looking at it. I will let you know if I get something\ntogether.\n\n> Michael\n> \n> [1] Branch \"strbuf-grow-to-sketch\" on https://github.com/mhagger/git\n> \n> -- \n> Michael Haggerty\n> mhagger@alum.mit.edu\n> http://softwareswirl.blogspot.com/\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"}]}