On Sun, Feb 18, 2024 at 08:59:38PM +0100, Maarten Bosmans wrote:
> @@ -705,12 +703,11 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> if (!prev_buf)
> die(_("unable to read %s"), oid_to_hex(note));
> if (size)
> - strbuf_add(&buf, prev_buf, size);
> + strbuf_attach(&buf, prev_buf, size, size + 1);
> if (d.buf.len && size)
> append_separator(&buf);
> strbuf_insert(&d.buf, 0, buf.buf, buf.len);
>
> - free(prev_buf);
> strbuf_release(&buf);
> }
Is it possible for "size" to be 0, but prev_buf to be non-NULL? I assume
it is so if the previous note is the empty object (and anyway, we'd have
died earlier if prev_buf was NULL). In that case your patch introduces a
leak (we do not attach prev_buf to buf, but we no longer free prev_buf).
I'm a little skeptical that this is actually increasing the speed of the
command in a measurable way, though. It's one allocation/copy, right
next to a big old strbuf_insert() that is going to splice into an
existing array.
-Peff
