Re: [PATCH] use strbuf_addbuf() for appending a strbuf to another

[Jeff King <peff@peff.net>]

On Tue, Jul 19, 2016 at 08:36:29PM +0200, René Scharfe wrote:

> Use strbuf_addbuf() where possible; it's shorter and more efficient.

After seeing "efficient", I was momentarily surprised by the first hunk:

> diff --git a/dir.c b/dir.c
> index 6172b34..0ea235f 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -2364,7 +2364,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
>  
>  	varint_len = encode_varint(untracked->ident.len, varbuf);
>  	strbuf_add(out, varbuf, varint_len);
> -	strbuf_add(out, untracked->ident.buf, untracked->ident.len);
> +	strbuf_addbuf(out, &untracked->ident);

This is actually slightly _less_ efficient, because we already are using
the precomputed len, and the new code will call an extra strbuf_grow()
to cover the case where the two arguments are the same.  See 81d2cae
(strbuf_addbuf(): allow passing the same buf to dst and src,
2010-01-12).

But it almost certainly doesn't matter, and it definitely _is_ an
improvement for the other "addstr" cases, which are doing an unncessary
strlen().

And anyway the readability improvement trumps all of that in my mind. So
I think overall it is a nice cleanup; I'm mostly just commenting for the
sake of other reviewers.

-Peff