# Re: \[PATCH 2/8\] builtin/multi-pack-index.c: support --stdin-packs mode

- From: Eric Sunshine \<hidden\>
- Date: 2021-09-15 01:56:01 
- List: git
- Page: https://ratatoskr.run/git/2021/09/7902528
- Thread: https://ratatoskr.run/git/2021/09/7902077/t.md

```text
On Tue, Sep 14, 2021 at 7:48 PM Taylor Blau [ref] 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.
```
