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

From: Jeff King <peff@peff.net>
Date: Jul 20, 2016, 13:20 UTC
Message-ID: <20160720132025.GD17469@sigill.intra.peff.net>
In-Reply-To: <578E732D.2090803@web.de>

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:

Show 10 quoted lines

> 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