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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html