On Tue, Sep 14, 2021 at 7:48 PM Taylor Blau <me@xxxxxxxxxxxx> wrote: > On Tue, Sep 14, 2021 at 03:02:38PM -0400, Jeff King wrote: > > I think the root of the problem here is the non-idiomatic use of > > strbuf_getline(). The usual thing (and in fact the thing done by the > > quite-similar code in read_packs_list_from_stdin() in pack-objects.c) > > is not to detach, because strbuf_getline() will reset the buffer each > > time. I.e.: > > > > struct string_list to = STRING_LIST_INIT_DUP; > > ... > > struct strbuf buf = STRBUF > > while (strbuf_getline(&buf, stdin) != EOF) > > string_list_append(to, &buf); > > > That avoids any clever string-list allocation games. The number of heap > > allocations is about the same (one strbuf and N list items, versus N > > strbufs and 0 list items). There's a little extra copying (from the > > strbuf into the list items), but the strings themselves are more > > efficiently allocated (strbuf may over-allocate, and we lock in > > that choice forever by handing over the string). > > > > Not that efficiency probably matters either way for this spot. I'd do it > > this way because it's simple and idiomatic for our code base. > > More than anything, I'm persuaded by that (though I was quite partial to > Eric's suggestion, which I found very clever). For what it's worth, Peff's suggestion seems best of all those presented thus far.
