{"thread":{"id":"36871","subject":"[PATCH/RFC v1 0/5] add strbuf_set operations","startedAt":"2014-06-09T08:36:24Z","lastAt":"2014-06-09T22:06:41Z","messageCount":12,"participants":["Jeremiah Mahler","Eric Sunshine","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"243609","messageId":"cover.1402301815.git.jmmahler@gmail.com","threadId":"36871","inReplyTo":null,"subject":"[PATCH/RFC v1 0/5] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T08:36:24Z","receivedAt":"2014-06-09T08:36:24Z","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\nOnly the first few files have been converted in this preliminary patch set.\n\nJeremiah Mahler (5):\n  add strbuf_set operations\n  add strbuf_set operations documentation\n  sha1_name.c: cleanup using strbuf_set operations\n  fast-import.c: cleanup using strbuf_set operations\n  builtin/remote.c: cleanup using strbuf_set operations\n\n Documentation/technical/api-strbuf.txt | 18 ++++++++++++\n builtin/remote.c                       | 51 ++++++++++++----------------------\n fast-import.c                          | 19 ++++---------\n sha1_name.c                            | 15 ++++------\n strbuf.c                               | 21 ++++++++++++++\n strbuf.h                               | 14 ++++++++++\n 6 files changed, 81 insertions(+), 57 deletions(-)\n\n-- \n2.0.0.573.ged771ce.dirty\n"},{"id":"243610","messageId":"25ef7350029a96c02fc4450c981728d9746d0324.1402301816.git.jmmahler@gmail.com","threadId":"36871","inReplyTo":"cover.1402301815.git.jmmahler@gmail.com","subject":"[PATCH/RFC v1 1/5] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T08:36:25Z","receivedAt":"2014-06-09T08:36:25Z","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 <jmmahler@gmail.com>\n---\n strbuf.c | 21 +++++++++++++++++++++\n strbuf.h | 14 ++++++++++++++\n 2 files changed, 35 insertions(+)\n\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.573.ged771ce.dirty\n"},{"id":"243611","messageId":"ce477667323a531b4b3c39724160e66360061816.1402301816.git.jmmahler@gmail.com","threadId":"36871","inReplyTo":"cover.1402301815.git.jmmahler@gmail.com","subject":"[PATCH/RFC v1 2/5] add strbuf_set operations documentation","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T08:36:26Z","receivedAt":"2014-06-09T08:36:26Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Add documentation of the strbuf_set operations to\ntechnical/api-strbuf.txt.\n\nSigned-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n---\n Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex 077a709..ab430d9 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+    Set the buffer to some data up to a given length.\n+\n+`strbuf_setstr`::\n+\n+\tSet the buffer to a NUL-terminated string.\n+\n+`strbuf_setf`::\n+\n+\tSet the buffer to a formatted string.\n+\n+`strbuf_setbuf`::\n+\n+\tSet the current buffer to the contents of some other 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-- \n2.0.0.573.ged771ce.dirty\n"},{"id":"243613","messageId":"d98e710757df0ad7465e074f283f3bf3781f7652.1402301816.git.jmmahler@gmail.com","threadId":"36871","inReplyTo":"cover.1402301815.git.jmmahler@gmail.com","subject":"[PATCH/RFC v1 3/5] sha1_name.c: cleanup using strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T08:36:27Z","receivedAt":"2014-06-09T08:36:27Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplified cases where a strbuf_reset was immediately followed by a\nstrbuf_add using the new strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler <jmmahler@gmail.com>\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.573.ged771ce.dirty\n"},{"id":"243612","messageId":"b64218c4f2a188c5aeb48a07da01972ef4d0ca33.1402301816.git.jmmahler@gmail.com","threadId":"36871","inReplyTo":"cover.1402301815.git.jmmahler@gmail.com","subject":"[PATCH/RFC v1 4/5] fast-import.c: cleanup using strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T08:36:28Z","receivedAt":"2014-06-09T08:36:28Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplified cases where a strbuf_reset was immediately followed by a\nstrbuf_add using the new strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n---\n fast-import.c | 19 ++++++-------------\n 1 file changed, 6 insertions(+), 13 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex e8ec34d..c23935c 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -2741,8 +2741,7 @@ static void parse_new_commit(void)\n \thashcpy(b->branch_tree.versions[0].sha1,\n \t\tb->branch_tree.versions[1].sha1);\n \n-\tstrbuf_reset(&new_data);\n-\tstrbuf_addf(&new_data, \"tree %s\\n\",\n+\tstrbuf_setf(&new_data, \"tree %s\\n\",\n \t\tsha1_to_hex(b->branch_tree.versions[1].sha1));\n \tif (!is_null_sha1(b->sha1))\n \t\tstrbuf_addf(&new_data, \"parent %s\\n\", sha1_to_hex(b->sha1));\n@@ -2829,9 +2828,7 @@ static void parse_new_tag(void)\n \tparse_data(&msg, 0, NULL);\n \n \t/* build the tag object */\n-\tstrbuf_reset(&new_data);\n-\n-\tstrbuf_addf(&new_data,\n+\tstrbuf_setf(&new_data,\n \t\t    \"object %s\\n\"\n \t\t    \"type %s\\n\"\n \t\t    \"tag %s\\n\",\n@@ -2898,8 +2895,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 +2906,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@@ -3034,14 +3029,12 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path)\n \n \tif (!mode) {\n \t\t/* missing SP path LF */\n-\t\tstrbuf_reset(&line);\n-\t\tstrbuf_addstr(&line, \"missing \");\n+\t\tstrbuf_setstr(&line, \"missing \");\n \t\tquote_c_style(path, &line, NULL, 0);\n \t\tstrbuf_addch(&line, '\\n');\n \t} else {\n \t\t/* mode SP type SP object_name TAB path LF */\n-\t\tstrbuf_reset(&line);\n-\t\tstrbuf_addf(&line, \"%06o %s %s\\t\",\n+\t\tstrbuf_setf(&line, \"%06o %s %s\\t\",\n \t\t\t\tmode & ~NO_DELTA, type, sha1_to_hex(sha1));\n \t\tquote_c_style(path, &line, NULL, 0);\n \t\tstrbuf_addch(&line, '\\n');\n-- \n2.0.0.573.ged771ce.dirty\n"},{"id":"243614","messageId":"ddc394a84d897bbbb3b24253a2e45bbcbe4b9f55.1402301816.git.jmmahler@gmail.com","threadId":"36871","inReplyTo":"cover.1402301815.git.jmmahler@gmail.com","subject":"[PATCH/RFC v1 5/5] builtin/remote.c: cleanup using strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T08:36:29Z","receivedAt":"2014-06-09T08:36:29Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Simplified cases where a strbuf_reset was immediately followed by a\nstrbuf_add using the new strbuf_set operations.\n\nSigned-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n---\n builtin/remote.c | 51 +++++++++++++++++----------------------------------\n 1 file changed, 17 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex c9b67ff..b059852 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@@ -589,14 +584,12 @@ static int migrate_file(struct remote *remote)\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@@ -644,23 +637,20 @@ static int mv(int argc, const char **argv)\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_setf(&buf, \"remote.%s\", rename.old);\n \tstrbuf_addf(&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 \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.573.ged771ce.dirty\n"},{"id":"243615","messageId":"CAPig+cSB78YZiqWm7oTn3oFXE2yZZFbKmD6R5oHmEJMc9CtwvQ@mail.gmail.com","threadId":"36871","inReplyTo":"ce477667323a531b4b3c39724160e66360061816.1402301816.git.jmmahler@gmail.com","subject":"Re: [PATCH/RFC v1 2/5] add strbuf_set operations documentation","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-09T09:53:49Z","receivedAt":"2014-06-09T09:53:49Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler <jmmahler@gmail.com> wrote:\n> Add documentation of the strbuf_set operations to\n> technical/api-strbuf.txt.\n\nSince this patch is concise and so closely related to patch 1/5, it\nprobably should be squashed into that one.\n\nMore below.\n\n> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n> ---\n>  Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n>  1 file changed, 18 insertions(+)\n>\n> diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\n> index 077a709..ab430d9 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> +    Set the buffer to some data up to a given length.\n\nI personally find this slightly ambiguous. Upon reading it, the first\nquestion that pops into my mind is whether or not the existing strbuf\ncontent is replaced (even though \"set\" should imply that it is). I\nwonder if it would make sense to rewrite as:\n\n    Set the buffer to [...], replacing the old content\n    of the buffer.\n\nAlternately:\n\n    Replace the buffer content with [...].\n\nDitto for the others.\n\n> +`strbuf_setstr`::\n> +\n> +       Set the buffer to a NUL-terminated string.\n> +\n> +`strbuf_setf`::\n> +\n> +       Set the buffer to a formatted string.\n> +\n> +`strbuf_setbuf`::\n> +\n> +       Set the current buffer to the contents of some other 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> --\n> 2.0.0.573.ged771ce.dirty\n"},{"id":"243616","messageId":"CAPig+cRPPN7N+KUHULXxoGHetN8WPtASxnC7L1fe5rFoBFQ4KA@mail.gmail.com","threadId":"36871","inReplyTo":"b64218c4f2a188c5aeb48a07da01972ef4d0ca33.1402301816.git.jmmahler@gmail.com","subject":"Re: [PATCH/RFC v1 4/5] fast-import.c: cleanup using strbuf_set operations","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-06-09T10:12:12Z","receivedAt":"2014-06-09T10:12:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler <jmmahler@gmail.com> wrote:\n> Subject: fast-import.c: cleanup using strbuf_set operations\n\nThis might read more clearly if written:\n\n    fast-import: simplify via strbuf_set()\n\n> Simplified cases where a strbuf_reset was immediately followed by a\n> strbuf_add using the new strbuf_set operations.\n\nOn this project, commit messages are written in imperative mood:\n\n    Simplify cases where ... is immediately ...\n\nMore below.\n\n> Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n> ---\n>  fast-import.c | 19 ++++++-------------\n>  1 file changed, 6 insertions(+), 13 deletions(-)\n>\n> diff --git a/fast-import.c b/fast-import.c\n> index e8ec34d..c23935c 100644\n> --- a/fast-import.c\n> +++ b/fast-import.c\n> @@ -2741,8 +2741,7 @@ static void parse_new_commit(void)\n>         hashcpy(b->branch_tree.versions[0].sha1,\n>                 b->branch_tree.versions[1].sha1);\n>\n> -       strbuf_reset(&new_data);\n> -       strbuf_addf(&new_data, \"tree %s\\n\",\n> +       strbuf_setf(&new_data, \"tree %s\\n\",\n>                 sha1_to_hex(b->branch_tree.versions[1].sha1));\n>         if (!is_null_sha1(b->sha1))\n>                 strbuf_addf(&new_data, \"parent %s\\n\", sha1_to_hex(b->sha1));\n\nUnlike the cases in patches 3/5 and 5/5 where the strbuf is used or\nreturned immediately following the strbuf_set() call, I am not\nconvinced that this change is an improvement. This code has the\ngeneral form:\n\n    strbuf_reset(...);\n    strbuf_add(...);\n    if (condition)\n        strbuf_add(...);\n    strbuf_add(...);\n\nin which it is clear that the string is being built piecemeal, and\nit's easy for a programmer to insert, remove, or re-order strbuf_add()\ncalls.\n\nReplacing the first two lines with strbuf_set() somewhat obscures the\nfact that the string is going to be built up piecemeal. Plus, the\nchange makes it more difficult to insert, remove, or re-order the\nstrbuf_add() invocations.\n\nThis isn't a strong objection, but the benefit of the change seems\nminimal or non-existent.\n\nDitto for several remaining cases in this patch.\n\n> @@ -2829,9 +2828,7 @@ static void parse_new_tag(void)\n>         parse_data(&msg, 0, NULL);\n>\n>         /* build the tag object */\n> -       strbuf_reset(&new_data);\n> -\n> -       strbuf_addf(&new_data,\n> +       strbuf_setf(&new_data,\n>                     \"object %s\\n\"\n>                     \"type %s\\n\"\n>                     \"tag %s\\n\",\n> @@ -2898,8 +2895,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n>          * Output based on batch_one_object() from cat-file.c.\n>          */\n>         if (type <= 0) {\n> -               strbuf_reset(&line);\n> -               strbuf_addf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n> +               strbuf_setf(&line, \"%s missing\\n\", sha1_to_hex(sha1));\n>                 cat_blob_write(line.buf, line.len);\n>                 strbuf_release(&line);\n>                 free(buf);\n> @@ -2910,8 +2906,7 @@ static void cat_blob(struct object_entry *oe, unsigned char sha1[20])\n>         if (type != OBJ_BLOB)\n>                 die(\"Object %s is a %s but a blob was expected.\",\n>                     sha1_to_hex(sha1), typename(type));\n> -       strbuf_reset(&line);\n> -       strbuf_addf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n> +       strbuf_setf(&line, \"%s %s %lu\\n\", sha1_to_hex(sha1),\n>                                                 typename(type), size);\n>         cat_blob_write(line.buf, line.len);\n>         strbuf_release(&line);\n> @@ -3034,14 +3029,12 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path)\n>\n>         if (!mode) {\n>                 /* missing SP path LF */\n> -               strbuf_reset(&line);\n> -               strbuf_addstr(&line, \"missing \");\n> +               strbuf_setstr(&line, \"missing \");\n>                 quote_c_style(path, &line, NULL, 0);\n>                 strbuf_addch(&line, '\\n');\n>         } else {\n>                 /* mode SP type SP object_name TAB path LF */\n> -               strbuf_reset(&line);\n> -               strbuf_addf(&line, \"%06o %s %s\\t\",\n> +               strbuf_setf(&line, \"%06o %s %s\\t\",\n>                                 mode & ~NO_DELTA, type, sha1_to_hex(sha1));\n>                 quote_c_style(path, &line, NULL, 0);\n>                 strbuf_addch(&line, '\\n');\n> --\n> 2.0.0.573.ged771ce.dirty\n"},{"id":"243617","messageId":"CACsJy8D_eM9OtH=6Z_F0P7GZpqmAdSCEnBjCr_gniZr649P1uw@mail.gmail.com","threadId":"36871","inReplyTo":"cover.1402301815.git.jmmahler@gmail.com","subject":"Re: [PATCH/RFC v1 0/5] add strbuf_set operations","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-06-09T10:39:12Z","receivedAt":"2014-06-09T10:39:12Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Jun 9, 2014 at 3:36 PM, Jeremiah Mahler <jmmahler@gmail.com> 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\nHmm.. I wonder if git-grep could do this.. There's pcre support but I\nnever tried.\n\n>   CNT=$(echo \"$CNT / 2\" | bc)\n>   echo $CNT\n>   70\n\nThe change in this series looks nice. There's another pattern, save\nstrbuf length, then strbuf_setlen() at the beginning or the end of a\nloop. But I think it's less often.\n-- \nDuy\n"},{"id":"243647","messageId":"20140609214958.GA18783@hudson.localdomain","threadId":"36871","inReplyTo":"CAPig+cSB78YZiqWm7oTn3oFXE2yZZFbKmD6R5oHmEJMc9CtwvQ@mail.gmail.com","subject":"Re: [PATCH/RFC v1 2/5] add strbuf_set operations documentation","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T21:49:58Z","receivedAt":"2014-06-09T21:49:58Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Eric,\n\nOn Mon, Jun 09, 2014 at 05:53:49AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler <jmmahler@gmail.com> wrote:\n> > Add documentation of the strbuf_set operations to\n> > technical/api-strbuf.txt.\n> \n> Since this patch is concise and so closely related to patch 1/5, it\n> probably should be squashed into that one.\n> \nFixed.\n\n> More below.\n> \n> > Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n> > ---\n> >  Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++\n> >  1 file changed, 18 insertions(+)\n> >\n> > diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\n> > index 077a709..ab430d9 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> > +    Set the buffer to some data up to a given length.\n> \n> I personally find this slightly ambiguous. Upon reading it, the first\n> question that pops into my mind is whether or not the existing strbuf\n> content is replaced (even though \"set\" should imply that it is). I\n> wonder if it would make sense to rewrite as:\n> \n>     Set the buffer to [...], replacing the old content\n>     of the buffer.\n> \n> Alternately:\n> \n>     Replace the buffer content with [...].\n> \nOn a second reading, I agree that it is ambigous.  'Replace' is much\nmore clear.  Great suggestion.\n\n> Ditto for the others.\n> \n> > +`strbuf_setstr`::\n> > +\n> > +       Set the buffer to a NUL-terminated string.\n> > +\n> > +`strbuf_setf`::\n> > +\n> > +       Set the buffer to a formatted string.\n> > +\n> > +`strbuf_setbuf`::\n> > +\n> > +       Set the current buffer to the contents of some other 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> > --\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"243648","messageId":"20140609220039.GB18783@hudson.localdomain","threadId":"36871","inReplyTo":"CAPig+cRPPN7N+KUHULXxoGHetN8WPtASxnC7L1fe5rFoBFQ4KA@mail.gmail.com","subject":"Re: [PATCH/RFC v1 4/5] fast-import.c: cleanup using strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:00:39Z","receivedAt":"2014-06-09T22:00:39Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Eric,\n\nOn Mon, Jun 09, 2014 at 06:12:12AM -0400, Eric Sunshine wrote:\n> On Mon, Jun 9, 2014 at 4:36 AM, Jeremiah Mahler <jmmahler@gmail.com> wrote:\n> > Subject: fast-import.c: cleanup using strbuf_set operations\n> \n...\n> \n> > Signed-off-by: Jeremiah Mahler <jmmahler@gmail.com>\n> > ---\n> >  fast-import.c | 19 ++++++-------------\n> >  1 file changed, 6 insertions(+), 13 deletions(-)\n> >\n> > diff --git a/fast-import.c b/fast-import.c\n> > index e8ec34d..c23935c 100644\n> > --- a/fast-import.c\n> > +++ b/fast-import.c\n> > @@ -2741,8 +2741,7 @@ static void parse_new_commit(void)\n> >         hashcpy(b->branch_tree.versions[0].sha1,\n> >                 b->branch_tree.versions[1].sha1);\n> >\n> > -       strbuf_reset(&new_data);\n> > -       strbuf_addf(&new_data, \"tree %s\\n\",\n> > +       strbuf_setf(&new_data, \"tree %s\\n\",\n> >                 sha1_to_hex(b->branch_tree.versions[1].sha1));\n> >         if (!is_null_sha1(b->sha1))\n> >                 strbuf_addf(&new_data, \"parent %s\\n\", sha1_to_hex(b->sha1));\n> \n> Unlike the cases in patches 3/5 and 5/5 where the strbuf is used or\n> returned immediately following the strbuf_set() call, I am not\n> convinced that this change is an improvement. This code has the\n> general form:\n> \n>     strbuf_reset(...);\n>     strbuf_add(...);\n>     if (condition)\n>         strbuf_add(...);\n>     strbuf_add(...);\n> \n> in which it is clear that the string is being built piecemeal, and\n> it's easy for a programmer to insert, remove, or re-order strbuf_add()\n> calls.\n> \n> Replacing the first two lines with strbuf_set() somewhat obscures the\n> fact that the string is going to be built up piecemeal. Plus, the\n> change makes it more difficult to insert, remove, or re-order the\n> strbuf_add() invocations.\n> \n> This isn't a strong objection, but the benefit of the change seems\n> minimal or non-existent.\n> \n> Ditto for several remaining cases in this patch.\n> \n...\n\nThis is a great observation that I certainly did overlook.  Using\nstrbuf_add or strbuf_set to help make it more obvious what the code is\ndoing.\n\nBy the same token, strbuf_set can be used to replace strbuf_add to make\nit clear that nothing important was being added to and that the entire\nbuffer is being replaced.\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-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"},{"id":"243649","messageId":"20140609220641.GC18783@hudson.localdomain","threadId":"36871","inReplyTo":"CACsJy8D_eM9OtH=6Z_F0P7GZpqmAdSCEnBjCr_gniZr649P1uw@mail.gmail.com","subject":"Re: [PATCH/RFC v1 0/5] add strbuf_set operations","fromName":"Jeremiah Mahler","fromEmail":"jmmahler@gmail.com","sentAt":"2014-06-09T22:06:41Z","receivedAt":"2014-06-09T22:06:41Z","isPatch":true,"sender":{"key":"jmmahler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/154028?v=4"},"body":"Duy,\n\nOn Mon, Jun 09, 2014 at 05:39:12PM +0700, Duy Nguyen wrote:\n> On Mon, Jun 9, 2014 at 3:36 PM, Jeremiah Mahler <jmmahler@gmail.com> 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> \n> Hmm.. I wonder if git-grep could do this.. There's pcre support but I\n> never tried.\n> \nNot sure if git-grep does this.  The multi-line (-M) support was the\nthing I needed.\n\n> >   CNT=$(echo \"$CNT / 2\" | bc)\n> >   echo $CNT\n> >   70\n> \n> The change in this series looks nice. There's another pattern, save\n> strbuf length, then strbuf_setlen() at the beginning or the end of a\n> loop. But I think it's less often.\n\nA quick look did not see any obvious patterns for this.  I think you are\nright, there may be fewer cases.\n\n> -- \n> Duy\n\n-- \nJeremiah Mahler\njmmahler@gmail.com\nhttp://github.com/jmahler\n"}]}