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

On Thu, Jul 21, 2016 at 06:46:44PM +0200, René Scharfe wrote:

quoted

quoted

-	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).

Ah, I wasn't aware of that.  Calling strbuf_grow() twice shouldn't be
thaaat bad.  However, I wonder where we duplicate strbufs, or why we
would ever want to do so.  Anyway, here's a patch for that:

I don't think we do. Going back to the original discussion:

  http://thread.gmane.org/gmane.comp.version-control.git/136141/focus=136774

it was mostly just "hey, this would fail really confusingly if we ever
did, so let's make it safe".

The second strbuf_grow() is by definition a noop (which is why 81d2cae
works at all), but we do pay the size-computation cost.

-- >8 --
Subject: strbuf: avoid calling strbuf_grow() twice in strbuf_addbuf()

Implement strbuf_addbuf() as a normal function in order to avoid calling
strbuf_grow() twice, with the second callinside strbud_add() being a
no-op.  This is slightly faster and also reduces the text size a bit.

Seems reasonable.  IMHO the main advantage is that one does not have to
reason about the double strbuf_grow() (i.e., that the strbuf_add() is
safe because we know its grow is a noop).

-Peff