add strbuf_set operations

A common use case with strubfs is to set the buffer to a new value. This must be done in two steps: a reset followed by an add.

  strbuf_reset(buf);
  strbuf_add(buf, new_buf, len);

In cases where the buffer is being built up in steps, these operations make sense and correctly convey what is being performed.

  strbuf_reset(buf);
  strbuf_add(buf, data1, len1);
  strbuf_add(buf, data2, len2);
  strbuf_add(buf, data3, len3);

However, in other cases, it can be confusing and is not very concise.

  strbuf_reset(buf);
  strbuf_add(buf, default, len1);

  if (cond1) {
    strbuf_reset(buf);
    strbuf_add(buf, data2, len2);
  }

  if (cond2) {
    strbuf_reset(buf);
    strbuf_add(buf, data3, len3);
  }

Add strbuf_set operations so that it can be re-written in a clear and concise way.

  strbuf_set(buf, default len1);

Am 12.06.2014 09:29, schrieb Jeremiah Mahler:

Quoted 29 lines

A common use case with strubfs is to set the buffer to a new value. This must be done in two steps: a reset followed by an add.

  strbuf_reset(buf);
  strbuf_add(buf, new_buf, len);

In cases where the buffer is being built up in steps, these operations make sense and correctly convey what is being performed.

  strbuf_reset(buf);
  strbuf_add(buf, data1, len1);
  strbuf_add(buf, data2, len2);
  strbuf_add(buf, data3, len3);

However, in other cases, it can be confusing and is not very concise.

  strbuf_reset(buf);
  strbuf_add(buf, default, len1);

  if (cond1) {
    strbuf_reset(buf);
    strbuf_add(buf, data2, len2);
  }

  if (cond2) {
    strbuf_reset(buf);
    strbuf_add(buf, data3, len3);
  }

Quoted 4 lines

Add strbuf_set operations so that it can be re-written in a clear and concise way.

  strbuf_set(buf, default len1);

very minor nit: missing comma between default and len1.

Thomas,

Quoted 2 lines · Thomas Braun

Am 12.06.2014 09:29, schrieb Jeremiah Mahler:

Quoted 1 line

A common use case with strubfs is to set the buffer to a new value.

Quoted 28 lines

Quoted 28 lines

This must be done in two steps: a reset followed by an add.

  strbuf_reset(buf);
  strbuf_add(buf, new_buf, len);

In cases where the buffer is being built up in steps, these operations make sense and correctly convey what is being performed.

  strbuf_reset(buf);
  strbuf_add(buf, data1, len1);
  strbuf_add(buf, data2, len2);
  strbuf_add(buf, data3, len3);

However, in other cases, it can be confusing and is not very concise.

  strbuf_reset(buf);
  strbuf_add(buf, default, len1);

  if (cond1) {
    strbuf_reset(buf);
    strbuf_add(buf, data2, len2);
  }

  if (cond2) {
    strbuf_reset(buf);
    strbuf_add(buf, data3, len3);
  }

Quoted 5 lines

Quoted 4 lines

Add strbuf_set operations so that it can be re-written in a clear and concise way.

  strbuf_set(buf, default len1);

very minor nit: missing comma between default and len1.

I can't believe I missed that. Good catch ;-)

Quoted 5 lines · Jeremiah Mahler

Thomas,

Quoted 2 lines · Thomas Braun

Am 12.06.2014 09:29, schrieb Jeremiah Mahler:

Quoted 1 line

A common use case with strubfs is to set the buffer to a new value.

strubfs???

Quoted 4 lines · Junio C Hamano

Quoted 2 lines · Jeremiah Mahler

Thomas,

Quoted 6 lines

Quoted 3 lines · Thomas Braun

Quoted 2 lines

Am 12.06.2014 09:29, schrieb Jeremiah Mahler:

Quoted 1 line

A common use case with strubfs is to set the buffer to a new value.

strubfs???

I was trying to make it plural. Perhaps strbuf's?

Quoted 6 lines · Jeremiah Mahler

Quoted 5 lines · Junio C Hamano

Quoted 3 lines · Jeremiah Mahler

Thomas,

On Thu, Jun 12, 2014 at 10:11:36AM +0200, Thomas Braun wrote:

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

Am 12.06.2014 09:29, schrieb Jeremiah Mahler:

Quoted 1 line

A common use case with strubfs is to set the buffer to a new value.

strubfs???

I was trying to make it plural. Perhaps strbuf's?

Junio was pointing out your misspelling, not your intention to pluralize.

A common use case with strubfs is to set the buffer to a new value. This must be done in two steps: a reset followed by an add.

  strbuf_reset(buf);
  strbuf_add(buf, new_buf, len);

In cases where the buffer is being built up in steps, these operations make sense and correctly convey what is being performed.

  strbuf_reset(buf);
  strbuf_add(buf, data1, len1);
  strbuf_add(buf, data2, len2);
  strbuf_add(buf, data3, len3);

However, in other cases, it can be confusing and is not very concise.

  strbuf_reset(buf);
  strbuf_add(buf, default, len1);

  if (cond1) {
    strbuf_reset(buf);
    strbuf_add(buf, data2, len2);
  }

  if (cond2) {
    strbuf_reset(buf);
    strbuf_add(buf, data3, len3);
  }

Add strbuf_set operations so that it can be re-written in a clear and concise way.

  strbuf_set(buf, default len1);

  if (cond1) {
    strbuf_set(buf, data2, len2);
  }

  if (cond2) {
    strbuf_set(buf, data3, len3);
  }

Signed-off-by Jeremiah Mahler <[email protected]>

---
 Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++
 strbuf.c                               | 21 +++++++++++++++++++++
 strbuf.h                               | 13 +++++++++++++
 3 files changed, 52 insertions(+)

Documentation/technical/api-strbuf.txt +18 -0

index f9c06a7..ae9c9cc 100644
@@ -149,6 +149,24 @@ Functions
 	than zero if the first buffer is found, respectively, to be less than,
 	to match, or be greater than the second buffer.
 
+* Setting the buffer
+
+`strbuf_set`::
+
+	Replace content with data of a given length.
+
+`strbuf_setstr`::
+
+	Replace content with data from a NUL-terminated string.
+
+`strbuf_setf`::
+
+	Replace content with a formatted string.
+
+`strbuf_setbuf`::
+
+	Replace content with data from another buffer.
+
 * Adding data to the buffer
 
 NOTE: All of the functions in this section will grow the buffer as necessary.

strbuf.c +21 -0

index ac62982..9d64b00 100644
@@ -189,6 +189,27 @@ void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
 	strbuf_setlen(sb, sb->len + dlen - len);
 }
 
+void strbuf_set(struct strbuf *sb, const void *data, size_t len)
+{
+	strbuf_reset(sb);
+	strbuf_add(sb, data, len);
+}
+
+void strbuf_setf(struct strbuf *sb, const char *fmt, ...)
+{
+	va_list ap;
+	strbuf_reset(sb);
+	va_start(ap, fmt);
+	strbuf_vaddf(sb, fmt, ap);
+	va_end(ap);
+}
+
+void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2)
+{
+	strbuf_reset(sb);
+	strbuf_add(sb, sb2->buf, sb2->len);
+}
+
 void strbuf_insert(struct strbuf *sb, size_t pos, const void *data, size_t len)
 {
 	strbuf_splice(sb, pos, 0, data, len);

strbuf.h +13 -0

index e9ad03e..5041c35 100644
@@ -101,6 +101,19 @@ static inline struct strbuf **strbuf_split(const struct strbuf *sb,
  */
 extern void strbuf_list_free(struct strbuf **);
 
+/*----- set buffer to data -----*/
+extern void strbuf_set(struct strbuf *sb, const void *data, size_t len);
+
+static inline void strbuf_setstr(struct strbuf *sb, const char *s)
+{
+	strbuf_set(sb, s, strlen(s));
+}
+
+__attribute__((format (printf,2,3)))
+extern void strbuf_setf(struct strbuf *sb, const char *fmt, ...);
+
+extern void strbuf_setbuf(struct strbuf *sb, const struct strbuf *sb2);
+
 /*----- add data in your buffer -----*/
 static inline void strbuf_addch(struct strbuf *sb, int c)
 {

Quoted 32 lines · Jeremiah Mahler

A common use case with strubfs is to set the buffer to a new value. This must be done in two steps: a reset followed by an add.

  strbuf_reset(buf);
  strbuf_add(buf, new_buf, len);

In cases where the buffer is being built up in steps, these operations make sense and correctly convey what is being performed.

  strbuf_reset(buf);
  strbuf_add(buf, data1, len1);
  strbuf_add(buf, data2, len2);
  strbuf_add(buf, data3, len3);

However, in other cases, it can be confusing and is not very concise.

  strbuf_reset(buf);
  strbuf_add(buf, default, len1);

  if (cond1) {
    strbuf_reset(buf);
    strbuf_add(buf, data2, len2);
  }

  if (cond2) {
    strbuf_reset(buf);
    strbuf_add(buf, data3, len3);
  }

Add strbuf_set operations so that it can be re-written in a clear and concise way.

Quoted 9 lines

  strbuf_set(buf, default len1);

  if (cond1) {
    strbuf_set(buf, data2, len2);
  }

  if (cond2) {
    strbuf_set(buf, data3, len3);
  }

Or even more concisely without making unnecessary internal calls to strbuf_reset():

	strbuf_reset(buf);
        if (cond2)
        	strbuf_add(buf, data3, len3);
	else if (cond1)
        	strbuf_add(buf, data2, len2);
	else
        	strbuf_add(buf, default, len2);

;-)

I am on the fence.

I have this suspicion that the addition of strbuf_set() would *only* help when the original written with reset-and-then-add sequence was suboptimal to begin with, and it helps *only* how the code reads, without correcting the fact that it is still doing unnecessary "first set to a value to be discarded and then reset to set the right value", sweeping the issue under the rug.

Repeated reset-and-then-add on the same strbuf used to be something that may indicate that the code is doing unnecessary work. Now, repeated uses of strbuf_set on the same strbuf replaced that pattern to be watched for to spot wasteful code paths.

I dunno...

Quoted 15 lines

Signed-off-by Jeremiah Mahler <[email protected]>

---
 Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++
 strbuf.c                               | 21 +++++++++++++++++++++
 strbuf.h                               | 13 +++++++++++++
 3 files changed, 52 insertions(+)

Documentation/technical/api-strbuf.txt +0 -0

index f9c06a7..ae9c9cc 100644
@@ -149,6 +149,24 @@ Functions
 	than zero if the first buffer is found, respectively, to be less than,
 	to match, or be greater than the second buffer.
 /*----- add data in your buffer -----*/

Quoted 2 lines

static inline void strbuf_addch(struct strbuf *sb, int c) {

Quoted 9 lines · Junio C Hamano

I am on the fence.

I have this suspicion that the addition of strbuf_set() would *only* help when the original written with reset-and-then-add sequence was suboptimal to begin with, and it helps *only* how the code reads, without correcting the fact that it is still doing unnecessary "first set to a value to be discarded and then reset to set the right value", sweeping the issue under the rug.

It is certainly possible that builtin/remote.c (PATCH 2/2) saw the most benefit from this operation because it is so badly designed. But this seems unlikely given the review process around here ;-)

The one case where it would be doing extra work is when a strbuf_set replaces a strbuf_add that didn't previously have a strbuf_reset. strbuf_set is not appropriate for all cases, as I mentioned in the patch, but in some cases I think it makes it more readable. And in this case it would be doing a reset on an empty strbuf. Is avoiding this expense worth the reduction in readability?

Also, as Eric Sunshine pointed out, being able to easily re-order operations can make the code easier to maintain.

Quoted 5 lines

Repeated reset-and-then-add on the same strbuf used to be something that may indicate that the code is doing unnecessary work. Now, repeated uses of strbuf_set on the same strbuf replaced that pattern to be watched for to spot wasteful code paths.

If a reset followed by and add was a rare occurrence I would tend to agree more.

Quoted 17 lines

I dunno...

Quoted 15 lines

Signed-off-by Jeremiah Mahler <[email protected]>

---
 Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++
 strbuf.c                               | 21 +++++++++++++++++++++
 strbuf.h                               | 13 +++++++++++++
 3 files changed, 52 insertions(+)

Documentation/technical/api-strbuf.txt +0 -0

index f9c06a7..ae9c9cc 100644
@@ -149,6 +149,24 @@ Functions
    than zero if the first buffer is found, respectively, to be less than,
    to match, or be greater than the second buffer.
 /*----- add data in your buffer -----*/

...

Quoted 2 lines

Quoted 2 lines

static inline void strbuf_addch(struct strbuf *sb, int c) {

--
Jeremiah Mahler
[email protected]
http://github.com/jmahler

Quoted 24 lines · Jeremiah Mahler

Quoted 9 lines · Junio C Hamano

I am on the fence.

I have this suspicion that the addition of strbuf_set() would *only* help when the original written with reset-and-then-add sequence was suboptimal to begin with, and it helps *only* how the code reads, without correcting the fact that it is still doing unnecessary "first set to a value to be discarded and then reset to set the right value", sweeping the issue under the rug.

It is certainly possible that builtin/remote.c (PATCH 2/2) saw the most benefit from this operation because it is so badly designed. But this seems unlikely given the review process around here ;-)

The one case where it would be doing extra work is when a strbuf_set replaces a strbuf_add that didn't previously have a strbuf_reset. strbuf_set is not appropriate for all cases, as I mentioned in the patch, but in some cases I think it makes it more readable. And in this case it would be doing a reset on an empty strbuf. Is avoiding this expense worth the reduction in readability?

Also, as Eric Sunshine pointed out, being able to easily re-order operations can make the code easier to maintain.

Quoted 7 lines

Quoted 5 lines

Repeated reset-and-then-add on the same strbuf used to be something that may indicate that the code is doing unnecessary work. Now, repeated uses of strbuf_set on the same strbuf replaced that pattern to be watched for to spot wasteful code paths.

If a reset followed by and add was a rare occurrence I would tend to agree more.

When composing my review of the builtin/remote.c changes, I wrote something like this:

Although strbuf_set() does make the code a bit easier to read when strbufs are repeatedly re-used, re-using a variable for different purposes is generally considered poor programming practice. It's likely that heavy re-use of strbufs has been tolerated to avoid multiple heap allocations, but that may be a case of premature (allocation) optimization, rather than good programming. A different ("better") way to make the code more readable and maintainable may be to ban re-use of strbufs for different purposes.

But I deleted it before sending because it's a somewhat tangential issue not introduced by your changes. However, I do see strbuf_set() as a Band-Aid for the problem described above, rather than as a useful feature on its own. If the practice of re-using strbufs (as a premature optimization) ever becomes taboo, then strbuf_set() loses its value.

Quoted 25 lines

Quoted 17 lines

I dunno...

Quoted 15 lines

Signed-off-by Jeremiah Mahler <[email protected]>

---
 Documentation/technical/api-strbuf.txt | 18 ++++++++++++++++++
 strbuf.c                               | 21 +++++++++++++++++++++
 strbuf.h                               | 13 +++++++++++++
 3 files changed, 52 insertions(+)

Documentation/technical/api-strbuf.txt +0 -0

index f9c06a7..ae9c9cc 100644
@@ -149,6 +149,24 @@ Functions
    than zero if the first buffer is found, respectively, to be less than,
    to match, or be greater than the second buffer.
 /*----- add data in your buffer -----*/

...

Quoted 2 lines

Quoted 2 lines

static inline void strbuf_addch(struct strbuf *sb, int c) {

--
Jeremiah Mahler
[email protected]
http://github.com/jmahler

Quoted 10 lines

Although strbuf_set() does make the code a bit easier to read when strbufs are repeatedly re-used, re-using a variable for different purposes is generally considered poor programming practice. It's likely that heavy re-use of strbufs has been tolerated to avoid multiple heap allocations, but that may be a case of premature (allocation) optimization, rather than good programming. A different ("better") way to make the code more readable and maintainable may be to ban re-use of strbufs for different purposes.

Quoted 7 lines

But I deleted it before sending because it's a somewhat tangential issue not introduced by your changes. However, I do see strbuf_set() as a Band-Aid for the problem described above, rather than as a useful feature on its own. If the practice of re-using strbufs (as a premature optimization) ever becomes taboo, then strbuf_set() loses its value.

I am getting the feeling that I have mis-understood the purpose of strbufs. It is not just a library to use in place of char*.

If strbufs should only be added to and never reset a good test would be to re-write builtin/remote.c without the use of strbuf_reset.

builtin/remote.c does re-use the buffers. But it seems if a buffer is used N times then to avoid a reset you would need N buffers.

But on the other hand I agree with your comment that re-using a variable for different purposes is poor practice.

Now I am not even sure if I want my own patch :-)

Quoted 24 lines · Jeremiah Mahler

Quoted 10 lines

Although strbuf_set() does make the code a bit easier to read when strbufs are repeatedly re-used, re-using a variable for different purposes is generally considered poor programming practice. It's likely that heavy re-use of strbufs has been tolerated to avoid multiple heap allocations, but that may be a case of premature (allocation) optimization, rather than good programming. A different ("better") way to make the code more readable and maintainable may be to ban re-use of strbufs for different purposes.

Quoted 7 lines

But I deleted it before sending because it's a somewhat tangential issue not introduced by your changes. However, I do see strbuf_set() as a Band-Aid for the problem described above, rather than as a useful feature on its own. If the practice of re-using strbufs (as a premature optimization) ever becomes taboo, then strbuf_set() loses its value.

I am getting the feeling that I have mis-understood the purpose of strbufs. It is not just a library to use in place of char*.

If strbufs should only be added to and never reset a good test would be to re-write builtin/remote.c without the use of strbuf_reset.

Quoted 7 lines

builtin/remote.c does re-use the buffers. But it seems if a buffer is used N times then to avoid a reset you would need N buffers.

But on the other hand I agree with your comment that re-using a variable for different purposes is poor practice.

Now I am not even sure if I want my own patch :-)

I think reusing buffers like remote.c does makes things uglier and more confusing than necessary, and probably doesn't have any appreciable performance gain. We are saving a handful of allocations, and have such wonderful variable names as "buf2" (when is it OK to reuse that one, versus regular "buf"?).

A better reason I think is to reuse allocations across a loop, like:

  struct strbuf buf = STRBUF_INIT;

  for (i = 0; i < nr; i++) {
	strbuf_reset(&buf);
	strbuf_add(&buf, foo[i]);
	... do something with buf ...

}
strbuf_release(&buf);

You can write that as:

  for (i = 0; i < nr; i++) {
	struct strbuf buf = STRBUF_INIT;
	strbuf_add(&buf, foo[i]);
	... do something ...
	strbuf_release(&buf);

}

and it is definitely still a case of premature optimization. But:

1. "nr" here may be very large, so the amortized benefits are greater

2. It's only one call to strbuf_reset to cover many items. Not one sprinkled every few lines.

You'll note that strbuf_getline uses a "set" convention (making it different from the rest of strbuf) to handle this looping case.

I don't have a problem with strbuf_set, but just peeking at remote.c, I think I'd rather see it cleaned up. It looks like one of the major uses is setting config variables. I wonder how hard it would be to make a git_config_set variant that took printf-style formats, and handled the strbuf itself.

-Peff

builtin/remote.c has many cases where a strbuf was being set to a new value. Improve its readability by using strbuf_set() operations instead of reset + add.

Signed-off-by Jeremiah Mahler <[email protected]>

---
 builtin/remote.c | 63 +++++++++++++++++++++-----------------------------------
 1 file changed, 23 insertions(+), 40 deletions(-)

builtin/remote.c +1 -1

index c9b67ff..94f03e2 100644
@@ -184,17 +184,16 @@ static int add(int argc, const char **argv)
                        remote->fetch_refspec_nr))
                die(_("remote %s already exists."), name);
 
-       strbuf_addf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
+       strbuf_setf(&buf2, "refs/heads/test:refs/remotes/%s/test", name);
        if (!valid_fetch_refspec(buf2.buf))
                die(_("'%s' is not a valid remote name"), name);

builtin/remote.c +20 -37

                if (track.nr == 0)
                        string_list_append(&track, "*");
                for (i = 0; i < track.nr; i++) {

@@ -205,15 +204,13 @@ static int add(int argc, const char **argv)
        }
 
        if (mirror & MIRROR_PUSH) {
-               strbuf_reset(&buf);
-               strbuf_addf(&buf, "remote.%s.mirror", name);
+               strbuf_setf(&buf, "remote.%s.mirror", name);
                if (git_config_set(buf.buf, "true"))
                        return 1;
        }
 
        if (fetch_tags != TAGS_DEFAULT) {
-               strbuf_reset(&buf);
-               strbuf_addf(&buf, "remote.%s.tagopt", name);
+               strbuf_setf(&buf, "remote.%s.tagopt", name);
                if (git_config_set(buf.buf,
                        fetch_tags == TAGS_SET ? "--tags" : "--no-tags"))
                        return 1;

@@ -223,11 +220,9 @@ static int add(int argc, const char **argv)
                return 1;
 
        if (master) {
-               strbuf_reset(&buf);
-               strbuf_addf(&buf, "refs/remotes/%s/HEAD", name);
+               strbuf_setf(&buf, "refs/remotes/%s/HEAD", name);
 
-               strbuf_reset(&buf2);
-               strbuf_addf(&buf2, "refs/remotes/%s/%s", name, master);
+               strbuf_setf(&buf2, "refs/remotes/%s/%s", name, master);
 
                if (create_symref(buf.buf, buf2.buf, "remote add"))
                        return error(_("Could not setup master '%s'"), master);

@@ -584,19 +579,17 @@ static int migrate_file(struct remote *remote)
        int i;
        const char *path = NULL;
 
-       strbuf_addf(&buf, "remote.%s.url", remote->name);
+       strbuf_setf(&buf, "remote.%s.url", remote->name);
        for (i = 0; i < remote->url_nr; i++)
                if (git_config_set_multivar(buf.buf, remote->url[i], "^$", 0))
                        return error(_("Could not append '%s' to '%s'"),
                                        remote->url[i], buf.buf);
-       strbuf_reset(&buf);
-       strbuf_addf(&buf, "remote.%s.push", remote->name);
+       strbuf_setf(&buf, "remote.%s.push", remote->name);
        for (i = 0; i < remote->push_refspec_nr; i++)
                if (git_config_set_multivar(buf.buf, remote->push_refspec[i], "^$", 0))
                        return error(_("Could not append '%s' to '%s'"),
                                        remote->push_refspec[i], buf.buf);
-       strbuf_reset(&buf);
-       strbuf_addf(&buf, "remote.%s.fetch", remote->name);
+       strbuf_setf(&buf, "remote.%s.fetch", remote->name);
        for (i = 0; i < remote->fetch_refspec_nr; i++)
                if (git_config_set_multivar(buf.buf, remote->fetch_refspec[i], "^$", 0))
                        return error(_("Could not append '%s' to '%s'"),

@@ -640,27 +633,24 @@ static int mv(int argc, const char **argv)
        if (newremote && (newremote->url_nr > 1 || newremote->fetch_refspec_nr))
                die(_("remote %s already exists."), rename.new);
 
-       strbuf_addf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
+       strbuf_setf(&buf, "refs/heads/test:refs/remotes/%s/test", rename.new);
        if (!valid_fetch_refspec(buf.buf))
                die(_("'%s' is not a valid remote name"), rename.new);
 
-       strbuf_reset(&buf);
-       strbuf_addf(&buf, "remote.%s", rename.old);
-       strbuf_addf(&buf2, "remote.%s", rename.new);
+       strbuf_setf(&buf, "remote.%s", rename.old);
+       strbuf_setf(&buf2, "remote.%s", rename.new);
        if (git_config_rename_section(buf.buf, buf2.buf) < 1)
                return error(_("Could not rename config section '%s' to '%s'"),
                                buf.buf, buf2.buf);
 
-       strbuf_reset(&buf);
-       strbuf_addf(&buf, "remote.%s.fetch", rename.new);
+       strbuf_setf(&buf, "remote.%s.fetch", rename.new);
        if (git_config_set_multivar(buf.buf, NULL, NULL, 1))
                return error(_("Could not remove config section '%s'"), buf.buf);
-       strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old);
+       strbuf_setf(&old_remote_context, ":refs/remotes/%s/", rename.old);
        for (i = 0; i < oldremote->fetch_refspec_nr; i++) {
                char *ptr;
 
-               strbuf_reset(&buf2);
-               strbuf_addstr(&buf2, oldremote->fetch_refspec[i]);
+               strbuf_setstr(&buf2, oldremote->fetch_refspec[i]);
                ptr = strstr(buf2.buf, old_remote_context.buf);
                if (ptr) {
                        refspec_updated = 1;

@@ -683,8 +673,7 @@ static int mv(int argc, const char **argv)
                struct string_list_item *item = branch_list.items + i;
                struct branch_info *info = item->util;
                if (info->remote_name && !strcmp(info->remote_name, rename.old)) {
-                       strbuf_reset(&buf);
-                       strbuf_addf(&buf, "branch.%s.remote", item->string);
+                       strbuf_setf(&buf, "branch.%s.remote", item->string);
                        if (git_config_set(buf.buf, rename.new)) {
                                return error(_("Could not set '%s'"), buf.buf);
                        }

@@ -715,12 +704,10 @@ static int mv(int argc, const char **argv)
 
                if (item->util)
                        continue;
-               strbuf_reset(&buf);
-               strbuf_addstr(&buf, item->string);
+               strbuf_setstr(&buf, item->string);
                strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old),
                                rename.new, strlen(rename.new));
-               strbuf_reset(&buf2);
-               strbuf_addf(&buf2, "remote: renamed %s to %s",
+               strbuf_setf(&buf2, "remote: renamed %s to %s",
                                item->string, buf.buf);
                if (rename_ref(item->string, buf.buf, buf2.buf))
                        die(_("renaming '%s' failed"), item->string);

@@ -730,16 +717,13 @@ static int mv(int argc, const char **argv)
 
                if (!item->util)
                        continue;
-               strbuf_reset(&buf);
-               strbuf_addstr(&buf, item->string);
+               strbuf_setstr(&buf, item->string);
                strbuf_splice(&buf, strlen("refs/remotes/"), strlen(rename.old),
                                rename.new, strlen(rename.new));
-               strbuf_reset(&buf2);
-               strbuf_addstr(&buf2, item->util);
+               strbuf_setstr(&buf2, item->util);
                strbuf_splice(&buf2, strlen("refs/remotes/"), strlen(rename.old),
                                rename.new, strlen(rename.new));
-               strbuf_reset(&buf3);
-               strbuf_addf(&buf3, "remote: renamed %s to %s",
+               strbuf_setf(&buf3, "remote: renamed %s to %s",
                                item->string, buf.buf);
                if (create_symref(buf.buf, buf2.buf, buf3.buf))
                        die(_("creating '%s' failed"), buf.buf);

@@ -804,8 +788,7 @@ static int rm(int argc, const char **argv)
                if (info->remote_name && !strcmp(info->remote_name, remote->name)) {
                        const char *keys[] = { "remote", "merge", NULL }, **k;
                        for (k = keys; *k; k++) {
-                               strbuf_reset(&buf);
-                               strbuf_addf(&buf, "branch.%s.%s",
+                               strbuf_setf(&buf, "branch.%s.%s",
                                                item->string, *k);
                                if (git_config_set(buf.buf, NULL)) {
                                        strbuf_release(&buf);
--

2.0.0